gh-130472: Remove readline-only hacks from PyREPL completions (#148161)

PyREPL was still carrying over two readline-specific tricks from the
fancy completer: a synthetic CSI prefix to influence sorting and a fake
blank completion entry to suppress readline's prefix insertion. Those
workarounds are not appropriate in PyREPL because the reader already
owns completion ordering and menu rendering, so the fake entries leaked
into the UI as real terminal attributes and empty menu cells.

Sort completion candidates in ReadlineAlikeReader by their visible text
with stripcolor(), and let the fancy completer return only real matches.
That keeps colored completions stable without emitting bogus escape
sequences, removes the empty completion slot, and adds regression tests
for both the low-level completer output and the reader integration.

diff --git a/Lib/_pyrepl/fancycompleter.py b/Lib/_pyrepl/fancycompleter.py
index 5b5b7ae5f2bb59ca78a586b4b423db687c71c1c5..7a639afd74ef3cc35d636547bbd53cc5a2e99182 100644 (file)
--- a/Lib/_pyrepl/fancycompleter.py
+++ b/Lib/_pyrepl/fancycompleter.py
@@ -105,9 +105,6 @@ class Completer(rlcompleter.Completer):
         names = [f'{expr}.{name}' for name in names]
         if self.use_colors:
             return self.colorize_matches(names, values)
-
-        if prefix:
-            names.append(' ')
         return names
 
     def _attr_matches(self, text):
@@ -173,21 +170,15 @@ class Completer(rlcompleter.Completer):
         return expr, attr, names, values
 
     def colorize_matches(self, names, values):
-        matches = [self._color_for_obj(i, name, obj)
-                   for i, (name, obj)
-                   in enumerate(zip(names, values))]
-        # We add a space at the end to prevent the automatic completion of the
-        # common prefix, which is the ANSI escape sequence.
-        matches.append(' ')
-        return matches
-
-    def _color_for_obj(self, i, name, value):
+        return [
+            self._color_for_obj(name, obj)
+            for name, obj in zip(names, values)
+        ]
+
+    def _color_for_obj(self, name, value):
         t = type(value)
         color = self._color_by_type(t)
-        # Encode the match index into a fake escape sequence that
-        # stripcolor() can still remove once i reaches four digits.
-        N = f"\x1b[{i // 100:03d};{i % 100:02d}m"
-        return f"{N}{color}{name}{ANSIColors.RESET}"
+        return f"{color}{name}{ANSIColors.RESET}"
 
     def _color_by_type(self, t):
         typename = t.__name__

diff --git a/Lib/_pyrepl/readline.py b/Lib/_pyrepl/readline.py
index 687084601e77c1674a46e234f5c7350f684d4bc6..8d3be37b4adeec154f3a0386ba6aa777041c4346 100644 (file)
--- a/Lib/_pyrepl/readline.py
+++ b/Lib/_pyrepl/readline.py
@@ -37,7 +37,7 @@ import sys
 from rlcompleter import Completer as RLCompleter
 
 from . import commands, historical_reader
-from .completing_reader import CompletingReader
+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
@@ -163,9 +163,9 @@ class ReadlineAlikeReader(historical_reader.HistoricalReader, CompletingReader):
                     break
                 result.append(next)
                 state += 1
-            # emulate the behavior of the standard readline that sorts
-            # the completions before displaying them.
-            result.sort()
+            # Emulate readline's sorting using the visible text rather than
+            # the raw ANSI escape sequences used for colorized matches.
+            result.sort(key=stripcolor)
         return result, None
 
     def get_module_completions(self) -> tuple[list[str], CompletionAction | None] | None:

diff --git a/Lib/test/test_pyrepl/test_fancycompleter.py b/Lib/test/test_pyrepl/test_fancycompleter.py
index 77c80853a3c0e38505bba3cce1ff3063c37fc6f4..d2646cd3050428fb954bd2b551fe633f2ac83589 100644 (file)
--- a/Lib/test/test_pyrepl/test_fancycompleter.py
+++ b/Lib/test/test_pyrepl/test_fancycompleter.py
@@ -55,7 +55,7 @@ class FancyCompleterTests(unittest.TestCase):
         self.assertEqual(compl.attr_matches('a.'), ['a.attr', 'a.mro'])
         self.assertEqual(
             compl.attr_matches('a._'),
-            ['a._C__attr__attr', 'a._attr', ' '],
+            ['a._C__attr__attr', 'a._attr'],
         )
         matches = compl.attr_matches('a.__')
         self.assertNotIn('__class__', matches)
@@ -79,7 +79,7 @@ class FancyCompleterTests(unittest.TestCase):
                 break
         else:
             self.assertFalse(True, matches)
-        self.assertIn(' ', matches)
+        self.assertNotIn(' ', matches)
 
     def test_preserves_callable_postfix_for_single_attribute_match(self):
         compl = Completer({'os': os}, use_colors=False)
@@ -159,22 +159,17 @@ class FancyCompleterTests(unittest.TestCase):
         self.assertEqual(compl.global_matches('foo'), ['fooba'])
         matches = compl.global_matches('fooba')
 
-        # these are the fake escape sequences which are needed so that
-        # readline displays the matches in the proper order
-        N0 = f"\x1b[000;00m"
-        N1 = f"\x1b[000;01m"
         int_color = theme.fancycompleter.int
-        self.assertEqual(set(matches), {
-            ' ',
-            f'{N0}{int_color}foobar{ANSIColors.RESET}',
-            f'{N1}{int_color}foobazzz{ANSIColors.RESET}',
-        })
+        self.assertEqual(matches, [
+            f'{int_color}foobar{ANSIColors.RESET}',
+            f'{int_color}foobazzz{ANSIColors.RESET}',
+        ])
         self.assertEqual(compl.global_matches('foobaz'), ['foobazzz'])
         self.assertEqual(compl.global_matches('nothing'), [])
 
-    def test_large_color_sort_prefix_is_stripped(self):
+    def test_colorized_match_is_stripped(self):
         compl = Completer({'a': 42}, use_colors=True)
-        match = compl._color_for_obj(1000, 'spam', 1)
+        match = compl._color_for_obj('spam', 1)
         self.assertEqual(stripcolor(match), 'spam')
 
     def test_complete_with_indexer(self):
@@ -197,13 +192,11 @@ class FancyCompleterTests(unittest.TestCase):
         compl = Completer({'A': A}, use_colors=False)
         #
         # In this case, we want to display all attributes which start with
-        # 'a'. Moreover, we also include a space to prevent readline to
-        # automatically insert the common prefix (which will the the ANSI escape
-        # sequence if we use colors).
+        # 'a'.
         matches = compl.attr_matches('A.a')
         self.assertEqual(
             sorted(matches),
-            [' ', 'A.aaa', 'A.abc_1', 'A.abc_2', 'A.abc_3'],
+            ['A.aaa', 'A.abc_1', 'A.abc_2', 'A.abc_3'],
         )
         #
         # If there is an actual common prefix, we return just it, so that readline
@@ -211,13 +204,12 @@ class FancyCompleterTests(unittest.TestCase):
         matches = compl.attr_matches('A.ab')
         self.assertEqual(matches, ['A.abc_'])
         #
-        # Finally, at the next tab, we display again all the completions available
-        # for this common prefix. Again, we insert a spurious space to prevent the
-        # automatic completion of ANSI sequences.
+        # Finally, at the next tab, we display again all the completions
+        # available for this common prefix.
         matches = compl.attr_matches('A.abc_')
         self.assertEqual(
             sorted(matches),
-            [' ', 'A.abc_1', 'A.abc_2', 'A.abc_3'],
+            ['A.abc_1', 'A.abc_2', 'A.abc_3'],
         )
 
     def test_complete_exception(self):

diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py
index c3556823c72476e0541f52e0244856159e77863a..8a3cae966a6e054a49263f3b668392c9a3cf38dc 100644 (file)
--- a/Lib/test/test_pyrepl/test_pyrepl.py
+++ b/Lib/test/test_pyrepl/test_pyrepl.py
@@ -36,6 +36,7 @@ from .support import (
     code_to_events,
 )
 from _pyrepl.console import Event
+from _pyrepl.completing_reader import stripcolor
 from _pyrepl._module_completer import (
     ImportParser,
     ModuleCompleter,
@@ -999,6 +1000,27 @@ class TestPyReplFancyCompleter(TestCase):
         self.assertNotIn("banana", menu)
         self.assertNotIn("mro", menu)
 
+    def test_get_completions_sorts_colored_matches_by_visible_text(self):
+        console = FakeConsole(iter(()))
+        config = ReadlineConfig()
+        config.readline_completer = FancyCompleter(
+            {
+                "foo_str": "value",
+                "foo_int": 1,
+                "foo_none": None,
+            },
+            use_colors=True,
+        ).complete
+        reader = ReadlineAlikeReader(console=console, config=config)
+
+        matches, action = reader.get_completions("foo_")
+
+        self.assertIsNone(action)
+        self.assertEqual(
+            [stripcolor(match) for match in matches],
+            ["foo_int", "foo_none", "foo_str"],
+        )
+
 
 class TestPyReplReadlineSetup(TestCase):
     def test_setup_ignores_basic_completer_env_when_env_is_disabled(self):