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

Make shell completion prioritize option values over new options #2041

Merged
merged 1 commit into from Mar 28, 2022

Conversation

spanglerco
Copy link
Contributor

Swap the order of checks in _resolve_incomplete so that we first check to see if we're completing an option value before detecting if we're completing a new option name. Allows option values that start with non-alphanumeric characters to complete rather than treating them like new options.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism
Copy link
Member

davidism commented Aug 9, 2021

How does this affect options without values?

@spanglerco
Copy link
Contributor Author

How does this affect options without values?

Flag options are unaffected (_is_incomplete_option returns False early if is_flag).

Count options do end up unintentionally taking over the completion for the next arg. That could be easily fixed by extending the previously mentioned is_flag check to if param.is_flag or param.count.

Are there other kinds of options without values? Those are the only two cases I've seen in the documentation.

@spanglerco
Copy link
Contributor Author

spanglerco commented Aug 9, 2021

Ah, I guess there's also options with optional values. This change also makes them claim the next arg for completion. If I understand correctly, the parser collects all of the possible prefixes from each option and then decides whether an arg is an optional value vs. another option based on those prefixes.

Should the shell_completions do the same, using the collected prefixes from the parser?

@davidism davidism added this to the 8.1.0 milestone Aug 14, 2021
@spanglerco
Copy link
Contributor Author

@davidism I pushed a rewrite of the fix to instead grab the opt_prefixes from the parser and store them into the context. Then the shell completion can use that information to more closely match what the parser will do.

How does this affect options without values?

The new implementation should treat them the same as the parser: if the incomplete string starts with a symbol used by any of the options, it will complete as a new option. Otherwise, it will complete as the value of the current option. See the added tests in test_shell_completion.py.

@spanglerco
Copy link
Contributor Author

Hi @davidism is there any interest in this PR or anything else I could help answer about it?

@spanglerco spanglerco force-pushed the shell-completion-option-values branch from 9944ebf to d93b74c Compare January 24, 2022 15:13
@davidism davidism changed the base branch from 8.0.x to main March 17, 2022 22:19
@davidism davidism force-pushed the shell-completion-option-values branch from d93b74c to 4b03a6b Compare March 17, 2022 22:19
Allow option values that start with an option prefix to
complete rather than treating them like new options.

Don't treat count options as needing a value to complete.
@davidism davidism force-pushed the shell-completion-option-values branch from 4b03a6b to f2e579a Compare March 28, 2022 15:05
@davidism davidism merged commit ef11be6 into pallets:main Mar 28, 2022
@spanglerco spanglerco deleted the shell-completion-option-values branch March 28, 2022 15:24
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shell completion fails for option values that start with symbols
2 participants