Skip to content
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

fix issue with libedit messing up terminal state #337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rofl0r
Copy link

@rofl0r rofl0r commented Apr 24, 2019

when switching to the internal shell and using tab completion, libedit messes
up the terminal state, unless it uses the readline initialization routine that
the "classic" shell code uses as well.
additionally to that, it requires that the console screen is stopped before
doing that for the first time.

fixes #336

when switching to the internal shell and using tab completion, libedit messes
up the terminal state, unless it uses the readline initialization routine that
the "classic" shell code uses as well.
additionally to that, it requires that the console screen is stopped before
doing that for the first time.

fixes inducer#336
@asmeurer
Copy link
Collaborator

asmeurer commented Jul 9, 2019

It seems the issue at #166 is also caused by libedit. I'm curious if you see that at all. I checked and this PR doesn't fix it.

To reproduce, type

Ctrl-x
1<ENTER>
Ctrl-x

If the issue occurs, it will enter ^X in the shell instead of leaving it, and all other keyboard input will be broken.

IMHO we should not use readline at all in the internal shell (it isn't used for keyboard input, is it?). Instead we should just implement basic completion natively. I guess we would also lose history support, although I'm not really sure about that because for me it doesn't persist any shell history between sessions. So maybe it's broken anyway (?)

OTOH actually using it for input would be nice. That would enable more readline bindings that don't work, like basic emacs-style word movement.

import pudb.shell as sh
histfile, ns = sh.readline_init(frame.f_globals, frame.f_locals)
ret = toggle_cmdline_focus(w, size, key)
sh.readline_fini(histfile)
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need to call readline_init/readline_fini here at all?

@inducer
Copy link
Owner

inducer commented Jul 9, 2019

OTOH actually using it for input would be nice.

Not very easy to integrate with Urwid though. Not impossible, but potentially a considerable headache.

IMHO we should not use readline at all in the internal shell (it isn't used for keyboard input, is it?).

The only connection between readline and the internal shell is that the internal shell uses the rlcompleter module, which imports readline.

@asmeurer
Copy link
Collaborator

asmeurer commented Jul 9, 2019

Apparently just importing readline at all causes the problem. I removed the rlcompleter stuff and it still happened just from importing readline. I don't know if we can detect if readline is libedit without actually importing it. Although I tried importing readline in the debugged script and it didn't reproduce the issue.

Also rlcompleter imports readline at import time. So if we want to use its completer, we would need to copy the Completer code from rlcompleter to pudb.

The only connection between readline and the internal shell is that the internal shell uses the rlcompleter module, which imports readline.

We supposedly use readline for shell history reading and writing too, but as I noted, it doesn't seem to work.

@inducer
Copy link
Owner

inducer commented Jul 9, 2019

we would need to copy the Completer code from rlcompleter to pudb.

Yuck. :/ I guess that's still a bit cleaner (as long as we keep the code unmodified) than some sys.modules hackery.

We supposedly use readline for shell history reading and writing too, but as I noted, it doesn't seem to work.

The built-in shell has a separate history mechanism though. Not sure it should be that way, but that's the intention at least. I don't think that mechanism persists history ATM, that should probably be fixed.

@asmeurer
Copy link
Collaborator

asmeurer commented Jul 9, 2019

Oh I was confusing run_classic_shell with the builtin shell. The explains a lot. I guess it was confusing me because those imports in shell.py are what affect this. I actually didn't touch the rlcompleter import in debugger.py at all. So maybe the issue only occurs if readline isn't imported right away (at the module level).

@asmeurer
Copy link
Collaborator

asmeurer commented Jul 9, 2019

Yeah, this fixes #166 (no idea about #336)

diff --git a/pudb/shell.py b/pudb/shell.py
index 4d5763c..8917728 100644
--- a/pudb/shell.py
+++ b/pudb/shell.py
@@ -26,14 +26,6 @@ else:
     HAVE_PTPYTHON = True


-try:
-    import readline
-    import rlcompleter
-    HAVE_READLINE = True
-except ImportError:
-    HAVE_READLINE = False
-
-
 # {{{ combined locals/globals dict

 class SetPropagatingDict(dict):
@@ -88,6 +80,13 @@ def run_classic_shell(globals, locals, first_time=[True]):
             get_save_config_path(),
             "shell-history")

+    try:
+        import readline
+        import rlcompleter
+        HAVE_READLINE = True
+    except ImportError:
+        HAVE_READLINE = False
+
     if HAVE_READLINE:
         readline.set_completer(
                 rlcompleter.Completer(ns).complete)

@asmeurer
Copy link
Collaborator

asmeurer commented Jul 9, 2019

I wonder if we can actually use the SetPropogatingDict in Python 2 #330. Supposedly it doesn't work to pass a dict subclass to exec in Python 2, so maybe not.

@rofl0r
Copy link
Author

rofl0r commented Jul 10, 2019

Also rlcompleter imports readline at import time. So if we want to use its completer, we would need to copy the Completer code from rlcompleter to pudb.

one should think there's a pure python module that handles autocompletion/history without resorting to terminal escape hackery...

Base automatically changed from master to main March 8, 2021 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tab completion in internal shell causes escape mode mess
3 participants