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

Move the readline import out of the top (module) level and add Jedi completion #350

Merged
merged 4 commits into from
Mar 31, 2020

Conversation

asmeurer
Copy link
Collaborator

@asmeurer asmeurer commented Jul 9, 2019

This fixes #166. I have no idea why, but if readline is really libedit (like
for a system or python.org install on Mac), it causes the built in shell to
break the terminal inputs, but only when imported at the top level. If the
import is deferred, as far as I've seen, it doesn't happen.

CC @rofl0r

This fixes inducer#166. I have no idea why, but if readline is really libedit (like
for a system or python.org install on Mac), it causes the built in shell to
break the terminal inputs, but only when imported at the top level. If the
import is deferred, as far as I've seen, it doesn't happen.
@asmeurer
Copy link
Collaborator Author

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.

@inducer
Copy link
Owner

inducer commented Jul 9, 2019

Thanks for this PR. I'm mostly OK with this, but I have two comments:

  • I'd like it if a comment were added to the import with a link to this PR.
  • Wouldn't this still import readline from here via rlcompleter? (Once Tab is pressed?) Or is that harmless?

@asmeurer
Copy link
Collaborator Author

Ah you're right. If you C-x 1 TAB with this PR is still messes up.

This replaces rlcompleter, but it isn't a hard dependency. If we want to use
rlcompleter, we should extract the completer from it so we can use it without
importing readline (see inducer#350).
@asmeurer
Copy link
Collaborator Author

I've pushed a commit that uses jedi to do completion, if it is installed. We can also extract the rlcompleter completer and use it as a fallback if you like. I would try to do it without importing readline if possible, to avoid this stupid libedit issue. Unless someone knows of a way to detect libedit without importing readline (or if someone can figure out why importing libedit readline breaks urwid and how to fix it).

@asmeurer
Copy link
Collaborator Author

We can copy this if we want basic dir completion https://github.com/asmeurer/mypython/blob/master/mypython/dircompletion.py.

pudb/debugger.py Outdated Show resolved Hide resolved
pudb/debugger.py Show resolved Hide resolved
pudb/debugger.py Show resolved Hide resolved
@inducer
Copy link
Owner

inducer commented Jul 17, 2019

This looks good to me. Ready to merge when you are.

@asmeurer
Copy link
Collaborator Author

Let's see if the person who reported #352 or one of the other issues is able to test this first.

@asmeurer asmeurer changed the title Move the readline import out of the top (module) level Move the readline import out of the top (module) level and add Jedi completion Jan 17, 2020
@inducer
Copy link
Owner

inducer commented Mar 31, 2020

Based on all the feedback we've received, it seems like this might be a win. @asmeurer, thoughts?

@asmeurer
Copy link
Collaborator Author

Yeah I think we've had enough people confirm that this fixes this.

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.

Ctrl-x not function to leave console
2 participants