inducer / pudb

Full-screen console debugger for Python
https://documen.tician.de/pudb/
Other
3k stars 230 forks source link

fix issue with libedit messing up terminal state #337

Open rofl0r opened 5 years ago

rofl0r commented 5 years ago

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

asmeurer commented 5 years ago

It seems the issue at https://github.com/inducer/pudb/issues/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.

inducer commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

I wonder if we can actually use the SetPropogatingDict in Python 2 https://github.com/inducer/pudb/pull/330. Supposedly it doesn't work to pass a dict subclass to exec in Python 2, so maybe not.

rofl0r commented 5 years ago

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...