classification
Title: Allow multiple statements in code.InteractiveConsole.push
Type: enhancement Stage: test needed
Components: Library (Lib) Versions: Python 3.4
process
Status: open Resolution:
Dependencies: 16649 Superseder:
Assigned To: Nosy List: aliles, asvetlov, eric.araujo, kristjan.jonsson, ncoghlan
Priority: low Keywords: needs review, patch

Created on 2010-01-19 14:52 by kristjan.jonsson, last changed 2012-12-09 22:47 by cjw296.

Files
File name Uploaded Description Edit
code.patch kristjan.jonsson, 2010-01-19 14:51 patch to code.py review
issue7741_x.patch aliles, 2012-08-21 00:15 Incorporate Kristjan's patch with tests review
issue7741_y.patch aliles, 2012-08-21 00:18 review
Messages (15)
msg98056 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-01-19 14:51
The code.InteractiveConsole() is useful to emulate a python console.  However, any code currently "push"ed to it must be single statements.  This is because it passes the ´single´ symbol mode to the underlying compile function.
This patch allows the caller of InteractiveConsole.push to specify a different mode, e.g. ´exec´.  This is useful if one wants to paste entire code snippets into the console.  Without it, pasting the following:
'if True:\n  print 1\nprint 2' Won't run.  pushing such multiline code snippets with an additional 'exec' argument will allow it to work.
Patch included.
msg98057 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-01-19 15:06
Note, there are no regression tests for the code module.
msg98058 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-01-19 15:09
Here is how to test this manually:
from code import InteractiveConsole
c = InteractiveConsole()
s = "if True:\n  print 1\nprint 2"
c.push(s)  #fails
c.push(s, "exec")  #succeeds
msg98182 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2010-01-23 12:24
Please can you starts a small test suite for the code module that tests the fix you are proposing and include it as another patch?
msg168719 - (view) Author: Aaron Iles (aliles) * Date: 2012-08-21 00:15
Patch option 1 of 2.

Incorporates Kristjan's patch and adds unit tests. This has the side effect of changing InteractiveConsole's behaviour with respect to displayhook(). I'm unsure if this is desirable.
msg168720 - (view) Author: Aaron Iles (aliles) * Date: 2012-08-21 00:18
Patch option 2 of 2.

Alternative patch that adds a new method to InteractiveConsole to split the string into multiple lines, feeding each line to interpreter using push().

This doesn't change the behaviour regarding the displayhook. But this may not meet Kristjan's original goals.
msg168721 - (view) Author: Aaron Iles (aliles) * Date: 2012-08-21 00:21
A quick note regarding the last two patches submitted. These patches add unit tests using the test suite added for Issue #12643. This limits the patches suitable to Python 3.3 and up.
msg168844 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-08-22 06:51
What's the purpose of the new patch, particularly 2/2 since it is equivalent to multiple push() calls?
I.e. since this issue has laid dormant for two years, what prompts the sudden activity?
msg168848 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-08-22 07:45
Aaron was looking for something to work on for the PyConAU sprints, and set himself the task of closing as many issues related to the code module as possible.

The main outcome of that was the new test suite added in #12643, which should make it easier to work on the module (at least in 3.3+)
msg168868 - (view) Author: Aaron Iles (aliles) * Date: 2012-08-22 11:27
I agree that the second patch adds little value to InteractiveConsole.

A third alternative would be to accept default grammar start symbol to be passed to __init__(). Although this would prevent mixing use of 'single' with 'exec'.
msg177160 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-12-08 15:19
OK, after a long detour that delved deep into codeop and the vagaries of PyCF_DONT_IMPLY_DEDENT (due to a problem that turned out to be due to a missing "\n" in a test case I added), my main conclusion is:

Coupling the "single vs multiple statement" decision to the "implicit print after every call" decision is *really* annoying. The latter should be its own flag *or else* also implied by the "DONT_IMPLY_DEDENT" flag that is already passed to the compiler by codeop.

If *that* gets fixed, then the code module could simply switch over to compiling in exec mode always, without any side effects on the implicit display of expression results.
msg177185 - (view) Author: Aaron Iles (aliles) * Date: 2012-12-08 21:44
Should a new issue be created to decouple "print after every call" from the
single vs multiple statement condition that is a blocker for this issue? Or
can it be resolved here?

On Sunday, 9 December 2012, Nick Coghlan wrote:

>
> Nick Coghlan added the comment:
>
> OK, after a long detour that delved deep into codeop and the vagaries of
> PyCF_DONT_IMPLY_DEDENT (due to a problem that turned out to be due to a
> missing "\n" in a test case I added), my main conclusion is:
>
> Coupling the "single vs multiple statement" decision to the "implicit
> print after every call" decision is *really* annoying. The latter should be
> its own flag *or else* also implied by the "DONT_IMPLY_DEDENT" flag that is
> already passed to the compiler by codeop.
>
> If *that* gets fixed, then the code module could simply switch over to
> compiling in exec mode always, without any side effects on the implicit
> display of expression results.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org <javascript:;>>
> <http://bugs.python.org/issue7741>
> _______________________________________
>
msg177200 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-12-09 06:37
More implicit magic seems like a bad idea, so I've split out a proposal for an explicit PyCF_DISPLAY_EXPRESSION_RESULTS flag as #16649.

The behaviour would then be selectable regardless of the compilation mode, but would remain the default for "single".
msg177205 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-12-09 09:30
Sounds fine.
Just a note to my original intent in #7741:
We were using the InteractiveConsole class to implement a remote web-based console for our EVE servers.  Often, as a means to hot-fix certain issues, we would paste code snippets into these windows to define functions, execute code, etc.  Previously we had our own console like implementation, but the interactive features of the InteractiveConsole were _really_ nice, but lacking in multi-line support.  We have had the stdlib patched as per my original suggestion for the past few years to support it.
msg177210 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-12-09 11:42
Good to know - I guess in most circumstances copy-and-paste already works, because the input will be arriving via a line-buffered IO stream.

I was thinking that with #16649 implemented, it would be possible to simply switch from "single" to "exec", without users needing to request the multi-statement support explicitly. However, I'm now back to wondering if such a change might have a few unforeseen consequences I haven't thought of.

So if that seems like too much of a risk to backwards compatibility, how about moving the "symbol" argument to __init__, rather than needing to supply it with each call to push?
History
Date User Action Args
2012-12-09 22:47:37cjw296setnosy: - cjw296
2012-12-09 11:42:50ncoghlansetmessages: + msg177210
2012-12-09 09:30:50kristjan.jonssonsetmessages: + msg177205
2012-12-09 06:37:41ncoghlansetpriority: normal -> low

dependencies: + Add a PyCF_DISPLAY_EXPRESSION_RESULTS flag
messages: + msg177200
versions: + Python 3.4, - Python 2.7
2012-12-08 21:44:56alilessetmessages: + msg177185
2012-12-08 15:19:40ncoghlansetmessages: + msg177160
2012-08-22 11:27:55alilessetmessages: + msg168868
2012-08-22 11:12:45asvetlovsetnosy: + asvetlov
2012-08-22 07:45:24ncoghlansetmessages: + msg168848
2012-08-22 06:51:56kristjan.jonssonsetmessages: + msg168844
2012-08-21 00:21:37alilessetnosy: + ncoghlan
messages: + msg168721
2012-08-21 00:18:14alilessetfiles: + issue7741_y.patch

messages: + msg168720
2012-08-21 00:15:36alilessetfiles: + issue7741_x.patch
nosy: + aliles
messages: + msg168719

2010-08-16 17:50:37eric.araujosetnosy: + eric.araujo
2010-01-23 12:24:09cjw296setnosy: + cjw296
messages: + msg98182
2010-01-19 15:09:06kristjan.jonssonsetmessages: + msg98058
2010-01-19 15:06:34kristjan.jonssonsetmessages: + msg98057
2010-01-19 14:58:57brian.curtinsetpriority: normal
keywords: + needs review
stage: test needed
2010-01-19 14:52:01kristjan.jonssoncreate