Skip to content

Conversation

wojtekn
Copy link
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
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
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
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