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

Don't use SetPropagatingDict in the built in console #236

Merged
merged 2 commits into from
Apr 4, 2017

Conversation

asmeurer
Copy link
Collaborator

@asmeurer asmeurer commented Apr 4, 2017

Passing a dict subclass to eval is not supported in Python 2. This was causing
weird issues where the C-x would break the UI (I am not clear exactly how it
caused those issues).

The point of SetPropagatingDict is to combine globals and locals into a single
dictionary for things like code.InteractiveConsole and rlcompleter.Completer,
which take a single dictionary, but to allow changes to propagate, so that
assignments in the shell can modify global names. However, this is completely
unnecessary for the built in shell, as we call eval() ourselves, so we can
just pass the exact globals and locals dicts separately.

This fixes #166.

Passing a dict subclass to eval is not supported in Python 2. This was causing
weird issues where the C-x would break the UI (I am not clear exactly how it
caused those issues).

The point of SetPropagatingDict is to combine globals and locals into a single
dictionary for things like code.InteractiveConsole and rlcompleter.Completer,
which take a single dictionary, but to allow changes to propagate, so that
assignments in the shell can modify global names. However, this is completely
unnecessary for the built in shell, as we call eval() ourselves, so we can
just pass the exact globals and locals dicts separately.

This fixes inducer#166.
@asmeurer
Copy link
Collaborator Author

asmeurer commented Apr 4, 2017

I'll also add some comments on SetPropagatingDict so that we can know what it is for in the future.

@inducer
Copy link
Owner

inducer commented Apr 4, 2017

I'll also add some comments on SetPropagatingDict so that we can know what it is for in the future.

Awesome, thx!

@asmeurer
Copy link
Collaborator Author

asmeurer commented Apr 4, 2017

Another point of confusion. The usage everywhere is SetPropagatingDict([locals, globals], locals), meaning assignments are supposed to go to locals. But assigning to locals doesn't work. The only reason it works is that at the module level, locals() is globals(). I suppose this is the desired behavior, though, assignments in the shell should work if and only if we are at the module level. It definitely needs some clarification in the code, however.

@inducer inducer merged commit 55ef2e2 into inducer:master Apr 4, 2017
@inducer
Copy link
Owner

inducer commented Apr 4, 2017

Thx!

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