Fix shell completion for --option=value format#3237
Fix shell completion for --option=value format#3237Simon99 wants to merge 3 commits intopallets:stablefrom
--option=value format#3237Conversation
Fixes pallets#2847 When using `--option=value` style completion in bash/zsh, the shell passes `--option=partial` as the incomplete value. After resolving completions, the code was returning plain values (e.g., `auto`) without the option prefix. This caused the shell to replace `--option=` with just `auto`, resulting in `command auto` instead of `command --option=auto`. The fix detects when the incomplete value is in `--option=...` format and prepends the `--option=` prefix to each completion value.
|
I like the extensive |
--option=value format
| @pytest.mark.parametrize( | ||
| ("shell", "env", "expect"), | ||
| [ | ||
| # Test --option= format (issue #2847) |
There was a problem hiding this comment.
Don't mention the issue number, all information should be in the comment if it's needed at all.
| {"COMP_WORDS": "cli --color=r", "COMP_CWORD": "1"}, | ||
| "plain,--color=red\n", | ||
| ), | ||
| # Space format should still work without prefix |
There was a problem hiding this comment.
This test is redundant. It's the default behavior already tested by other cases.
| {"COMP_WORDS": "cli --color ", "COMP_CWORD": "2"}, | ||
| "plain,red\nplain,green\nplain,blue\n", | ||
| ), | ||
| # Zsh tests |
There was a problem hiding this comment.
The fix doesn't differ based on shell type so this test is unnecessary.
| def test_option_equals_completion(runner, shell, env, expect): | ||
| """Test that --option=value style completion works correctly. | ||
|
|
||
| Fixes https://github.com/pallets/click/issues/2847 |
There was a problem hiding this comment.
Don't include issues all information should be in the comment if needed at all.
| """ | ||
| args, incomplete = self.get_completion_args() | ||
|
|
||
| # Fix #2847: When completing ``--option=value`` style, the shell |
There was a problem hiding this comment.
Don't include the issue all information should be in the comment.
This comment is extremely verbose for what it's actually saying don't use AI to write comments
| option_prefix = "" | ||
| if "=" in incomplete: | ||
| name, _, _ = incomplete.partition("=") | ||
| if name.startswith("-"): |
| if option_prefix: | ||
| for item in completions: | ||
| # Only add prefix to values, not to option names | ||
| if not str(item.value).startswith("-"): |
There was a problem hiding this comment.
Why does the value need to be converted to a string here?
| if "=" in incomplete: | ||
| name, _, _ = incomplete.partition("=") | ||
| if name.startswith("-"): | ||
| option_prefix = name + "=" |
There was a problem hiding this comment.
Use F strings not concatenation.
| if name.startswith("-"): | ||
| option_prefix = name + "=" | ||
|
|
||
| completions = self.get_completions(args, incomplete) |
There was a problem hiding this comment.
How is this working? Shouldn't we need to add the modified option token to the arguments list?
|
@kdeldycke this looks like AI junk to me. We shouldn't merge this. |
Oh yes you are right. I did not evaluate the fix, I was just looking at the tests and compared them to the problem described in #2847. Which looked good enough to me. Should have been more explicit in my previous comment. |
Summary
Fixes #2847
When using
--option=valuestyle completion in bash/zsh, the shell passes--option=partialas the incomplete value. After resolving completions, the code was returning plain values (e.g.,auto) without the option prefix. This caused the shell to replace--option=with justauto, resulting incommand autoinstead ofcommand --option=auto.The Problem
The Fix
In
ShellComplete.complete(), detect when the incomplete value is in--option=...format and prepend the--option=prefix to each completion value.Testing
--option=format--option value(space) format still works correctly