Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Standard completion of current option#5873

Merged
schlessera merged 1 commit into
wp-cli:mainwp-cli/wp-cli:mainfrom
Roy-Orbison:patch-1Copy head branch name to clipboard
Feb 5, 2024
Merged

Standard completion of current option#5873
schlessera merged 1 commit into
wp-cli:mainwp-cli/wp-cli:mainfrom
Roy-Orbison:patch-1Copy head branch name to clipboard

Conversation

@Roy-Orbison

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@schlessera schlessera added this to the 2.10.0 milestone Feb 5, 2024
@schlessera schlessera added the command:cli-completions Related to 'cli completions' command label Feb 5, 2024
@schlessera

Copy link
Copy Markdown
Member

Thanks, @Roy-Orbison !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:cli-completions Related to 'cli completions' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected tab completion behaviour with prefixes

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.