Skip to content

Add missing required arguments when using --prompt #5865

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

Merged

Conversation

2ndkauboy
Copy link
Contributor

@2ndkauboy 2ndkauboy commented Nov 10, 2023

When using the --prompt argument on a command, the executed command gets logged, showing the command and arguments the prompt had collected.

This logged command, however, lacks all required $args and only lists the $assoc_args (that are not empty).

This patch will add the required arguments as well.

Fixes #5768

Related: #5859

@2ndkauboy 2ndkauboy requested a review from a team as a code owner November 10, 2023 11:19
@2ndkauboy 2ndkauboy force-pushed the fix/5768-prompt-missing-required-parameters branch 4 times, most recently from 7d10ddc to 5645c05 Compare November 10, 2023 12:14
@@ -480,7 +480,7 @@ public function invoke( $args, $assoc_args, $extra_args ) {
sprintf(
'wp %s %s',
$cmd,
ltrim( Utils\assoc_args_to_str( $actual_args ), ' ' )
ltrim( implode( ' ', [ Utils\args_to_str( $args ), Utils\assoc_args_to_str( $actual_args ) ] ), ' ' )
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ltrim( implode( ' ', [ Utils\args_to_str( $args ), Utils\assoc_args_to_str( $actual_args ) ] ), ' ' )
ltrim( implode( '', [ Utils\args_to_str( $args ), Utils\assoc_args_to_str( $actual_args ) ] ), ' ' )

This might fix the double space issue. assoc_args_to_str() will have a leading space, so no need to add another.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I came up with a solution - forgot to push it before lunch 🙈 - I hope this works now and passes all the tests.

@2ndkauboy 2ndkauboy force-pushed the fix/5768-prompt-missing-required-parameters branch from 5645c05 to c42c372 Compare November 10, 2023 13:06
@danielbachhuber
Copy link
Member

@2ndkauboy Looks like it needs a PHPCS fix

@2ndkauboy 2ndkauboy force-pushed the fix/5768-prompt-missing-required-parameters branch from c42c372 to b1f71e0 Compare November 10, 2023 13:33
@2ndkauboy
Copy link
Contributor Author

@danielbachhuber you are right. It's fixed now.

When using the `--prompt` argument on a command, the executed command
gets logged, showing the command and arguments the prompt had collected.

This logged command, however, lacks all required `$args` and only lists
the `$assoc_args` (that are not empty).

This patch will add the required arguments as well.
@2ndkauboy 2ndkauboy force-pushed the fix/5768-prompt-missing-required-parameters branch from b1f71e0 to 438cdbc Compare November 10, 2023 13:35
@swissspidy swissspidy merged commit cd531b0 into wp-cli:main Nov 10, 2023
@2ndkauboy 2ndkauboy deleted the fix/5768-prompt-missing-required-parameters branch November 10, 2023 14:03
@danielbachhuber danielbachhuber added this to the 2.10.0 milestone Nov 10, 2023
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.

--prompt output does not include missing positional parameters
3 participants