-
-
Notifications
You must be signed in to change notification settings - Fork 34.4k
gh-130472: Use fancycompleter in import completions #148188
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
base: main
Are you sure you want to change the base?
Changes from all commits
89dde8f
2b05c47
8e75eb6
3b20cf3
9e9d9d8
a1576a0
7c95992
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |||||||
| from dataclasses import dataclass | ||||||||
| from itertools import chain | ||||||||
| from tokenize import TokenInfo | ||||||||
| from .fancycompleter import safe_getattr | ||||||||
|
|
||||||||
| TYPE_CHECKING = False | ||||||||
|
|
||||||||
|
|
@@ -71,7 +72,7 @@ def __init__(self, namespace: Mapping[str, Any] | None = None) -> None: | |||||||
| self._curr_sys_path: list[str] = sys.path[:] | ||||||||
| self._stdlib_path = os.path.dirname(importlib.__path__[0]) | ||||||||
|
|
||||||||
| def get_completions(self, line: str) -> tuple[list[str], CompletionAction | None] | None: | ||||||||
| def get_completions(self, line: str) -> tuple[list[str], list[Any], CompletionAction | None] | None: | ||||||||
| """Return the next possible import completions for 'line'. | ||||||||
|
|
||||||||
| For attributes completion, if the module to complete from is not | ||||||||
|
|
@@ -86,26 +87,40 @@ def get_completions(self, line: str) -> tuple[list[str], CompletionAction | None | |||||||
| except Exception: | ||||||||
| # Some unexpected error occurred, make it look like | ||||||||
| # no completions are available | ||||||||
| return [], None | ||||||||
| return [], [], None | ||||||||
|
|
||||||||
| def complete(self, from_name: str | None, name: str | None) -> tuple[list[str], CompletionAction | None]: | ||||||||
| def complete(self, from_name: str | None, name: str | None) -> tuple[list[str], list[Any], CompletionAction | None]: | ||||||||
| if from_name is None: | ||||||||
| # import x.y.z<tab> | ||||||||
| assert name is not None | ||||||||
| path, prefix = self.get_path_and_prefix(name) | ||||||||
| modules = self.find_modules(path, prefix) | ||||||||
| return [self.format_completion(path, module) for module in modules], None | ||||||||
| names = [self.format_completion(path, module) for module in modules] | ||||||||
| # These are always modules, use dummy values to get the right color | ||||||||
| values = [sys] * len(names) | ||||||||
| return names, values, None | ||||||||
|
|
||||||||
| if name is None: | ||||||||
| # from x.y.z<tab> | ||||||||
| path, prefix = self.get_path_and_prefix(from_name) | ||||||||
| modules = self.find_modules(path, prefix) | ||||||||
| return [self.format_completion(path, module) for module in modules], None | ||||||||
| names = [self.format_completion(path, module) for module in modules] | ||||||||
| # These are always modules, use dummy values to get the right color | ||||||||
| values = [sys] * len(names) | ||||||||
| return names, values, None | ||||||||
|
|
||||||||
| # from x.y import z<tab> | ||||||||
| submodules = self.find_modules(from_name, name) | ||||||||
| attributes, action = self.find_attributes(from_name, name) | ||||||||
| return sorted({*submodules, *attributes}), action | ||||||||
| attr_names, attr_values, action = self.find_attributes(from_name, name) | ||||||||
| all_names = sorted({*submodules, *attr_names}) | ||||||||
| # Build values list matching the sorted order: | ||||||||
| # submodules use `sys` as a dummy value so they get the 'module' color, | ||||||||
| # attributes use their actual value. | ||||||||
| submodule_set = set(submodules) | ||||||||
| attr_map = dict(zip(attr_names, attr_values)) | ||||||||
| all_values = [attr_map.get(n) if n not in submodule_set else sys | ||||||||
| for n in all_names] | ||||||||
|
Comment on lines
+121
to
+122
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a name is both a submodule and an attribute, the attribute takes precedence at runtime, so I believe this should be
Suggested change
|
||||||||
| return all_names, all_values, action | ||||||||
|
|
||||||||
| def find_modules(self, path: str, prefix: str) -> list[str]: | ||||||||
| """Find all modules under 'path' that start with 'prefix'.""" | ||||||||
|
|
@@ -166,31 +181,43 @@ def _is_stdlib_module(self, module_info: pkgutil.ModuleInfo) -> bool: | |||||||
| return (isinstance(module_info.module_finder, FileFinder) | ||||||||
| and module_info.module_finder.path == self._stdlib_path) | ||||||||
|
|
||||||||
| def find_attributes(self, path: str, prefix: str) -> tuple[list[str], CompletionAction | None]: | ||||||||
| def find_attributes(self, path: str, prefix: str) -> tuple[list[str], list[Any], CompletionAction | None]: | ||||||||
| """Find all attributes of module 'path' that start with 'prefix'.""" | ||||||||
| attributes, action = self._find_attributes(path, prefix) | ||||||||
| attributes, values, action = self._find_attributes(path, prefix) | ||||||||
| # Filter out invalid attribute names | ||||||||
| # (for example those containing dashes that cannot be imported with 'import') | ||||||||
| return [attr for attr in attributes if attr.isidentifier()], action | ||||||||
|
|
||||||||
| def _find_attributes(self, path: str, prefix: str) -> tuple[list[str], CompletionAction | None]: | ||||||||
| filtered_names = [] | ||||||||
| filtered_values = [] | ||||||||
| for attr, val in zip(attributes, values): | ||||||||
| if attr.isidentifier(): | ||||||||
| filtered_names.append(attr) | ||||||||
| filtered_values.append(val) | ||||||||
| return filtered_names, filtered_values, action | ||||||||
|
|
||||||||
| def _find_attributes(self, path: str, prefix: str) -> tuple[list[str], list[Any], CompletionAction | None]: | ||||||||
| path = self._resolve_relative_path(path) # type: ignore[assignment] | ||||||||
| if path is None: | ||||||||
| return [], None | ||||||||
| return [], [], None | ||||||||
|
|
||||||||
| imported_module = sys.modules.get(path) | ||||||||
| if not imported_module: | ||||||||
| if path in self._failed_imports: # Do not propose to import again | ||||||||
| return [], None | ||||||||
| return [], [], None | ||||||||
| imported_module = self._maybe_import_module(path) | ||||||||
| if not imported_module: | ||||||||
| return [], self._get_import_completion_action(path) | ||||||||
| return [], [], self._get_import_completion_action(path) | ||||||||
| try: | ||||||||
| module_attributes = dir(imported_module) | ||||||||
| except Exception: | ||||||||
| module_attributes = [] | ||||||||
| return [attr_name for attr_name in module_attributes | ||||||||
| if self.is_suggestion_match(attr_name, prefix)], None | ||||||||
| names = [] | ||||||||
| values = [] | ||||||||
| for attr_name in module_attributes: | ||||||||
| if not self.is_suggestion_match(attr_name, prefix): | ||||||||
| continue | ||||||||
| names.append(attr_name) | ||||||||
| values.append(safe_getattr(imported_module, attr_name)) | ||||||||
| return names, values, None | ||||||||
|
|
||||||||
| def is_suggestion_match(self, module_name: str, prefix: str) -> bool: | ||||||||
| if prefix: | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,7 @@ | |
| from .completing_reader import CompletingReader, stripcolor | ||
| from .console import Console as ConsoleType | ||
| from ._module_completer import ModuleCompleter, make_default_module_completer | ||
| from .fancycompleter import Completer as FancyCompleter | ||
| from .fancycompleter import Completer as FancyCompleter, colorize_matches | ||
|
|
||
| Console: type[ConsoleType] | ||
| _error: tuple[type[Exception], ...] | type[Exception] | ||
|
|
@@ -104,6 +104,7 @@ class ReadlineConfig: | |
| readline_completer: Completer | None = None | ||
| completer_delims: frozenset[str] = frozenset(" \t\n`~!@#$%^&*()-=+[{]}\\|;:'\",<>/?") | ||
| module_completer: ModuleCompleter = field(default_factory=make_default_module_completer) | ||
| colorize_completions: Callable[[list[str], list[object]], list[str]] | None = None | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a rationale for using |
||
|
|
||
| @dataclass(kw_only=True) | ||
| class ReadlineAlikeReader(historical_reader.HistoricalReader, CompletingReader): | ||
|
|
@@ -169,8 +170,14 @@ def get_completions(self, stem: str) -> tuple[list[str], CompletionAction | None | |
| return result, None | ||
|
|
||
| def get_module_completions(self) -> tuple[list[str], CompletionAction | None] | None: | ||
| line = self.get_line() | ||
| return self.config.module_completer.get_completions(line) | ||
| line = stripcolor(self.get_line()) | ||
| result = self.config.module_completer.get_completions(line) | ||
| if result is None: | ||
| return None | ||
| names, values, action = result | ||
| if self.config.colorize_completions: | ||
| names = self.config.colorize_completions(names, values) | ||
| return names, action | ||
|
|
||
| def get_trimmed_history(self, maxlength: int) -> list[str]: | ||
| if maxlength >= 0: | ||
|
|
@@ -609,13 +616,18 @@ def _setup(namespace: Mapping[str, Any]) -> None: | |
| # set up namespace in rlcompleter, which requires it to be a bona fide dict | ||
| if not isinstance(namespace, dict): | ||
| namespace = dict(namespace) | ||
| _wrapper.config.module_completer = ModuleCompleter(namespace) | ||
| use_basic_completer = ( | ||
| not sys.flags.ignore_environment | ||
| and os.getenv("PYTHON_BASIC_COMPLETER") | ||
| ) | ||
| completer_cls = RLCompleter if use_basic_completer else FancyCompleter | ||
| _wrapper.config.readline_completer = completer_cls(namespace).complete | ||
| completer = completer_cls(namespace) | ||
| _wrapper.config.readline_completer = completer.complete | ||
| if getattr(completer, 'use_colors', False): | ||
| def _colorize(names: list[str], values: list[object]) -> list[str]: | ||
| return colorize_matches(names, values, completer.theme) | ||
| _wrapper.config.colorize_completions = _colorize | ||
| _wrapper.config.module_completer = ModuleCompleter(namespace) | ||
|
|
||
| # this is not really what readline.c does. Better than nothing I guess | ||
| import builtins | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,14 +35,15 @@ | |
| multiline_input, | ||
| code_to_events, | ||
| ) | ||
| from _colorize import ANSIColors, get_theme | ||
| from _pyrepl.console import Event | ||
| from _pyrepl.completing_reader import stripcolor | ||
| from _pyrepl._module_completer import ( | ||
| ImportParser, | ||
| ModuleCompleter, | ||
| HARDCODED_SUBMODULES, | ||
| ) | ||
| from _pyrepl.fancycompleter import Completer as FancyCompleter | ||
| from _pyrepl.fancycompleter import Completer as FancyCompleter, colorize_matches | ||
| import _pyrepl.readline as pyrepl_readline | ||
| from _pyrepl.readline import ( | ||
| ReadlineAlikeReader, | ||
|
|
@@ -1629,7 +1630,7 @@ def test_suggestions_and_messages(self) -> None: | |
| result = completer.get_completions(code) | ||
| self.assertEqual(result is None, expected is None) | ||
| if result: | ||
| compl, act = result | ||
| compl, _values, act = result | ||
| self.assertEqual(compl, expected[0]) | ||
| self.assertEqual(act is None, expected[1] is None) | ||
| if act: | ||
|
|
@@ -1641,6 +1642,39 @@ def test_suggestions_and_messages(self) -> None: | |
| new_imports = sys.modules.keys() - _imported | ||
| self.assertSetEqual(new_imports, expected_imports) | ||
|
|
||
| def test_colorize_import_completions(self) -> None: | ||
| theme = get_theme() | ||
| type_color = theme.fancycompleter.type | ||
| module_color = theme.fancycompleter.module | ||
| R = ANSIColors.RESET | ||
|
|
||
| colorize = lambda names, values: colorize_matches(names, values, theme) | ||
| config = ReadlineConfig(colorize_completions=colorize) | ||
| reader = ReadlineAlikeReader( | ||
| console=FakeConsole(events=[]), | ||
| config=config, | ||
| ) | ||
|
|
||
| # "from collections import de" -> defaultdict (type) and deque (type) | ||
| reader.buffer = list("from collections import de") | ||
| reader.pos = len(reader.buffer) | ||
| names, action = reader.get_module_completions() | ||
| self.assertEqual(names, [ | ||
| f"{type_color}defaultdict{R}", | ||
| f"{type_color}deque{R}", | ||
| ]) | ||
| self.assertIsNone(action) | ||
|
|
||
| # "from importlib.m" has submodule completions colored as modules | ||
| reader.buffer = list("from importlib.m") | ||
| reader.pos = len(reader.buffer) | ||
| names, action = reader.get_module_completions() | ||
| self.assertEqual(names, [ | ||
| f"{module_color}importlib.machinery{R}", | ||
| f"{module_color}importlib.metadata{R}", | ||
| ]) | ||
| self.assertIsNone(action) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe a test with a name both an attribute + a submodule with the same name would be great, to be sure everything combines well? |
||
|
|
||
| # Audit hook used to check for stdlib modules import side-effects | ||
| # Defined globally to avoid adding one hook per test run (refleak) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would've find it more natural to return a
dict[str, Any]instead of two parallel lists, but I see that's what fancycompleter does, so let be it!But I wonder if we haven't passed the point where a namedtuple would be more readeable / maintainable? I hesitated when adding completion action, something like
the docstring could use an update either way 😄