Skip to content

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

Merged
danielbachhuber merged 8 commits into
wp-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 into
wp-cli:mainfrom
wojtekn:fix/empty-path-handling

Conversation

@wojtekn

@wojtekn wojtekn commented Dec 5, 2022

Copy link
Copy Markdown
Contributor

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
Comment thread php/utils.php

@danielbachhuber danielbachhuber left a comment

Copy link
Copy Markdown
Member

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

wojtekn commented Dec 5, 2022

Copy link
Copy Markdown
Contributor Author

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

Comment thread php/WP_CLI/Runner.php Outdated
Comment thread php/utils.php
Comment thread php/WP_CLI/Runner.php Outdated
Comment thread php/WP_CLI/Runner.php Outdated
Comment thread php/WP_CLI/Runner.php Outdated
@danielbachhuber danielbachhuber added this to the 2.8.0 milestone Dec 7, 2022
@danielbachhuber
danielbachhuber self-requested a review December 7, 2022 23:56

@danielbachhuber danielbachhuber left a comment

Copy link
Copy Markdown
Member

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

2 participants