Skip to content

gh-122273: Support PyREPL history in Windows #122274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from

Conversation

devdanzin
Copy link
Contributor

@devdanzin devdanzin commented Jul 25, 2024

This PR aims to add support for recording and loading command history in Windows, using _pyrepl.readline to support handling the history file.

@devdanzin devdanzin marked this pull request as ready for review July 25, 2024 19:39
@JeffersGlass
Copy link
Contributor

Gave this a try, and it does appear to work as expected, at least on a stock Windows 10 build:

image

@vstinner
Copy link
Member

vstinner commented Oct 9, 2024

@devdanzin: There is now a merge conflict after my latest site change. Would you mind to update your PR?

Lib/site.py Outdated
import readline
real_readline = True
except ImportError:
import _pyrepl.readline as readline
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not import if PYTHON_BASIC_REPL is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part replaces an unconditional import of readline:

 import atexit
     try:
-        import readline
+        try:
+            import readline
+            real_readline = True
+        except ImportError:
+            import _pyrepl.readline as readline
+            real_readline = False
         import rlcompleter  # noqa: F401

Then readline is used in a couple places without checking it's a valid module. Should I set readline = None for the PYTHON_BASIC_REPL case and check against it being falsy where needed?

Should look something like:

diff --git a/Lib/site.py b/Lib/site.py
index 9d3352c70e3..a27373000fa 100644
--- a/Lib/site.py
+++ b/Lib/site.py
@@ -502,12 +502,13 @@ def register_readline():
             import readline
             real_readline = True
         except ImportError:
-            import _pyrepl.readline as readline
+            readline = None
             real_readline = False
         import rlcompleter  # noqa: F401
         if PYTHON_BASIC_REPL:
             CAN_USE_PYREPL = False
         else:
+            import _pyrepl.readline as readline
             from _pyrepl.main import CAN_USE_PYREPL
             if real_readline:
                 import _pyrepl.unix_console
@@ -521,9 +522,10 @@ def register_readline():
 
     # Reading the initialization (config) file may not be enough to set a
     # completion key, so we set one first and then read the file.
-    if getattr(readline, "backend", None) == 'editline':
+    backend = getattr(readline, "backend", None)
+    if backend == 'editline':
         readline.parse_and_bind('bind ^I rl_complete')
-    else:
+    elif backend:
         readline.parse_and_bind('tab: complete')
 
     try:
@@ -536,7 +538,7 @@ def register_readline():
         # want to ignore the exception.
         pass
 
-    if readline.get_current_history_length() == 0:
+    if readline and readline.get_current_history_length() == 0:
         # If no history was loaded, default to .python_history,
         # or PYTHON_HISTORY.
         # The guard is necessary to avoid doubling history size at
@@ -553,13 +555,15 @@ def register_readline():
             exceptions = OSError
 
         try:
-            readline_module.read_history_file(history)
+            if readline:
+                readline_module.read_history_file(history)
         except exceptions:
             pass
 
         def write_history():
             try:
-                readline_module.write_history_file(history)
+                if readline:
+                    readline_module.write_history_file(history)
             except (FileNotFoundError, PermissionError):
                 # home directory does not exist or is not writable
                 # https://bugs.python.org/issue19891

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've committed a version of the approach above.

@vstinner
Copy link
Member

I'm not fully satisfied by proposed change, so I proposed a PR based somehow on this one: PR gh-127141.

@devdanzin
Copy link
Contributor Author

Closing in favor of PR #127141, which does it better.

@devdanzin devdanzin closed this Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants