From 02e0fc222d759368080291d100bb5f4c6287f0a9 Mon Sep 17 00:00:00 2001 From: Matthew Hughes Date: Fri, 3 Jan 2025 14:32:44 +0000 Subject: [PATCH 1/2] Fix: include sub-actions in tab completion For commands like `pip cache` with sub-actions like `remove`, so that e.g. `pip cache re` 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` and `pip cache --user re` 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` 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: https://github.com/pypa/pip/issues/4659 [1] Fixes: https://github.com/pypa/pip/issues/13133 --- ...ad6-3007-47f8-9c53-984e9116c7ff.bugfix.rst | 1 + src/pip/_internal/cli/autocompletion.py | 4 +++ src/pip/_internal/cli/base_command.py | 8 +++++- src/pip/_internal/commands/cache.py | 25 +++++++++++-------- src/pip/_internal/commands/configuration.py | 15 ++++++----- src/pip/_internal/commands/index.py | 15 ++++++----- tests/functional/test_completion.py | 25 +++++++++++++++++++ 7 files changed, 69 insertions(+), 24 deletions(-) create mode 100644 news/0741cad6-3007-47f8-9c53-984e9116c7ff.bugfix.rst diff --git a/news/0741cad6-3007-47f8-9c53-984e9116c7ff.bugfix.rst b/news/0741cad6-3007-47f8-9c53-984e9116c7ff.bugfix.rst new file mode 100644 index 00000000000..d292fe37260 --- /dev/null +++ b/news/0741cad6-3007-47f8-9c53-984e9116c7ff.bugfix.rst @@ -0,0 +1 @@ +Fix: include sub-actions in tab completion diff --git a/src/pip/_internal/cli/autocompletion.py b/src/pip/_internal/cli/autocompletion.py index f3f70ac8553..9a2645bc7bc 100644 --- a/src/pip/_internal/cli/autocompletion.py +++ b/src/pip/_internal/cli/autocompletion.py @@ -101,6 +101,10 @@ def autocomplete() -> None: if option[1] and option[0][:2] == "--": opt_label += "=" print(opt_label) + + for handler_name in subcommand.handler_map(): + if handler_name.startswith(current): + print(handler_name) else: # show main parser options only when necessary diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index bc1ab65949d..6d2e123c7af 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -7,7 +7,7 @@ import sys import traceback from optparse import Values -from typing import List, Optional, Tuple +from typing import Callable, List, Optional, Tuple from pip._vendor.rich import reconfigure from pip._vendor.rich import traceback as rich_traceback @@ -229,3 +229,9 @@ def _main(self, args: List[str]) -> int: options.cache_dir = None return self._run_wrapper(level_number, options, args) + + def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + """ + map of names to handler actions for commands with sub-actions + """ + return {} diff --git a/src/pip/_internal/commands/cache.py b/src/pip/_internal/commands/cache.py index ad65641edb2..293cf1343b2 100644 --- a/src/pip/_internal/commands/cache.py +++ b/src/pip/_internal/commands/cache.py @@ -1,7 +1,7 @@ import os import textwrap from optparse import Values -from typing import Any, List +from typing import Callable, List from pip._internal.cli.base_command import Command from pip._internal.cli.status_codes import ERROR, SUCCESS @@ -49,8 +49,8 @@ def add_options(self) -> None: self.parser.insert_option_group(0, self.cmd_opts) - def run(self, options: Values, args: List[str]) -> int: - handlers = { + def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + return { "dir": self.get_cache_dir, "info": self.get_cache_info, "list": self.list_cache_items, @@ -58,15 +58,18 @@ def run(self, options: Values, args: List[str]) -> int: "purge": self.purge_cache, } + def run(self, options: Values, args: List[str]) -> int: + handler_map = self.handler_map() + if not options.cache_dir: logger.error("pip cache commands can not function since cache is disabled.") return ERROR # Determine action - if not args or args[0] not in handlers: + if not args or args[0] not in handler_map: logger.error( "Need an action (%s) to perform.", - ", ".join(sorted(handlers)), + ", ".join(sorted(handler_map)), ) return ERROR @@ -74,20 +77,20 @@ def run(self, options: Values, args: List[str]) -> int: # Error handling happens here, not in the action-handlers. try: - handlers[action](options, args[1:]) + handler_map[action](options, args[1:]) except PipError as e: logger.error(e.args[0]) return ERROR return SUCCESS - def get_cache_dir(self, options: Values, args: List[Any]) -> None: + def get_cache_dir(self, options: Values, args: List[str]) -> None: if args: raise CommandError("Too many arguments") logger.info(options.cache_dir) - def get_cache_info(self, options: Values, args: List[Any]) -> None: + def get_cache_info(self, options: Values, args: List[str]) -> None: if args: raise CommandError("Too many arguments") @@ -129,7 +132,7 @@ def get_cache_info(self, options: Values, args: List[Any]) -> None: logger.info(message) - def list_cache_items(self, options: Values, args: List[Any]) -> None: + def list_cache_items(self, options: Values, args: List[str]) -> None: if len(args) > 1: raise CommandError("Too many arguments") @@ -161,7 +164,7 @@ def format_for_abspath(self, files: List[str]) -> None: if files: logger.info("\n".join(sorted(files))) - def remove_cache_items(self, options: Values, args: List[Any]) -> None: + def remove_cache_items(self, options: Values, args: List[str]) -> None: if len(args) > 1: raise CommandError("Too many arguments") @@ -188,7 +191,7 @@ def remove_cache_items(self, options: Values, args: List[Any]) -> None: logger.verbose("Removed %s", filename) logger.info("Files removed: %s (%s)", len(files), format_size(bytes_removed)) - def purge_cache(self, options: Values, args: List[Any]) -> None: + def purge_cache(self, options: Values, args: List[str]) -> None: if args: raise CommandError("Too many arguments") diff --git a/src/pip/_internal/commands/configuration.py b/src/pip/_internal/commands/configuration.py index 1a1dc6b6cd8..56754c5b0d7 100644 --- a/src/pip/_internal/commands/configuration.py +++ b/src/pip/_internal/commands/configuration.py @@ -2,7 +2,7 @@ import os import subprocess from optparse import Values -from typing import Any, List, Optional +from typing import Any, Callable, List, Optional from pip._internal.cli.base_command import Command from pip._internal.cli.status_codes import ERROR, SUCCESS @@ -93,8 +93,8 @@ def add_options(self) -> None: self.parser.insert_option_group(0, self.cmd_opts) - def run(self, options: Values, args: List[str]) -> int: - handlers = { + def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + return { "list": self.list_values, "edit": self.open_in_editor, "get": self.get_name, @@ -103,11 +103,14 @@ def run(self, options: Values, args: List[str]) -> int: "debug": self.list_config_values, } + def run(self, options: Values, args: List[str]) -> int: + handler_map = self.handler_map() + # Determine action - if not args or args[0] not in handlers: + if not args or args[0] not in handler_map: logger.error( "Need an action (%s) to perform.", - ", ".join(sorted(handlers)), + ", ".join(sorted(handler_map)), ) return ERROR @@ -131,7 +134,7 @@ def run(self, options: Values, args: List[str]) -> int: # Error handling happens here, not in the action-handlers. try: - handlers[action](options, args[1:]) + handler_map[action](options, args[1:]) except PipError as e: logger.error(e.args[0]) return ERROR diff --git a/src/pip/_internal/commands/index.py b/src/pip/_internal/commands/index.py index 2e2661bba71..622dc1d8dc5 100644 --- a/src/pip/_internal/commands/index.py +++ b/src/pip/_internal/commands/index.py @@ -1,6 +1,6 @@ import logging from optparse import Values -from typing import Any, Iterable, List, Optional +from typing import Any, Callable, Iterable, List, Optional from pip._vendor.packaging.version import Version @@ -45,11 +45,14 @@ def add_options(self) -> None: self.parser.insert_option_group(0, index_opts) self.parser.insert_option_group(0, self.cmd_opts) - def run(self, options: Values, args: List[str]) -> int: - handlers = { + def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + return { "versions": self.get_available_package_versions, } + def run(self, options: Values, args: List[str]) -> int: + handler_map = self.handler_map() + logger.warning( "pip index is currently an experimental command. " "It may be removed/changed in a future release " @@ -57,10 +60,10 @@ def run(self, options: Values, args: List[str]) -> int: ) # Determine action - if not args or args[0] not in handlers: + if not args or args[0] not in handler_map: logger.error( "Need an action (%s) to perform.", - ", ".join(sorted(handlers)), + ", ".join(sorted(handler_map)), ) return ERROR @@ -68,7 +71,7 @@ def run(self, options: Values, args: List[str]) -> int: # Error handling happens here, not in the action-handlers. try: - handlers[action](options, args[1:]) + handler_map[action](options, args[1:]) except PipError as e: logger.error(e.args[0]) return ERROR diff --git a/tests/functional/test_completion.py b/tests/functional/test_completion.py index a52b135c8b0..549da6ece3c 100644 --- a/tests/functional/test_completion.py +++ b/tests/functional/test_completion.py @@ -421,3 +421,28 @@ def test_completion_uses_same_executable_name( expect_stderr=deprecated_python, ) assert executable_name in result.stdout + + +@pytest.mark.parametrize( + "subcommand, handler_prefix, expected", + [ + ("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"), + ], +) +def test_completion_for_action_handler( + subcommand: str, handler_prefix: str, expected: str, autocomplete: DoAutocomplete +) -> None: + res, _ = autocomplete(f"pip {subcommand} {handler_prefix}", cword="2") + + assert [expected] == res.stdout.split() From 78f19ece03bdd5220a59c44f2be2ace823d22cf5 Mon Sep 17 00:00:00 2001 From: Matthew Hughes Date: Fri, 3 Jan 2025 15:58:31 +0000 Subject: [PATCH 2/2] fixup! Fix: include sub-actions in tab completion --- src/pip/_internal/cli/base_command.py | 4 ++-- src/pip/_internal/commands/cache.py | 4 ++-- src/pip/_internal/commands/configuration.py | 4 ++-- src/pip/_internal/commands/index.py | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index 6d2e123c7af..ddc35218743 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -7,7 +7,7 @@ import sys import traceback from optparse import Values -from typing import Callable, List, Optional, Tuple +from typing import Callable, Dict, List, Optional, Tuple from pip._vendor.rich import reconfigure from pip._vendor.rich import traceback as rich_traceback @@ -230,7 +230,7 @@ def _main(self, args: List[str]) -> int: return self._run_wrapper(level_number, options, args) - def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + def handler_map(self) -> Dict[str, Callable[[Values, List[str]], None]]: """ map of names to handler actions for commands with sub-actions """ diff --git a/src/pip/_internal/commands/cache.py b/src/pip/_internal/commands/cache.py index 293cf1343b2..df9daa3368a 100644 --- a/src/pip/_internal/commands/cache.py +++ b/src/pip/_internal/commands/cache.py @@ -1,7 +1,7 @@ import os import textwrap from optparse import Values -from typing import Callable, List +from typing import Callable, Dict, List from pip._internal.cli.base_command import Command from pip._internal.cli.status_codes import ERROR, SUCCESS @@ -49,7 +49,7 @@ def add_options(self) -> None: self.parser.insert_option_group(0, self.cmd_opts) - def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + def handler_map(self) -> Dict[str, Callable[[Values, List[str]], None]]: return { "dir": self.get_cache_dir, "info": self.get_cache_info, diff --git a/src/pip/_internal/commands/configuration.py b/src/pip/_internal/commands/configuration.py index 56754c5b0d7..00c8794bbea 100644 --- a/src/pip/_internal/commands/configuration.py +++ b/src/pip/_internal/commands/configuration.py @@ -2,7 +2,7 @@ import os import subprocess from optparse import Values -from typing import Any, Callable, List, Optional +from typing import Any, Callable, Dict, List, Optional from pip._internal.cli.base_command import Command from pip._internal.cli.status_codes import ERROR, SUCCESS @@ -93,7 +93,7 @@ def add_options(self) -> None: self.parser.insert_option_group(0, self.cmd_opts) - def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + def handler_map(self) -> Dict[str, Callable[[Values, List[str]], None]]: return { "list": self.list_values, "edit": self.open_in_editor, diff --git a/src/pip/_internal/commands/index.py b/src/pip/_internal/commands/index.py index 622dc1d8dc5..1bda326189e 100644 --- a/src/pip/_internal/commands/index.py +++ b/src/pip/_internal/commands/index.py @@ -1,6 +1,6 @@ import logging from optparse import Values -from typing import Any, Callable, Iterable, List, Optional +from typing import Any, Callable, Dict, Iterable, List, Optional from pip._vendor.packaging.version import Version @@ -45,7 +45,7 @@ def add_options(self) -> None: self.parser.insert_option_group(0, index_opts) self.parser.insert_option_group(0, self.cmd_opts) - def handler_map(self) -> dict[str, Callable[[Values, list[str]], None]]: + def handler_map(self) -> Dict[str, Callable[[Values, List[str]], None]]: return { "versions": self.get_available_package_versions, }