Skip to content

Commit f8293fa

Browse files
authored
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.
1 parent 36f15ba commit f8293fa

File tree

4 files changed

+46
-41
lines changed

4 files changed

+46
-41
lines changed

Lib/_pyrepl/fancycompleter.py

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,6 @@ def attr_matches(self, text):
105105
names = [f'{expr}.{name}' for name in names]
106106
if self.use_colors:
107107
return self.colorize_matches(names, values)
108-
109-
if prefix:
110-
names.append(' ')
111108
return names
112109

113110
def _attr_matches(self, text):
@@ -173,21 +170,15 @@ def _attr_matches(self, text):
173170
return expr, attr, names, values
174171

175172
def colorize_matches(self, names, values):
176-
matches = [self._color_for_obj(i, name, obj)
177-
for i, (name, obj)
178-
in enumerate(zip(names, values))]
179-
# We add a space at the end to prevent the automatic completion of the
180-
# common prefix, which is the ANSI escape sequence.
181-
matches.append(' ')
182-
return matches
183-
184-
def _color_for_obj(self, i, name, value):
173+
return [
174+
self._color_for_obj(name, obj)
175+
for name, obj in zip(names, values)
176+
]
177+
178+
def _color_for_obj(self, name, value):
185179
t = type(value)
186180
color = self._color_by_type(t)
187-
# Encode the match index into a fake escape sequence that
188-
# stripcolor() can still remove once i reaches four digits.
189-
N = f"\x1b[{i // 100:03d};{i % 100:02d}m"
190-
return f"{N}{color}{name}{ANSIColors.RESET}"
181+
return f"{color}{name}{ANSIColors.RESET}"
191182

192183
def _color_by_type(self, t):
193184
typename = t.__name__

Lib/_pyrepl/readline.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
from rlcompleter import Completer as RLCompleter
3838

3939
from . import commands, historical_reader
40-
from .completing_reader import CompletingReader
40+
from .completing_reader import CompletingReader, stripcolor
4141
from .console import Console as ConsoleType
4242
from ._module_completer import ModuleCompleter, make_default_module_completer
4343
from .fancycompleter import Completer as FancyCompleter
@@ -163,9 +163,9 @@ def get_completions(self, stem: str) -> tuple[list[str], CompletionAction | None
163163
break
164164
result.append(next)
165165
state += 1
166-
# emulate the behavior of the standard readline that sorts
167-
# the completions before displaying them.
168-
result.sort()
166+
# Emulate readline's sorting using the visible text rather than
167+
# the raw ANSI escape sequences used for colorized matches.
168+
result.sort(key=stripcolor)
169169
return result, None
170170

171171
def get_module_completions(self) -> tuple[list[str], CompletionAction | None] | None:

Lib/test/test_pyrepl/test_fancycompleter.py

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class C(object):
5555
self.assertEqual(compl.attr_matches('a.'), ['a.attr', 'a.mro'])
5656
self.assertEqual(
5757
compl.attr_matches('a._'),
58-
['a._C__attr__attr', 'a._attr', ' '],
58+
['a._C__attr__attr', 'a._attr'],
5959
)
6060
matches = compl.attr_matches('a.__')
6161
self.assertNotIn('__class__', matches)
@@ -79,7 +79,7 @@ def test_complete_attribute_colored(self):
7979
break
8080
else:
8181
self.assertFalse(True, matches)
82-
self.assertIn(' ', matches)
82+
self.assertNotIn(' ', matches)
8383

8484
def test_preserves_callable_postfix_for_single_attribute_match(self):
8585
compl = Completer({'os': os}, use_colors=False)
@@ -159,22 +159,17 @@ def test_complete_global_colored(self):
159159
self.assertEqual(compl.global_matches('foo'), ['fooba'])
160160
matches = compl.global_matches('fooba')
161161

162-
# these are the fake escape sequences which are needed so that
163-
# readline displays the matches in the proper order
164-
N0 = f"\x1b[000;00m"
165-
N1 = f"\x1b[000;01m"
166162
int_color = theme.fancycompleter.int
167-
self.assertEqual(set(matches), {
168-
' ',
169-
f'{N0}{int_color}foobar{ANSIColors.RESET}',
170-
f'{N1}{int_color}foobazzz{ANSIColors.RESET}',
171-
})
163+
self.assertEqual(matches, [
164+
f'{int_color}foobar{ANSIColors.RESET}',
165+
f'{int_color}foobazzz{ANSIColors.RESET}',
166+
])
172167
self.assertEqual(compl.global_matches('foobaz'), ['foobazzz'])
173168
self.assertEqual(compl.global_matches('nothing'), [])
174169

175-
def test_large_color_sort_prefix_is_stripped(self):
170+
def test_colorized_match_is_stripped(self):
176171
compl = Completer({'a': 42}, use_colors=True)
177-
match = compl._color_for_obj(1000, 'spam', 1)
172+
match = compl._color_for_obj('spam', 1)
178173
self.assertEqual(stripcolor(match), 'spam')
179174

180175
def test_complete_with_indexer(self):
@@ -197,27 +192,24 @@ class A:
197192
compl = Completer({'A': A}, use_colors=False)
198193
#
199194
# In this case, we want to display all attributes which start with
200-
# 'a'. Moreover, we also include a space to prevent readline to
201-
# automatically insert the common prefix (which will the the ANSI escape
202-
# sequence if we use colors).
195+
# 'a'.
203196
matches = compl.attr_matches('A.a')
204197
self.assertEqual(
205198
sorted(matches),
206-
[' ', 'A.aaa', 'A.abc_1', 'A.abc_2', 'A.abc_3'],
199+
['A.aaa', 'A.abc_1', 'A.abc_2', 'A.abc_3'],
207200
)
208201
#
209202
# If there is an actual common prefix, we return just it, so that readline
210203
# will insert it into place
211204
matches = compl.attr_matches('A.ab')
212205
self.assertEqual(matches, ['A.abc_'])
213206
#
214-
# Finally, at the next tab, we display again all the completions available
215-
# for this common prefix. Again, we insert a spurious space to prevent the
216-
# automatic completion of ANSI sequences.
207+
# Finally, at the next tab, we display again all the completions
208+
# available for this common prefix.
217209
matches = compl.attr_matches('A.abc_')
218210
self.assertEqual(
219211
sorted(matches),
220-
[' ', 'A.abc_1', 'A.abc_2', 'A.abc_3'],
212+
['A.abc_1', 'A.abc_2', 'A.abc_3'],
221213
)
222214

223215
def test_complete_exception(self):

Lib/test/test_pyrepl/test_pyrepl.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
code_to_events,
3737
)
3838
from _pyrepl.console import Event
39+
from _pyrepl.completing_reader import stripcolor
3940
from _pyrepl._module_completer import (
4041
ImportParser,
4142
ModuleCompleter,
@@ -999,6 +1000,27 @@ class Obj:
9991000
self.assertNotIn("banana", menu)
10001001
self.assertNotIn("mro", menu)
10011002

1003+
def test_get_completions_sorts_colored_matches_by_visible_text(self):
1004+
console = FakeConsole(iter(()))
1005+
config = ReadlineConfig()
1006+
config.readline_completer = FancyCompleter(
1007+
{
1008+
"foo_str": "value",
1009+
"foo_int": 1,
1010+
"foo_none": None,
1011+
},
1012+
use_colors=True,
1013+
).complete
1014+
reader = ReadlineAlikeReader(console=console, config=config)
1015+
1016+
matches, action = reader.get_completions("foo_")
1017+
1018+
self.assertIsNone(action)
1019+
self.assertEqual(
1020+
[stripcolor(match) for match in matches],
1021+
["foo_int", "foo_none", "foo_str"],
1022+
)
1023+
10021024

10031025
class TestPyReplReadlineSetup(TestCase):
10041026
def test_setup_ignores_basic_completer_env_when_env_is_disabled(self):

0 commit comments

Comments
 (0)