Skip to content

Improve support for cases with empty --path provided to wp-cli commands#5709

Merged
danielbachhuber merged 8 commits intowp-cli:mainfrom
wojtekn:fix/empty-path-handling
Dec 7, 2022
Merged

Improve support for cases with empty --path provided to wp-cli commands#5709
danielbachhuber merged 8 commits intowp-cli:mainfrom
wojtekn:fix/empty-path-handling

Conversation

@wojtekn
Copy link
Copy Markdown
Contributor

@wojtekn wojtekn commented Dec 5, 2022

I propose fixing the confusing messaging we show if an empty --path parameter is provided to command calls.

The empty parameter is considered a true flag, let's treat it as empty path:

% bin/wp import --path
Error: The --path parameter cannot be empty when it is provided

This is also an empty parameter considered a true flag because there is no equal sign used to assign a path to the parameter. Let's treat it as an empty path too:

% bin/wp import --path some
Error: The --path parameter cannot be empty when it is provided

Then handle an empty string as an empty path:

% bin/wp import --path=
Error: The --path parameter cannot be empty when it is provided

When a non-empty path is provided, include the full working directory in the error message:

% bin/wp import --path=some
Error: This (/Volumes/Sites/wp-cli/some/) does not seem to be a WordPress installation.
Pass --path=`path/to/wordpress` or run `wp core download`.

Fixes: #5559

@wojtekn wojtekn requested a review from a team as a code owner December 5, 2022 11:03
Copy link
Copy Markdown
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @wojtekn !

Can you write some functional tests for this change?

The handbook is editable if there are improvements you identify.

@wojtekn
Copy link
Copy Markdown
Contributor Author

wojtekn commented Dec 5, 2022

@danielbachhuber that is good idea, I've added functional tests for three cases.

@danielbachhuber danielbachhuber added this to the 2.8.0 milestone Dec 7, 2022
@danielbachhuber danielbachhuber self-requested a review December 7, 2022 23:56
Copy link
Copy Markdown
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@danielbachhuber danielbachhuber merged commit 2634038 into wp-cli:main Dec 7, 2022
@wojtekn wojtekn deleted the fix/empty-path-handling branch December 8, 2022 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Include path in "This does not seem to be a WordPress installation" error

2 participants