From a669227ac56146feead1a4404eb1467aeb58c4f7 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Tue, 24 Sep 2024 18:39:10 -0700 Subject: [PATCH 1/6] gh-123024: Correctly prepare/restore around help and show-history commands --- Lib/_pyrepl/commands.py | 12 +++++++--- Lib/_pyrepl/console.py | 1 + Lib/_pyrepl/simple_interact.py | 2 +- Lib/_pyrepl/unix_console.py | 44 +++++++++++++++++----------------- Lib/_pyrepl/windows_console.py | 26 ++++++++++---------- 5 files changed, 46 insertions(+), 39 deletions(-) diff --git a/Lib/_pyrepl/commands.py b/Lib/_pyrepl/commands.py index c3fce91013b001..285841ca5e5b1c 100644 --- a/Lib/_pyrepl/commands.py +++ b/Lib/_pyrepl/commands.py @@ -459,9 +459,15 @@ def do(self) -> None: from site import gethistoryfile # type: ignore[attr-defined] history = os.linesep.join(self.reader.history[:]) - with self.reader.suspend(): - pager = get_pager() - pager(history, gethistoryfile()) + self.reader.console.restore() + pager = get_pager() + pager(history, gethistoryfile()) + self.reader.console.prepare() + + # We need to copy over the state so that it's consistent between + # console and reader, and console does not overwrite/append stuff + self.reader.console.screen = self.reader.screen.copy() + self.reader.console.posxy = self.reader.cxy class paste_mode(Command): diff --git a/Lib/_pyrepl/console.py b/Lib/_pyrepl/console.py index 3e72a56807f6fb..773e11a47e6e6c 100644 --- a/Lib/_pyrepl/console.py +++ b/Lib/_pyrepl/console.py @@ -45,6 +45,7 @@ class Event: @dataclass class Console(ABC): + posxy: tuple[int, int] screen: list[str] = field(default_factory=list) height: int = 25 width: int = 80 diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index 3c79cf61d04051..985aba1f0e8f7e 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -74,7 +74,7 @@ def _clear_screen(): "exit": _sitebuiltins.Quitter('exit', ''), "quit": _sitebuiltins.Quitter('quit' ,''), "copyright": _sitebuiltins._Printer('copyright', sys.copyright), - "help": "help", + "help": _sitebuiltins._Helper(), "clear": _clear_screen, "\x1a": _sitebuiltins.Quitter('\x1a', ''), } diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index 2576b938a34c64..5a22ff8261b313 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -240,7 +240,7 @@ def refresh(self, screen, c_xy): self.__hide_cursor() self.__move(0, len(self.screen) - 1) self.__write("\n") - self.__posxy = 0, len(self.screen) + self.posxy = 0, len(self.screen) self.screen.append("") else: while len(self.screen) < len(screen): @@ -250,7 +250,7 @@ def refresh(self, screen, c_xy): self.__gone_tall = 1 self.__move = self.__move_tall - px, py = self.__posxy + px, py = self.posxy old_offset = offset = self.__offset height = self.height @@ -271,7 +271,7 @@ def refresh(self, screen, c_xy): if old_offset > offset and self._ri: self.__hide_cursor() self.__write_code(self._cup, 0, 0) - self.__posxy = 0, old_offset + self.posxy = 0, old_offset for i in range(old_offset - offset): self.__write_code(self._ri) oldscr.pop(-1) @@ -279,7 +279,7 @@ def refresh(self, screen, c_xy): elif old_offset < offset and self._ind: self.__hide_cursor() self.__write_code(self._cup, self.height - 1, 0) - self.__posxy = 0, old_offset + self.height - 1 + self.posxy = 0, old_offset + self.height - 1 for i in range(offset - old_offset): self.__write_code(self._ind) oldscr.pop(0) @@ -299,7 +299,7 @@ def refresh(self, screen, c_xy): while y < len(oldscr): self.__hide_cursor() self.__move(0, y) - self.__posxy = 0, y + self.posxy = 0, y self.__write_code(self._el) y += 1 @@ -321,7 +321,7 @@ def move_cursor(self, x, y): self.event_queue.insert(Event("scroll", None)) else: self.__move(x, y) - self.__posxy = x, y + self.posxy = x, y self.flushoutput() def prepare(self): @@ -350,7 +350,7 @@ def prepare(self): self.__buffer = [] - self.__posxy = 0, 0 + self.posxy = 0, 0 self.__gone_tall = 0 self.__move = self.__move_short self.__offset = 0 @@ -559,7 +559,7 @@ def clear(self): self.__write_code(self._clear) self.__gone_tall = 1 self.__move = self.__move_tall - self.__posxy = 0, 0 + self.posxy = 0, 0 self.screen = [] @property @@ -644,8 +644,8 @@ def __write_changed_line(self, y, oldline, newline, px_coord): # if we need to insert a single character right after the first detected change if oldline[x_pos:] == newline[x_pos + 1 :] and self.ich1: if ( - y == self.__posxy[1] - and x_coord > self.__posxy[0] + y == self.posxy[1] + and x_coord > self.posxy[0] and oldline[px_pos:x_pos] == newline[px_pos + 1 : x_pos + 1] ): x_pos = px_pos @@ -654,7 +654,7 @@ def __write_changed_line(self, y, oldline, newline, px_coord): self.__move(x_coord, y) self.__write_code(self.ich1) self.__write(newline[x_pos]) - self.__posxy = x_coord + character_width, y + self.posxy = x_coord + character_width, y # if it's a single character change in the middle of the line elif ( @@ -665,7 +665,7 @@ def __write_changed_line(self, y, oldline, newline, px_coord): character_width = wlen(newline[x_pos]) self.__move(x_coord, y) self.__write(newline[x_pos]) - self.__posxy = x_coord + character_width, y + self.posxy = x_coord + character_width, y # if this is the last character to fit in the line and we edit in the middle of the line elif ( @@ -677,14 +677,14 @@ def __write_changed_line(self, y, oldline, newline, px_coord): ): self.__hide_cursor() self.__move(self.width - 2, y) - self.__posxy = self.width - 2, y + self.posxy = self.width - 2, y self.__write_code(self.dch1) character_width = wlen(newline[x_pos]) self.__move(x_coord, y) self.__write_code(self.ich1) self.__write(newline[x_pos]) - self.__posxy = character_width + 1, y + self.posxy = character_width + 1, y else: self.__hide_cursor() @@ -692,7 +692,7 @@ def __write_changed_line(self, y, oldline, newline, px_coord): if wlen(oldline) > wlen(newline): self.__write_code(self._el) self.__write(newline[x_pos:]) - self.__posxy = wlen(newline), y + self.posxy = wlen(newline), y if "\x1b" in newline: # ANSI escape characters are present, so we can't assume @@ -711,32 +711,32 @@ def __maybe_write_code(self, fmt, *args): self.__write_code(fmt, *args) def __move_y_cuu1_cud1(self, y): - dy = y - self.__posxy[1] + dy = y - self.posxy[1] if dy > 0: self.__write_code(dy * self._cud1) elif dy < 0: self.__write_code((-dy) * self._cuu1) def __move_y_cuu_cud(self, y): - dy = y - self.__posxy[1] + dy = y - self.posxy[1] if dy > 0: self.__write_code(self._cud, dy) elif dy < 0: self.__write_code(self._cuu, -dy) def __move_x_hpa(self, x: int) -> None: - if x != self.__posxy[0]: + if x != self.posxy[0]: self.__write_code(self._hpa, x) def __move_x_cub1_cuf1(self, x: int) -> None: - dx = x - self.__posxy[0] + dx = x - self.posxy[0] if dx > 0: self.__write_code(self._cuf1 * dx) elif dx < 0: self.__write_code(self._cub1 * (-dx)) def __move_x_cub_cuf(self, x: int) -> None: - dx = x - self.__posxy[0] + dx = x - self.posxy[0] if dx > 0: self.__write_code(self._cuf, dx) elif dx < 0: @@ -766,12 +766,12 @@ def __show_cursor(self): def repaint(self): if not self.__gone_tall: - self.__posxy = 0, self.__posxy[1] + self.posxy = 0, self.posxy[1] self.__write("\r") ns = len(self.screen) * ["\000" * self.width] self.screen = ns else: - self.__posxy = 0, self.__offset + self.posxy = 0, self.__offset self.__move(0, self.__offset) ns = self.height * ["\000" * self.width] self.screen = ns diff --git a/Lib/_pyrepl/windows_console.py b/Lib/_pyrepl/windows_console.py index f7a0095d795ac6..ee93e5d807330c 100644 --- a/Lib/_pyrepl/windows_console.py +++ b/Lib/_pyrepl/windows_console.py @@ -148,10 +148,10 @@ def refresh(self, screen: list[str], c_xy: tuple[int, int]) -> None: self._hide_cursor() self._move_relative(0, len(self.screen) - 1) self.__write("\n") - self.__posxy = 0, len(self.screen) + self.posxy = 0, len(self.screen) self.screen.append("") - px, py = self.__posxy + px, py = self.posxy old_offset = offset = self.__offset height = self.height @@ -167,7 +167,7 @@ def refresh(self, screen: list[str], c_xy: tuple[int, int]) -> None: # portion of the window. We need to scroll the visible portion and the # entire history self._scroll(scroll_lines, self._getscrollbacksize()) - self.__posxy = self.__posxy[0], self.__posxy[1] + scroll_lines + self.posxy = self.posxy[0], self.posxy[1] + scroll_lines self.__offset += scroll_lines for i in range(scroll_lines): @@ -193,7 +193,7 @@ def refresh(self, screen: list[str], c_xy: tuple[int, int]) -> None: y = len(newscr) while y < len(oldscr): self._move_relative(0, y) - self.__posxy = 0, y + self.posxy = 0, y self._erase_to_end() y += 1 @@ -250,11 +250,11 @@ def __write_changed_line( if wlen(newline) == self.width: # If we wrapped we want to start at the next line self._move_relative(0, y + 1) - self.__posxy = 0, y + 1 + self.posxy = 0, y + 1 else: - self.__posxy = wlen(newline), y + self.posxy = wlen(newline), y - if "\x1b" in newline or y != self.__posxy[1] or '\x1a' in newline: + if "\x1b" in newline or y != self.posxy[1] or '\x1a' in newline: # ANSI escape characters are present, so we can't assume # anything about the position of the cursor. Moving the cursor # to the left margin should work to get to a known position. @@ -316,7 +316,7 @@ def prepare(self) -> None: self.screen = [] self.height, self.width = self.getheightwidth() - self.__posxy = 0, 0 + self.posxy = 0, 0 self.__gone_tall = 0 self.__offset = 0 @@ -324,9 +324,9 @@ def restore(self) -> None: pass def _move_relative(self, x: int, y: int) -> None: - """Moves relative to the current __posxy""" - dx = x - self.__posxy[0] - dy = y - self.__posxy[1] + """Moves relative to the current posxy""" + dx = x - self.posxy[0] + dy = y - self.posxy[1] if dx < 0: self.__write(MOVE_LEFT.format(-dx)) elif dx > 0: @@ -345,7 +345,7 @@ def move_cursor(self, x: int, y: int) -> None: self.event_queue.insert(0, Event("scroll", "")) else: self._move_relative(x, y) - self.__posxy = x, y + self.posxy = x, y def set_cursor_vis(self, visible: bool) -> None: if visible: @@ -443,7 +443,7 @@ def beep(self) -> None: def clear(self) -> None: """Wipe the screen""" self.__write(CLEAR) - self.__posxy = 0, 0 + self.posxy = 0, 0 self.screen = [""] def finish(self) -> None: From 42559efd0e0cb8d012bc39ad88b9cbd4ab4dea61 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Wed, 25 Sep 2024 10:56:45 -0700 Subject: [PATCH 2/6] Fix types in unix console --- Lib/_pyrepl/unix_console.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index 5a22ff8261b313..b4e7a472300f1f 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -711,6 +711,8 @@ def __maybe_write_code(self, fmt, *args): self.__write_code(fmt, *args) def __move_y_cuu1_cud1(self, y): + assert self._cud1 is not None + assert self._cuu1 is not None dy = y - self.posxy[1] if dy > 0: self.__write_code(dy * self._cud1) @@ -729,6 +731,8 @@ def __move_x_hpa(self, x: int) -> None: self.__write_code(self._hpa, x) def __move_x_cub1_cuf1(self, x: int) -> None: + assert self._cuf1 is not None + assert self._cub1 is not None dx = x - self.posxy[0] if dx > 0: self.__write_code(self._cuf1 * dx) From 316e6dbb5c09d7802f4b3a4920ecb08dd062e565 Mon Sep 17 00:00:00 2001 From: Emily Morehouse Date: Wed, 25 Sep 2024 17:04:50 -0700 Subject: [PATCH 3/6] Add starter for test function --- Lib/test/test_pyrepl/test_pyrepl.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 0f3e9996e77e45..e1d8d94384a49c 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1327,3 +1327,17 @@ def test_readline_history_file(self): def test_keyboard_interrupt_after_isearch(self): output, exit_code = self.run_repl(["\x12", "\x03", "exit"]) self.assertEqual(exit_code, 0) + + def test_prompt_after_help(self): + # Ensure that we don't see a double prompt after exiting `help` + output, exit_code = self.run_repl(["help", "q", "exit"]) + + # Regex pattern to remove ANSI escape sequences + ansi_escape = re.compile(r"(\x9B|\x1B\[)[0-?]*[ -\/]*[@-~]") + + # Remove ANSI escape codes from the string + cleaned_output = ansi_escape.sub("", output) + + self.assertEqual(exit_code, 0) + self.assertRegex(cleaned_output, r"(?>> )>>>\s*$") + From 829e58acb732aae7ba4428d28784ccc442bbc850 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 26 Sep 2024 16:30:59 -0700 Subject: [PATCH 4/6] Suspend history before commands & do not support string commands --- Lib/_pyrepl/historical_reader.py | 18 +++++++++++------- Lib/_pyrepl/simple_interact.py | 14 +++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/Lib/_pyrepl/historical_reader.py b/Lib/_pyrepl/historical_reader.py index 5d416f336ad5d2..c4b95fa2e81ee6 100644 --- a/Lib/_pyrepl/historical_reader.py +++ b/Lib/_pyrepl/historical_reader.py @@ -290,13 +290,17 @@ def get_item(self, i: int) -> str: @contextmanager def suspend(self) -> SimpleContextManager: - with super().suspend(): - try: - old_history = self.history[:] - del self.history[:] - yield - finally: - self.history[:] = old_history + with super().suspend(), self.suspend_history(): + yield + + @contextmanager + def suspend_history(self) -> SimpleContextManager: + try: + old_history = self.history[:] + del self.history[:] + yield + finally: + self.history[:] = old_history def prepare(self) -> None: super().prepare() diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index 53e43115c83df0..66e66eae7ead9c 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -124,18 +124,10 @@ def maybe_run_command(statement: str) -> bool: reader.history.pop() # skip internal commands in history command = REPL_COMMANDS[statement] if callable(command): - command() + # Make sure that history does not change because of commands + with reader.suspend_history(): + command() return True - - if isinstance(command, str): - # Internal readline commands require a prepared reader like - # inside multiline_input. - reader.prepare() - reader.refresh() - reader.do_cmd((command, [statement])) - reader.restore() - return True - return False while 1: From 2e991eb033fcbb6dca0d6f2914fc0de2f599ca20 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 26 Sep 2024 16:43:55 -0700 Subject: [PATCH 5/6] Add test for double prompt --- Lib/test/test_pyrepl/test_pyrepl.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index e1d8d94384a49c..3f5d500a6766a2 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1329,15 +1329,15 @@ def test_keyboard_interrupt_after_isearch(self): self.assertEqual(exit_code, 0) def test_prompt_after_help(self): - # Ensure that we don't see a double prompt after exiting `help` output, exit_code = self.run_repl(["help", "q", "exit"]) # Regex pattern to remove ANSI escape sequences - ansi_escape = re.compile(r"(\x9B|\x1B\[)[0-?]*[ -\/]*[@-~]") - - # Remove ANSI escape codes from the string + ansi_escape = re.compile(r"(\x1B(=|>|(\[)[0-?]*[ -\/]*[@-~]))") cleaned_output = ansi_escape.sub("", output) - self.assertEqual(exit_code, 0) - self.assertRegex(cleaned_output, r"(?>> )>>>\s*$") + + # Ensure that we don't see multiple prompts after exiting `help` + # Extra stuff (newline and `exit` rewrites) are necessary + # because of how run_repl works. + self.assertNotIn(">>> \n>>> >>>", cleaned_output) From 2cac4d56736517e1be2a86caf0bde233509edec0 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 27 Sep 2024 07:28:29 -0700 Subject: [PATCH 6/6] Fix linter --- Lib/test/test_pyrepl/test_pyrepl.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 3f5d500a6766a2..f06bf3c46ab6f9 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1340,4 +1340,3 @@ def test_prompt_after_help(self): # Extra stuff (newline and `exit` rewrites) are necessary # because of how run_repl works. self.assertNotIn(">>> \n>>> >>>", cleaned_output) -