-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Helper function WP_CLI\Entity\Utils::has_stdin()
misfires depending on shell environment
#164
Comments
At first glance, this looks like a bug in the argument parsing code. I could reproduce it by adding
I suppose it detects the |
Just stumbled opon the same issue.
} elseif ( \WP_CLI\Entity\Utils::has_stdin() ||true) {
$stdin_value = WP_CLI::get_value_from_arg_or_stdin( $args, -1 );
$patch_value = WP_CLI::read_value( trim( $stdin_value ), $assoc_args );
} else {
$patch_value = WP_CLI::read_value( array_pop( $key_path ), $assoc_args );
} It looks to me like the first branch here is missing that |
I came across this issue when running
with:
|
Some more details on this bug after trying to resolve it without success yesterday... The root cause lies in This in turn will cause the above logic to miscount the arguments. I fixed the symptom in the above code in the following way:
So, for the The one case I know of so far is |
WP_CLI\Entity\Utils::has_stdin()
misfires depending on shell environment
Related #194 |
I've been testing https://stackoverflow.com/a/11327451/1434155, and it works on all the shells I've tested (on my Mac). Maybe rewrite |
@soderlind I already experimented with a similar approach, reading the mode directly, but the main issue is not detecting what type of pipe we have, but rather, whether the pipe has content or not. Did you find a way to do that through the above (mode-based) method, or did you only test for the type of pipe? |
Initially I only tested detecting what type of pipe I had, but (so far) this works for reading stdin: private function read( $fp ) {
$fstats = fstat( $fp );
return fread( $fp, $fstats['size'] );
}
public function __construct() {
echo $this->read( STDIN );
} tested Reason I'm looking into this is that I want to do wxr2pdf is at https://github.com/soderlind/wxr2pdf |
@soderlind The above code for reading from STDIN would be a blocking request, though. That works fine if you can rely on STDIN content being available. But if the content is not available, WP-CLI will just hang, without the user knowing what is going on... I'll experiment some more with this, maybe it is just about finding the right combination of checking mode and reading/skipping. |
I may have found a related issue. The WordPress plugin Authorizer uses an option called auth_settings which contains numerous sub-options. Boolean values for these sub-options must be strings, so if I want to change the value of 'ldap' from off to on, I have to set it to '1', not 1. The following does not work, as it sets the value to a numeric 1 rather than a string '1': If I do the following, on non-interactive shells (like being called from a script), this works to set the value correctly: However, if I do the same in an interactive shell, it actually replaces the entirety of the auth_settings option with the string 'ldap', as if I'd run |
So glad this issue was here. Thought I was going mad. |
For anyone else struggling with this, I seem to be able to work around this issue by adding |
Given a command like:
wp option patch update gglcptch_options login_form 0
run on a non interactive shell (like through atd/crod/puppet exec) we receive an Error:
Error: No data exists for key "0"
if we run the same program through the command line everything works fine.
Taking a look at the code here
https://github.com/wp-cli/entity-command/blob/v1.2.0/src/Option_Command.php#L525
and trying the command:
echo 1 | wp option patch update gglcptch_options login_form
gave us the expected results even in atd/crod/puppet exec shells.
The text was updated successfully, but these errors were encountered: