Improve behavior when both submodules + prompt

Author: loic-simon

Lib/_pyrepl/completing_reader.py

@@ -188,14 +188,13 @@ def do(self) -> None:
             if not r.cmpltn_action:
                 r.error("no matches")
         elif len(completions) == 1:
-            if not r.cmpltn_action:
-                if completions_unchangable and len(completions[0]) == len(stem):
-                    r.msg = "[ sole completion ]"
-                    r.dirty = True
-                r.insert(completions[0][len(stem):])
+            if completions_unchangable and len(completions[0]) == len(stem):
+                r.msg = "[ sole completion ]"
+                r.dirty = True
+            r.insert(completions[0][len(stem):])
         else:
             p = prefix(completions, len(stem))
-            if p and not r.cmpltn_action:
+            if p:
                 r.insert(p)
             if last_is_completer:
                 r.cmpltn_menu_visible = True
@@ -214,12 +213,14 @@ def do(self) -> None:
                     r.dirty = True
 
         if r.cmpltn_action:
-            if r.msg:
-                r.msg += "\n" + r.cmpltn_action[0]
+            if r.msg and r.cmpltn_message_visible:
+                # There is already a message (eg. [ not unique ]) that
+                # would conflict for next tab: cancel action
+                r.cmpltn_action = None
             else:
                 r.msg = r.cmpltn_action[0]
-            r.cmpltn_message_visible = True
-            r.dirty = True
+                r.cmpltn_message_visible = True
+                r.dirty = True
 
 
 class self_insert(commands.self_insert):

Lib/test/test_pyrepl/test_pyrepl.py

@@ -1136,28 +1136,42 @@ def test_attribute_completion_module_on_demand(self):
             dir = pathlib.Path(_dir)
             (dir / "foo.py").write_text("bar = 42")
             (dir / "pack").mkdir()
-            (dir / "pack" / "__init__.py").touch()
+            (dir / "pack" / "__init__.py").write_text("attr = 42")
+            (dir / "pack" / "foo.py").touch()
             (dir / "pack" / "bar.py").touch()
+            (dir / "pack" / "baz.py").touch()
             with patch.object(sys, "path", [_dir, *sys.path]):
                 cases = (
-                    ("from foo import \t\n", "from foo import ", False),
-                    ("from foo import \t\t\n", "from foo import bar", True),
-                    ("from foo import ba\t\n", "from foo import ba", False),
-                    ("from foo import ba\t\t\n", "from foo import bar", True),
-                    ("from foo import \tb\ta\t\n", "from foo import ba", False),
-                    # only one suggestion but message: do not complete
-                    ("from pack import \t\n", "from pack import ", False),
+                    # needs 2 tabs to import (show prompt, then import)
+                    ("from foo import \t\n", "from foo import ", set()),
+                    ("from foo import \t\t\n", "from foo import bar", {"foo"}),
+                    ("from foo import ba\t\n", "from foo import ba", set()),
+                    ("from foo import ba\t\t\n", "from foo import bar", {"foo"}),
+                    # reset if a character is inserted between tabs
+                    ("from foo import \tb\ta\t\n", "from foo import ba", set()),
+                    # packages: needs 3 tabs ([ not unique ], prompt, import)
+                    ("from pack import \t\t\n", "from pack import ", set()),
+                    ("from pack import \t\t\t\n", "from pack import ", {"pack"}),
+                    ("from pack import \t\t\ta\t\n", "from pack import attr", {"pack"}),
+                    # one match: needs 2 tabs (insert + show prompt, import)
+                    ("from pack import f\t\n", "from pack import foo", set()),
+                    ("from pack import f\t\t\n", "from pack import foo", {"pack"}),
+                    # common prefix: needs 3 tabs (insert + [ not unique ], prompt, import)
+                    ("from pack import b\t\n", "from pack import ba", set()),
+                    ("from pack import b\t\t\n", "from pack import ba", set()),
+                    ("from pack import b\t\t\t\n", "from pack import ba", {"pack"}),
                 )
-                for code, expected, is_foo_imported in cases:
-                    with self.subTest(code=code):
+                for code, expected, expected_imports in cases:
+                    with self.subTest(code=code), patch.dict(sys.modules):
+                        _imported = set(sys.modules.keys())
                         events = code_to_events(code)
                         reader = self.prepare_reader(events, namespace={})
                         output = reader.readline()
                         self.assertEqual(output, expected)
-                        self.assertEqual("foo" in sys.modules, is_foo_imported)
-                        if is_foo_imported:
-                            del sys.modules["foo"]
+                        new_imports = sys.modules.keys() - _imported
+                        self.assertEqual(new_imports, expected_imports)
 
+    @patch.dict(sys.modules)
     def test_attribute_completion_error_on_import(self):
         with tempfile.TemporaryDirectory() as _dir:
             dir = pathlib.Path(_dir)
@@ -1175,7 +1189,7 @@ def test_attribute_completion_error_on_import(self):
                         output = reader.readline()
                         self.assertEqual(output, expected)
                 self.assertNotIn("boom", sys.modules)
-                del sys.modules["foo"]
+
 
     def test_attribute_completion_error_on_attributes_access(self):
         class BrokenModule:
@@ -1191,6 +1205,7 @@ def __dir__(self):
             # ignore attributes, just propose submodule
             self.assertEqual(output, "from boom import submodule")
 
+    @patch.dict(sys.modules)
     def test_attribute_completion_private_and_invalid_names(self):
         with tempfile.TemporaryDirectory() as _dir:
             dir = pathlib.Path(_dir)
@@ -1209,7 +1224,7 @@ def test_attribute_completion_private_and_invalid_names(self):
                         reader = self.prepare_reader(events, namespace={})
                         output = reader.readline()
                         self.assertEqual(output, expected)
-                del sys.modules["foo"]
+
 
     def test_get_path_and_prefix(self):
         cases = (
@@ -1399,6 +1414,7 @@ def test_suggestions_and_messages(self) -> None:
                             self.addCleanup(sys.modules.pop, mod)
 
 class TestHardcodedSubmodules(TestCase):
+    @patch.dict(sys.modules)
     def test_hardcoded_stdlib_submodules_are_importable(self):
         for parent_path, submodules in HARDCODED_SUBMODULES.items():
             for module_name in submodules: