From 7252828bb1b15979ff26c3f47f98039e427548e6 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 8 Apr 2025 15:03:04 +0300 Subject: [PATCH 1/7] gh-127495: append to history file after every successful statement in new repl --- Lib/_pyrepl/readline.py | 16 +++++++++++++++ Lib/_pyrepl/simple_interact.py | 6 +++++- Lib/test/test_pyrepl/test_pyrepl.py | 20 +++++++++++++++++++ ...-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst | 3 +++ 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst diff --git a/Lib/_pyrepl/readline.py b/Lib/_pyrepl/readline.py index be229488e54e37..7dafcbd7c0b5fe 100644 --- a/Lib/_pyrepl/readline.py +++ b/Lib/_pyrepl/readline.py @@ -89,6 +89,7 @@ # "set_pre_input_hook", "set_startup_hook", "write_history_file", + "append_history_file", # ---- multiline extensions ---- "multiline_input", ] @@ -446,6 +447,7 @@ def read_history_file(self, filename: str = gethistoryfile()) -> None: del buffer[:] if line: history.append(line) + self.set_history_length(self.get_current_history_length()) def write_history_file(self, filename: str = gethistoryfile()) -> None: maxlength = self.saved_history_length @@ -457,6 +459,19 @@ def write_history_file(self, filename: str = gethistoryfile()) -> None: entry = entry.replace("\n", "\r\n") # multiline history support f.write(entry + "\n") + def append_history_file(self, filename: str = gethistoryfile()) -> None: + reader = self.get_reader() + saved_length = self.get_history_length() + length = self.get_current_history_length() - saved_length + history = reader.get_trimmed_history(length) + f = open(os.path.expanduser(filename), "a", + encoding="utf-8", newline="\n") + with f: + for entry in history: + entry = entry.replace("\n", "\r\n") # multiline history support + f.write(entry + "\n") + self.set_history_length(saved_length + length) + def clear_history(self) -> None: del self.get_reader().history[:] @@ -526,6 +541,7 @@ def insert_text(self, text: str) -> None: get_current_history_length = _wrapper.get_current_history_length read_history_file = _wrapper.read_history_file write_history_file = _wrapper.write_history_file +append_history_file = _wrapper.append_history_file clear_history = _wrapper.clear_history get_history_item = _wrapper.get_history_item remove_history_item = _wrapper.remove_history_item diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index a08546a9319824..c168549bec32ab 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -31,7 +31,7 @@ import sys import code -from .readline import _get_reader, multiline_input +from .readline import _get_reader, multiline_input, append_history_file _error: tuple[type[Exception], ...] | type[Exception] @@ -143,6 +143,10 @@ def maybe_run_command(statement: str) -> bool: input_name = f"" more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single") # type: ignore[call-arg] + try: + append_history_file() + except (FileNotFoundError, PermissionError): + pass assert not more input_n += 1 except KeyboardInterrupt: diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 8b063a25913b0b..900f3cd398ad19 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1341,6 +1341,26 @@ def test_readline_history_file(self): self.assertEqual(exit_code, 0) self.assertNotIn("\\040", pathlib.Path(hfile.name).read_text()) + def test_history_survive_crash(self): + env = os.environ.copy() + commands = "1\nexit()\n" + output, exit_code = self.run_repl(commands, env=env) + if "can't use pyrepl" in output: + self.skipTest("pyrepl not available") + + with tempfile.NamedTemporaryFile() as hfile: + env["PYTHON_HISTORY"] = hfile.name + commands = "spam\nimport time\ntime.sleep(1000)\n" + try: + self.run_repl(commands, env=env) + except AssertionError: + pass + + history = pathlib.Path(hfile.name).read_text() + self.assertIn("spam", history) + self.assertIn("time", history) + self.assertNotIn("sleep", history) + def test_keyboard_interrupt_after_isearch(self): output, exit_code = self.run_repl(["\x12", "\x03", "exit"]) self.assertEqual(exit_code, 0) diff --git a/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst new file mode 100644 index 00000000000000..6ca175ddcb7698 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst @@ -0,0 +1,3 @@ +Save (append new entry) the repl history to file every time new statement +was succesful. This should preserve command-line history after interpreter +crash. Patch by Sergey B Kirpichev. From a28737fe865ced69d80bf87e30c565090b72febf Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 9 Apr 2025 03:26:02 +0300 Subject: [PATCH 2/7] + avoid warning --- Lib/test/test_pyrepl/test_pyrepl.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 900f3cd398ad19..20e5c5d54b4e02 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -111,6 +111,7 @@ def _run_repl( else: os.close(master_fd) process.kill() + process.wait(timeout=SHORT_TIMEOUT) self.fail(f"Timeout while waiting for output, got: {''.join(output)}") os.close(master_fd) From da157dc9b4142b611e557a64e626a7e12fdecb7b Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Thu, 10 Apr 2025 14:24:55 +0300 Subject: [PATCH 3/7] fix typo --- .../next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst index 6ca175ddcb7698..a90bf495f4fe8b 100644 --- a/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst +++ b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst @@ -1,3 +1,3 @@ Save (append new entry) the repl history to file every time new statement -was succesful. This should preserve command-line history after interpreter +was successful. This should preserve command-line history after interpreter crash. Patch by Sergey B Kirpichev. From 3db477a945ae8bec7e762e908d044b7aac18c4e6 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Fri, 11 Apr 2025 08:29:34 +0300 Subject: [PATCH 4/7] Prevent issues like #128066 --- Lib/_pyrepl/readline.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Lib/_pyrepl/readline.py b/Lib/_pyrepl/readline.py index 7dafcbd7c0b5fe..42a321a262a195 100644 --- a/Lib/_pyrepl/readline.py +++ b/Lib/_pyrepl/readline.py @@ -464,8 +464,14 @@ def append_history_file(self, filename: str = gethistoryfile()) -> None: saved_length = self.get_history_length() length = self.get_current_history_length() - saved_length history = reader.get_trimmed_history(length) - f = open(os.path.expanduser(filename), "a", - encoding="utf-8", newline="\n") + + try: + f = open(os.path.expanduser(filename), "a", + encoding="utf-8", newline="\n") + except OSError as e: + warnings.warn(f"failed to open the history file for writing: {e}") + return + with f: for entry in history: entry = entry.replace("\n", "\r\n") # multiline history support From 5dadcc4b46752000d289f37b3f3cb54cf8374b3d Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 15 Apr 2025 07:42:14 +0300 Subject: [PATCH 5/7] History saved *before* statement was executed --- Lib/_pyrepl/simple_interact.py | 4 ++-- Lib/test/test_pyrepl/test_pyrepl.py | 5 +++-- .../Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index c168549bec32ab..3b3fe997b1e17b 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -141,12 +141,12 @@ def maybe_run_command(statement: str) -> bool: if maybe_run_command(statement): continue - input_name = f"" - more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single") # type: ignore[call-arg] try: append_history_file() except (FileNotFoundError, PermissionError): pass + input_name = f"" + more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single") # type: ignore[call-arg] assert not more input_n += 1 except KeyboardInterrupt: diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 20e5c5d54b4e02..c0f9f8737515d1 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1351,7 +1351,7 @@ def test_history_survive_crash(self): with tempfile.NamedTemporaryFile() as hfile: env["PYTHON_HISTORY"] = hfile.name - commands = "spam\nimport time\ntime.sleep(1000)\n" + commands = "spam\nimport time\ntime.sleep(1000)\npreved\n" try: self.run_repl(commands, env=env) except AssertionError: @@ -1360,7 +1360,8 @@ def test_history_survive_crash(self): history = pathlib.Path(hfile.name).read_text() self.assertIn("spam", history) self.assertIn("time", history) - self.assertNotIn("sleep", history) + self.assertIn("sleep", history) + self.assertNotIn("preved", history) def test_keyboard_interrupt_after_isearch(self): output, exit_code = self.run_repl(["\x12", "\x03", "exit"]) diff --git a/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst index a90bf495f4fe8b..9762f550d563fb 100644 --- a/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst +++ b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst @@ -1,3 +1,3 @@ -Save (append new entry) the repl history to file every time new statement -was successful. This should preserve command-line history after interpreter -crash. Patch by Sergey B Kirpichev. +In PyREPL, append a new entry to the ``PYTHON_HISTORY`` file *before* every +statement was executed. This should preserve command-line history after +interpreter is terminated. Patch by Sergey B Kirpichev. From 8ea0cfae63e5f93163c42c1b3ed0ffba413ab34a Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 15 Apr 2025 07:52:00 +0300 Subject: [PATCH 6/7] catch OSError in maybe_run_command() instead --- Lib/_pyrepl/readline.py | 10 ++-------- Lib/_pyrepl/simple_interact.py | 5 +++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/Lib/_pyrepl/readline.py b/Lib/_pyrepl/readline.py index 42a321a262a195..7dafcbd7c0b5fe 100644 --- a/Lib/_pyrepl/readline.py +++ b/Lib/_pyrepl/readline.py @@ -464,14 +464,8 @@ def append_history_file(self, filename: str = gethistoryfile()) -> None: saved_length = self.get_history_length() length = self.get_current_history_length() - saved_length history = reader.get_trimmed_history(length) - - try: - f = open(os.path.expanduser(filename), "a", - encoding="utf-8", newline="\n") - except OSError as e: - warnings.warn(f"failed to open the history file for writing: {e}") - return - + f = open(os.path.expanduser(filename), "a", + encoding="utf-8", newline="\n") with f: for entry in history: entry = entry.replace("\n", "\r\n") # multiline history support diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index 3b3fe997b1e17b..3d1616c257b27b 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -30,6 +30,7 @@ import os import sys import code +import warnings from .readline import _get_reader, multiline_input, append_history_file @@ -143,8 +144,8 @@ def maybe_run_command(statement: str) -> bool: try: append_history_file() - except (FileNotFoundError, PermissionError): - pass + except (FileNotFoundError, PermissionError, OSError) as e: + warnings.warn(f"failed to open the history file for writing: {e}") input_name = f"" more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single") # type: ignore[call-arg] assert not more From 646ee8616439b95a08f699252e5092189ac2f77a Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 19 Apr 2025 07:39:31 +0300 Subject: [PATCH 7/7] address review: save history *after* statement was executed --- Lib/_pyrepl/simple_interact.py | 6 +++--- Lib/test/test_pyrepl/test_pyrepl.py | 2 +- .../Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index 3d1616c257b27b..4c74466118ba97 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -142,13 +142,13 @@ def maybe_run_command(statement: str) -> bool: if maybe_run_command(statement): continue + input_name = f"" + more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single") # type: ignore[call-arg] + assert not more try: append_history_file() except (FileNotFoundError, PermissionError, OSError) as e: warnings.warn(f"failed to open the history file for writing: {e}") - input_name = f"" - more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single") # type: ignore[call-arg] - assert not more input_n += 1 except KeyboardInterrupt: r = _get_reader() diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index c0f9f8737515d1..4830f4b1083997 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1360,7 +1360,7 @@ def test_history_survive_crash(self): history = pathlib.Path(hfile.name).read_text() self.assertIn("spam", history) self.assertIn("time", history) - self.assertIn("sleep", history) + self.assertNotIn("sleep", history) self.assertNotIn("preved", history) def test_keyboard_interrupt_after_isearch(self): diff --git a/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst index 9762f550d563fb..135d0f651174ad 100644 --- a/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst +++ b/Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst @@ -1,3 +1,3 @@ -In PyREPL, append a new entry to the ``PYTHON_HISTORY`` file *before* every -statement was executed. This should preserve command-line history after -interpreter is terminated. Patch by Sergey B Kirpichev. +In PyREPL, append a new entry to the ``PYTHON_HISTORY`` file *after* every +statement. This should preserve command-line history after interpreter is +terminated. Patch by Sergey B Kirpichev.