Skip to content
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

Quoting additional argument doesn't seem to work #21

Open
danielpataki opened this issue Mar 18, 2015 · 10 comments
Open

Quoting additional argument doesn't seem to work #21

danielpataki opened this issue Mar 18, 2015 · 10 comments

Comments

@danielpataki
Copy link

I'm sure this is an error on my end since this would have been reported by now, but it seems I am unable to add parameters to a command. I would like to force install a theme which I am doing like this:

wp ssh "theme install wp-content/themes/theme.zip --force" --host=staging

The command produces the following error, as if the whole quoted string was the command name:

Error: 'theme install wp-content/themes/bitesizebio.zip --force' is not a registered wp command. See 'wp help'.

I tested the command on the server without wp-cli-ssh and theme install wp-content/themes/theme.zip --force works just fine. In addition, commands without parameters fail with the same error, eg: wp ssh 'plugin list' --host=staging

I am using the latest version of wp-cli-ssh and wp-cli itself.

Thanks so much for this awesome tool!

@westonruter
Copy link
Contributor

westonruter commented Mar 18, 2015 via email

@davidallansson
Copy link

@danielpataki did you resolve this issue? I'm facing the same right now.

The command i want to run works perfectly fine on the remote server, but when i execute it from my local environment i get the "is not a registered wp command."-error if i add a paramter or try to quote the command to be used.

In my case it happens when i add the --network parameter to the search-replace command. Trying to change the host used on a wpmu network.

wp ssh search-replace www.domain.com dev.domain.com --host=dev

This works fine. But it will only affect the main site.

wp ssh search-replace --network www.domain.com dev.domain.com --host=dev

This will not work. It complains that --network is a unknown parameter.
Error: Parameter errors:
unknown --network parameter

wp ssh "search-replace --network www.domain.com dev.domain.com" --host=dev

This should work accordingly to the documentation. But it dosen't.
Connecting via ssh to host: dev
Error: 'search-replace --network www.domain.com dev.domain.com' is not a registered wp command. See 'wp help'.

For me it looks like wp-cli-ssh dosen't work with quotes anymore. And the quotes will be needed if you want to add parameters to the remote command?

@danielpataki
Copy link
Author

Hi there @davidallansson,

Sorry to @westonruter, I didn't notice his answer. I can't really try this out just now but I don't remember figuring it out. Since I can't try it out now I can't say but I get the feeling it wont work without quotes.

Daniel

@lkraav
Copy link

lkraav commented Jun 15, 2015

This definitely does not work right now, just like the OP says

$ [-] wp ssh plugin activate ewww-image-optimizer --network --host=production
Error: Parameter errors:
 unknown --network parameter
$ [-] wp cli info
PHP binary:     /usr/lib64/php5.6/bin/php
PHP version:    5.6.9-pl0-gentoo
php.ini used:   /etc/php/cli-php5.6/php.ini
WP-CLI root dir:        /home/leho/wp-cli
WP-CLI global config:   /home/leho/.wp-cli/config.yml
WP-CLI project config:  /home/warmpress/wp-cli.yml
WP-CLI version: 0.19.1

@lkraav
Copy link

lkraav commented Jul 3, 2015

Anybody here have any idea about how to go about debugging this? Secondary parameters not working are essentially killing all ability to wp ssh.

@jonathanbardo
Copy link
Contributor

@lkraav I think I found the little issue. Please test my pull request and tell me if it works for you.

A little tweak in the synopsis was all it tooks. Sorry it took me so long to get to it!

@lkraav
Copy link

lkraav commented Jul 10, 2015

It worksssssssss. Ty sir!

Probably not fully on topic here, but is bash-completion not supposed to work for wp-cli-ssh? Everything up to wp ssh --host completes, but host definitions can not be completed, nor anything after.

@jonathanbardo
Copy link
Contributor

The nature of the plugin makes it difficult to have bash completion since we are proxying the request to somewhere else. Would be a good improvement though 😉

@westonruter
Copy link
Contributor

Yeah, Bash completion would really only be feasible if we were able to figure out #15, caching an SSH connection to speed up calls.

@aj-adl
Copy link

aj-adl commented Apr 12, 2016

I'd just like to raise that this issue is still a problem, doing some digging in xDebug, it originally stems from the way you are accessing arguments at the beginning of __invoke.

The loop of

 foreach ( array_slice( $GLOBALS['argv'], 2 ) as  $arg ) {}

will result in an array like

$cli_args = [ 
    0 => 'search-replace foo bar --skip-tables'
];

where as the desired outcome would be

$cli_args = [ 
    0 => 'search-replace', 
    1 => 'foo',
    2 => 'bar',
    3 => '--skip-tables'
];

While the commands do get concatenated back together as a string later, this incorrect grouping means that the call to escapeshellarg will wrap the whole command in quotes, so WP-CLI on the remote server will try and execute a command called wp 'search-replace foo bar --skip-tables'

I'm evaluating a few different options to fix, I'll submit a PR for review when done. Originally I tried just exploding the string on spaces, and it worked for my particular use case - but there will be some commands that will have 'legitimate' spaces inside quotes so a regexp solution is looking likely.

My first attempt worked but has one too many layers of escapeshellarg called on the single quotes so you end up with values like plugin name: 'my plugin name'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants