Skip to content

gh-69605: PyREPL: insert "import" after "from foo <tab>" #148445

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.

Sign up for GitHub

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

Open
loic-simon wants to merge 1 commit into
base: main
Choose a base branch
from
+92 −72
Open

gh-69605: PyREPL: insert "import" after "from foo <tab>" #148445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 36 additions & 22 deletions Lib/_pyrepl/_module_completer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def get_completions(self, line: str) -> tuple[list[str], CompletionAction | None
# no completions are available
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, space_end: bool) -> tuple[list[str], CompletionAction | None]:
if from_name is None:
# import x.y.z<tab>
assert name is not None
Expand All @@ -97,10 +97,17 @@ def complete(self, from_name: str | None, name: str | None) -> tuple[list[str],
return [self.format_completion(path, module) for module in modules], None

if name is None:
if space_end and from_name:
# from x.y.z <tab>
return (["import "], 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]
if len(names) == 1:
# One match: insert a space to allow for "import" suggestion
names[0] = f"{names[0]} "
return names, None

# from x.y import z<tab>
submodules = self.find_modules(from_name, name)
Expand Down Expand Up @@ -307,11 +314,12 @@ class ImportParser:
suitable for autocomplete suggestions.

Examples:
- import foo -> Result(from_name=None, name='foo')
- import foo. -> Result(from_name=None, name='foo.')
- from foo -> Result(from_name='foo', name=None)
- from foo import bar -> Result(from_name='foo', name='bar')
- from .foo import ( -> Result(from_name='.foo', name='')
- `import foo` -> Result(from_name=None, name='foo')
- `import foo.` -> Result(from_name=None, name='foo.')
- `from foo` -> Result(from_name='foo', name=None)
- `from foo ` -> Result(from_name='foo', name=None, end_space=True)
Copy link
Copy Markdown
Member

@tomasr8 tomasr8 Apr 12, 2026

Choose a reason for hiding this comment

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

Should be space_end?

- `from foo import bar` -> Result(from_name='foo', name='bar')
- `from .foo import (` -> Result(from_name='.foo', name='')

Note that the parser works in reverse order, starting from the
last token in the input string. This makes the parser more robust
Expand Down Expand Up @@ -341,10 +349,10 @@ def __init__(self, code: str) -> None:
tokens = []
self.tokens = TokenQueue(tokens[::-1])

def parse(self) -> tuple[str | None, str | None] | None:
def parse(self) -> tuple[str | None, str | None, bool] | None:
if not (res := self._parse()):
return None
return res.from_name, res.name
return res.from_name, res.name, res.space_end

def _parse(self) -> Result | None:
with self.tokens.save_state():
Expand All @@ -354,7 +362,7 @@ def _parse(self) -> Result | None:

def parse_import(self) -> Result:
if self.code.rstrip().endswith('import') and self.code.endswith(' '):
return Result(name='')
return Result(name='', space_end=True)
if self.tokens.peek_string(','):
name = ''
else:
Expand All @@ -367,27 +375,32 @@ def parse_import(self) -> Result:
self.tokens.pop()
self.parse_dotted_as_name()
if self.tokens.peek_string('import'):
return Result(name=name)
return Result(name=name, space_end=self.code.endswith(' '))
raise ParseError('parse_import')

def parse_from_import(self) -> Result:
space_end = self.code.endswith(' ')
stripped = self.code.rstrip()
if stripped.endswith('import') and self.code.endswith(' '):
return Result(from_name=self.parse_empty_from_import(), name='')
if stripped.endswith('from') and self.code.endswith(' '):
return Result(from_name='')
if stripped.endswith('import') and space_end:
from_name = self.parse_empty_from_import()
return Result(from_name=from_name, name='', space_end=space_end)
if stripped.endswith('from') and space_end:
return Result(from_name='', space_end=space_end)
if self.tokens.peek_string('(') or self.tokens.peek_string(','):
return Result(from_name=self.parse_empty_from_import(), name='')
if self.code.endswith(' '):
raise ParseError('parse_from_import')
from_name = self.parse_empty_from_import()
return Result(from_name=from_name, name='', space_end=space_end)
name = self.parse_dotted_name()
if '.' in name:
if name.endswith(".") and space_end:
raise ParseError('parse_from_import')
self.tokens.pop_string('from')
return Result(from_name=name)
return Result(from_name=name, space_end=space_end)
if self.tokens.peek_string('from'):
return Result(from_name=name)
return Result(from_name=name, space_end=space_end)
if space_end:
raise ParseError('parse_from_import')
from_name = self.parse_empty_from_import()
return Result(from_name=from_name, name=name)
return Result(from_name=from_name, name=name, space_end=space_end)

def parse_empty_from_import(self) -> str:
if self.tokens.peek_string(','):
Expand Down Expand Up @@ -453,10 +466,11 @@ class ParseError(Exception):
pass


@dataclass(frozen=True)
@dataclass(frozen=True, kw_only=True)
class Result:
from_name: str | None = None
name: str | None = None
space_end: bool


class TokenQueue:
Expand Down
104 changes: 54 additions & 50 deletions Lib/test/test_pyrepl/test_pyrepl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1168,13 +1168,18 @@ def test_completions(self):
("import importlib.resources.\t\ta\t\n", "import importlib.resources.abc"),
("import foo, impo\t\n", "import foo, importlib"),
("import foo as bar, impo\t\n", "import foo as bar, importlib"),
("from impo\t\n", "from importlib"),
("from importlib.res\t\n", "from importlib.resources"),
("from importlib.\t\tres\t\n", "from importlib.resources"),
("from importlib.resources.ab\t\n", "from importlib.resources.abc"),
("from impo\t\n", "from importlib "),
Copy link
Copy Markdown
Member

@tomasr8 tomasr8 Apr 12, 2026

Choose a reason for hiding this comment

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

I'm not sure whether we should change this, what do you think? importlib has submodules so I think we should not add the final space, in case you want to type something like from importlib.foo ...

Copy link
Copy Markdown
Contributor Author

@loic-simon loic-simon Apr 12, 2026

Choose a reason for hiding this comment

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

Hmm yes, I didn't thought of that! But that applies to a lot of modules (including my example of from mat import <tab><tab>)...

I think having a different behaviour if the module has submodules or not would be both obscure and hard to implement.

So I see two three paths forward:

  1. abandon the space insertion idea, and stick to "insert import" only
  2. keep it anyway (for all imports), but I think that can be quite frustating if you want to import from a submodule indeed
  3. don't insert space, but add a new special case: when the text to complete is an exact match, insert import.

eg. to come back to my table:

Input Current Proposed
from math <tab> from math from math import
from mat<tab> from math from math (no more extra space)
from math<tab> from math from math import (insert space + import)

That allow from mat<tab><tab> to still work, and should be quite straightforward, so at a glance I quite like it!

Copy link
Copy Markdown
Member

@tomasr8 tomasr8 Apr 13, 2026

Choose a reason for hiding this comment

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

I think having a different behaviour if the module has submodules or not would be both obscure and hard to implement.

Hard probably, but UX-wise I think it'd be nice if it added the space for modules that don't have submodules. It would save you one keystroke and immediately signal that there are no submodules to import from.

("from impo\t\t\n", "from importlib import "),
("from impo \t\n", "from impo import "),
("from importlib.res\t\n", "from importlib.resources "),
("from importlib.\t\tres\t\n", "from importlib.resources "),
("from importlib.res\t\t\n", "from importlib.resources import "),
("from importlib.res \t\n", "from importlib.res import "),
("from importlib.resources.ab\t\n", "from importlib.resources.abc "),
("from importlib import mac\t\n", "from importlib import machinery"),
("from importlib import res\t\n", "from importlib import resources"),
("from importlib.res\t import a\t\n", "from importlib.resources import abc"),
("from importlib.res\timport a\t\n", "from importlib.resources import abc"),
("from importlib.res\t\ta\t\n", "from importlib.resources import abc"),
("from __phello__ import s\t\n", "from __phello__ import spam"), # frozen module
)
for code, expected in cases:
Expand All @@ -1191,10 +1196,10 @@ def test_private_completions(self):
cases = (
# Return public methods by default
("import \t\n", "import public"),
("from \t\n", "from public"),
("from \t\n", "from public "),
# Return private methods if explicitly specified
("import _\t\n", "import _private"),
("from _\t\n", "from _private"),
("from _\t\n", "from _private "),
)
for code, expected in cases:
with self.subTest(code=code):
Expand Down Expand Up @@ -1227,7 +1232,7 @@ def test_sub_module_private_completions(self):
def test_builtin_completion_top_level(self):
cases = (
("import bui\t\n", "import builtins"),
("from bui\t\n", "from builtins"),
("from bui\t\n", "from builtins "),
)
for code, expected in cases:
with self.subTest(code=code):
Expand All @@ -1240,11 +1245,11 @@ def test_relative_completions(self):
cases = (
(None, "from .readl\t\n", "from .readl"),
(None, "from . import readl\t\n", "from . import readl"),
("_pyrepl", "from .readl\t\n", "from .readline"),
("_pyrepl", "from .readl\t\n", "from .readline "),
("_pyrepl", "from . import readl\t\n", "from . import readline"),
("_pyrepl", "from .readline import mul\t\n", "from .readline import multiline_input"),
("_pyrepl", "from .. import toodeep\t\n", "from .. import toodeep"),
("concurrent", "from .futures.i\t\n", "from .futures.interpreter"),
("concurrent", "from .futures.i\t\n", "from .futures.interpreter "),
)
for package, code, expected in cases:
with self.subTest(code=code):
Expand Down Expand Up @@ -1545,41 +1550,43 @@ def test_get_path_and_prefix(self):

def test_parse(self):
cases = (
('import ', (None, '')),
('import foo', (None, 'foo')),
('import foo,', (None, '')),
('import foo, ', (None, '')),
('import foo, bar', (None, 'bar')),
('import foo, bar, baz', (None, 'baz')),
('import foo as bar,', (None, '')),
('import foo as bar, ', (None, '')),
('import foo as bar, baz', (None, 'baz')),
('import a.', (None, 'a.')),
('import a.b', (None, 'a.b')),
('import a.b.', (None, 'a.b.')),
('import a.b.c', (None, 'a.b.c')),
('import a.b.c, foo', (None, 'foo')),
('import a.b.c, foo.bar', (None, 'foo.bar')),
('import a.b.c, foo.bar,', (None, '')),
('import a.b.c, foo.bar, ', (None, '')),
('from foo', ('foo', None)),
('from a.', ('a.', None)),
('from a.b', ('a.b', None)),
('from a.b.', ('a.b.', None)),
('from a.b.c', ('a.b.c', None)),
('from foo import ', ('foo', '')),
('from foo import a', ('foo', 'a')),
('from ', ('', None)),
('from . import a', ('.', 'a')),
('from .foo import a', ('.foo', 'a')),
('from ..foo import a', ('..foo', 'a')),
('from foo import (', ('foo', '')),
('from foo import ( ', ('foo', '')),
('from foo import (a', ('foo', 'a')),
('from foo import (a,', ('foo', '')),
('from foo import (a, ', ('foo', '')),
('from foo import (a, c', ('foo', 'c')),
('from foo import (a as b, c', ('foo', 'c')),
('import ', (None, '', True)),
('import foo', (None, 'foo', False)),
('import foo,', (None, '', False)),
('import foo, ', (None, '', True)),
('import foo, bar', (None, 'bar', False)),
('import foo, bar, baz', (None, 'baz', False)),
('import foo as bar,', (None, '', False)),
('import foo as bar, ', (None, '', True)),
('import foo as bar, baz', (None, 'baz', False)),
('import a.', (None, 'a.', False)),
('import a.b', (None, 'a.b', False)),
('import a.b.', (None, 'a.b.', False)),
('import a.b.c', (None, 'a.b.c', False)),
('import a.b.c, foo', (None, 'foo', False)),
('import a.b.c, foo.bar', (None, 'foo.bar', False)),
('import a.b.c, foo.bar,', (None, '', False)),
('import a.b.c, foo.bar, ', (None, '', True)),
('from foo', ('foo', None, False)),
('from foo ', ('foo', None, True)),
('from a.', ('a.', None, False)),
('from a.b', ('a.b', None, False)),
('from a.b.', ('a.b.', None, False)),
('from a.b ', ('a.b', None, True)),
('from a.b.c', ('a.b.c', None, False)),
('from foo import ', ('foo', '', True)),
('from foo import a', ('foo', 'a', False)),
('from ', ('', None, True)),
('from . import a', ('.', 'a', False)),
('from .foo import a', ('.foo', 'a', False)),
('from ..foo import a', ('..foo', 'a', False)),
('from foo import (', ('foo', '', False)),
('from foo import ( ', ('foo', '', True)),
('from foo import (a', ('foo', 'a', False)),
('from foo import (a,', ('foo', '', False)),
('from foo import (a, ', ('foo', '', True)),
('from foo import (a, c', ('foo', 'c', False)),
('from foo import (a as b, c', ('foo', 'c', False)),
)
for code, parsed in cases:
parser = ImportParser(code)
Expand All @@ -1603,12 +1610,9 @@ def test_parse_error(self):
cases = (
'',
'import foo ',
'from foo ',
'import foo. ',
'import foo.bar ',
'from foo ',
'from foo. ',
'from foo.bar ',
'from foo import bar ',
'from foo import (bar ',
'from foo import bar, baz ',
Expand Down Expand Up @@ -1647,9 +1651,9 @@ def test_parse_error(self):
'if 1:\n pass\n\tpass', # _tokenize TabError -> tokenize TabError
)
for code in cases:
parser = ImportParser(code)
actual = parser.parse()
with self.subTest(code=code):
parser = ImportParser(code)
actual = parser.parse()
self.assertEqual(actual, None)

@patch.dict(sys.modules)
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2026-04-12-13-48-26.gh-issue-69605.T9HWOL.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Insert ``import`` after ``from x.y.z <tab>`` in the :term:`REPL`
auto-completion.
Loading