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

Tab completion for pip config suggests invalid options. #13133

Open
1 task done
hongyi-zhao opened this issue Dec 29, 2024 · 3 comments · May be fixed by #13140
Open
1 task done

Tab completion for pip config suggests invalid options. #13133

hongyi-zhao opened this issue Dec 29, 2024 · 3 comments · May be fixed by #13140
Labels
S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior

Comments

@hongyi-zhao
Copy link

hongyi-zhao commented Dec 29, 2024

Description

When using tab completion with pip config -[TAB], it suggests general pip options like --cache-dir, --cert, etc., as shown below:

(miniforge3-24.11.0-1) werner@x13dai-t:~$ pip config -
--cache-dir=                 --global                     --no-cache-dir=              --require-venv               --user
--cert=                      -h                           --no-color                   --require-virtualenv         -v
--client-cert=               --help                       --no-input                   --retries=                   -V
--debug                      --isolated                   --no-python-version-warning  --site                       --verbose
--default-timeout=           --keyring-provider=          --proxy=                     --timeout=                   --version
--disable-pip-version-check  --local-log=                 --python=                    --trusted-host=              
--editor=                    --log=                       -q                           --use-deprecated=            
--exists-action=             --log-file=                  --quiet                      --use-feature=   

However, pip config requires an action (debug, edit, get, list, set, unset) first and these suggestions are not given.

Expected behavior

Tab completion should show valid actions (debug, edit, get, list, set, unset) or their relevant options.

pip version

(miniforge3-24.11.0-1) werner@x13dai-t:~$ pip --version pip 24.3.1 from /home/werner/.pyenv/versions/miniforge3-24.11.0-1/lib/python3.12/site-packages/pip (python 3.12)

Python version

(miniforge3-24.11.0-1) werner@x13dai-t:~$ python --version
Python 3.12.8

OS

(miniforge3-24.11.0-1) werner@x13dai-t:~$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 22.04.4 LTS
Release: 22.04
Codename: jammy

How to Reproduce

See above.

Output

See above.

Code of Conduct

@hongyi-zhao hongyi-zhao added S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior labels Dec 29, 2024
@matthewhughes934
Copy link
Contributor

matthewhughes934 commented Jan 2, 2025

It looks like pip's completion isn't aware of arguments that aren't flags (i.e. not starting with - or --). At a glance this also impacts the cache and index commands.

I do see that each of those commands look to have a fairly common setup: they define a dict of handlers mapping names to methods taking the same argument types, check that args[0] is one of those names (and not a flag like --global). Maybe the common bits (at least the dictionary of names -> functions) could be moved to some base class that the completion command could then access.

EDIT: there's also discussion #4659 so maybe if pip is moving to another library for arg parsing it's easier to add that functionality then, rather than introduce it in the current setup

@ichard26
Copy link
Member

ichard26 commented Jan 3, 2025

there's also discussion #4659 so maybe if pip is moving to another library for arg parsing it's easier to add that functionality then, rather than introduce it in the current setup

Unless you're signing up as the volunteer who will lead a migration to a new argument parsing framework, I can assure you that ain't going to be happening any time soon 🙂

matthewhughes934 added a commit to matthewhughes934/pip that referenced this issue Jan 3, 2025
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
matthewhughes934 added a commit to matthewhughes934/pip that referenced this issue Jan 3, 2025
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
matthewhughes934 added a commit to matthewhughes934/pip that referenced this issue Jan 3, 2025
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
@matthewhughes934 matthewhughes934 linked a pull request Jan 3, 2025 that will close this issue
@matthewhughes934
Copy link
Contributor

Unless you're signing up as the volunteer who will lead a migration to a new argument parsing framework, I can assure you that ain't going to be happening any time soon 🙂

Tempting, though after a quick look I couldn't find a way to do this piece-wise (i.e. without having to rewrite it all at once) so just tried a simple change for this issue instead: #13140

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants