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

Fix: include sub-actions in tab completion #13140

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

matthewhughes934
Copy link
Contributor

For commands like pip cache with sub-actions like remove, so that e.g. pip cache re<TAB> completes to pip cache remove.

All the existing commands that used such sub-actions followed the same approach for: using a dictionary of names to methods to run, so the implementation is just teaching the Command object about this mapping and using it in the autocompletion function.

There's no handling for the position of the argument, so e.g. pip cache re<TAB> and pip cache --user re<TAB> will both complete the final word to remove. This is mostly because it was simpler to implement like this, but also I think due to how optparse works such invocations are valid, e.g. pip config --user set global.timeout 60. Similarly, there's no duplication handling so pip cache remove re<TAB> will also complete.

This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]).

I also took the opportunity to tighten some typing: dropping some use of Any

Link: #4659 [1]
Fixes: #13133

For commands like `pip cache` with sub-actions like `remove`, so that
e.g. `pip cache re<TAB>` completes to `pip cache remove`.

All the existing commands that used such sub-actions followed the same
approach for: using a dictionary of names to methods to run, so the
implementation is just  teaching the `Command` object about this mapping
and using it in the autocompletion function.

There's no handling for the position of the argument, so e.g. `pip cache
re<TAB>` and `pip  cache --user re<TAB>` will both complete the final
word to `remove`. This is mostly because it was simpler to implement like
this, but also I think due to how `optparse` works such invocations are
valid, e.g. `pip config --user set global.timeout 60`. Similarly,
there's no duplication handling so `pip cache remove re<TAB>` will also
complete.

This is a feature that may be simpler to implement, or just work out of
the box, with some argument parsing libraries, but moving to another
such library looks to be quite a bit of work (see discussion[1]).

I also took the opportunity to tighten some typing: dropping some use of
`Any`

Link: pypa#4659 [1]
Fixes: pypa#13133
Comment on lines +429 to +440
("cache", "d", "dir"),
("cache", "in", "info"),
("cache", "l", "list"),
("cache", "re", "remove"),
("cache", "pu", "purge"),
("config", "li", "list"),
("config", "e", "edit"),
("config", "ge", "get"),
("config", "se", "set"),
("config", "unse", "unset"),
("config", "d", "debug"),
("index", "ve", "versions"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this list could probably be auto-generated from all commands with a non-empty handler_map but I just hard coded it for simplicity (read: laziness 🙃)

@matthewhughes934 matthewhughes934 marked this pull request as ready for review January 4, 2025 11:26
sbidoul
sbidoul previously approved these changes Jan 4, 2025
@sbidoul sbidoul dismissed their stale review January 4, 2025 18:47

Needs more thought.

@sbidoul
Copy link
Member

sbidoul commented Jan 4, 2025

There's no handling for the position of the argument, so e.g. pip cache re<TAB> and pip cache --user re<TAB> will both complete the final word to remove. This is mostly because it was simpler to implement like this, but also I think due to how optparse works such invocations are valid, e.g. pip config --user set global.timeout 60. Similarly, there's no duplication handling so pip cache remove re<TAB> will also complete.

Do such limitation apply to the current completions for the main commands ? At first sight it seems they don't (at least pip cache ca<TAB> does not complete). So I'm not sure if possibly erroneous completions are better than no completion at all.

@matthewhughes934
Copy link
Contributor Author

There's no handling for the position of the argument, so e.g. pip cache re<TAB> and pip cache --user re<TAB> will both complete the final word to remove. This is mostly because it was simpler to implement like this, but also I think due to how optparse works such invocations are valid, e.g. pip config --user set global.timeout 60. Similarly, there's no duplication handling so pip cache remove re<TAB> will also complete.

Do such limitation apply to the current completions for the main commands ? At first sight it seems they don't (at least pip cache ca<TAB> does not complete). So I'm not sure if possibly erroneous completions are better than no completion at all.

No, those limitations don't apply: for the sub-commands the completion behaviour is split:

  • If a sub-command is present in any of the current words, present the completion for that sub-command, else
  • Present all sub-commands as completion options

So the only time we present sub-commands as completion options is when we haven't already seen any sub-commands

The relevant code starts at

subcommand_name: Optional[str] = None

If we want to avoid the behaviour you mentioned in this change I think we can do something like:

        handler_names = subcommand.handler_map().keys()
        # present handler names as completion only if we haven't already seen any handler names
        if not any(name in cwords for name in handler_names):
            for handler_name in handler_names:
                if handler_name.startswith(current):
                    print(handler_name)

This comes with two edge cases I can think of:

  • If you pass the name of a handler to an option, e.g. pip cache --log remove re<tab> (i.e. log the output to the file named remove ... for some reason) won't complete correctly. Off the top of my head I can only think of contrived examples to produce that behaviour
  • It might not play nice if one handler's name was a substring of another, e.g. if there was pip cache list and pip cache lists, but currently there's no such conflicts (so this is only a concern for future extensions)

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.

Tab completion for pip config suggests invalid options.
2 participants