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

Standard completion of current option #5873

Merged
merged 1 commit into from Feb 5, 2024
Merged

Conversation

Roy-Orbison
Copy link
Contributor

Make options complete in a manner more consistent with other shell programs.

Closes #5845.

This relies on array_key_last() which may need to be polyfilled if it isn't already.

@Roy-Orbison
Copy link
Contributor Author

@swissspidy What tests would you need for this change? You can verify it with the following commands:

(COMP_LINE='wp search-replace --report'; wp cli completions --line="$COMP_LINE" --point=${#COMP_LINE})

produces

--report 
--report-changed-only

preserving the current word. Repeating the word, e.g.

(COMP_LINE='wp search-replace --report --report'; wp cli completions --line="$COMP_LINE" --point=${#COMP_LINE})

produces only

--report-changed-only

which demonstrates that this change preserves the existing "don't complete options that have already been typed" behaviour. The $assoc_args could be named bit better if this is its only purpose.

@Roy-Orbison
Copy link
Contributor Author

array_key_last() appears to be polyfilled by symfony/polyfill-php73 and used in symfony/console on the dev repo, but I don't see it in the stable phar. Would you prefer another dependency, or a single line of more verbose code?

@swissspidy
Copy link
Member

We do have some tests in cli-bash-completion.feature that could be amended for this change, and a couple are already failing right now. I'm not really familiar with the completion script to provide exact guidance, I'll defer that to the others.

As for array_key_last, can this use something like key( end( $words ) ) instead? Then we wouldn't have to fiddle with the dependencies

@Roy-Orbison
Copy link
Contributor Author

I updated the tests to accommodate the change to the repeated option behaviour, but I don't think there are any commands in this package with an option that's a prefix of another. A command package would have to be included during testing to perform that check.

Make options complete in a manner more consistent with other shell programs.

Closes #5845.
@swissspidy swissspidy requested a review from a team November 20, 2023 08:08
@Roy-Orbison Roy-Orbison marked this pull request as ready for review November 27, 2023 03:28
@schlessera schlessera merged commit f2b0446 into wp-cli:main Feb 5, 2024
65 checks passed
@schlessera schlessera added this to the 2.10.0 milestone Feb 5, 2024
@schlessera
Copy link
Member

Thanks, @Roy-Orbison !

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.

Unexpected tab completion behaviour with prefixes
3 participants