classification
Title: python program starting with unmatched quote spews spaces to stdout
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: benjamin.peterson, eric.smith, francismb, petri.lehtinen, python-dev, r.david.murray, skrah
Priority: normal Keywords: patch

Created on 2010-10-26 20:01 by r.david.murray, last changed 2011-06-24 17:30 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
issues10206_test.patch petri.lehtinen, 2011-06-23 07:36 Test case review
issue10206.patch francismb, 2011-06-24 15:49 test case review
issue10206v2.patch francismb, 2011-06-24 16:21 test case review
Messages (16)
msg119646 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-10-26 20:01
To reproduce:

  python -c "'"

(Hint: don't do this on a slow terminal)
msg119723 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-10-27 17:46
This is caused by r85814. I've got a fix, as soon as I get it in a presentable state I'll post it.
msg119772 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-10-28 11:43
This patch fixes the issue, and makes print_error_text slightly more understandable (and efficient to boot, not that it matters).

But I'm not convinced it's the correct solution. I think the real error might be the computation "offset" in the caller. I'm still looking at it.
msg119773 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-10-28 11:46
Now that I think about it some more, I think my patch (although it fixes this issue and passes all tests) doesn't do what the original code does in the presence of multiple newlines. I'm going to rework it some more. I'll have another patch shortly.
msg119900 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-10-29 13:15
I think Benjamin fixed this in r85904. I'm going to verify and add some tests.
msg138852 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-06-23 06:28
I'm unable to reproduce this. I checked out the commit 65614:18989ad44636 (corresponding to r85814, right?), built and ran python -c "'", but didn't get a space flood on my face. Just a normal SyntaxError.
msg138853 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2011-06-23 06:55
I remember that I could reproduce it at the time. The issue was indeed
fixed in r85904.
msg138854 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-06-23 06:58
By checking out the parent of r85904 I now can reproduce this.
msg138859 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-06-23 07:36
Attached a test case. The patch is against the current default tip.
msg138884 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-24 01:10
Hmm.  I don't know that it is really necessary to cater to the particular failure mode, I was more interested in seeing a unit test that checked the correct behavior: that a syntax error is raised (by capturing the output using the tools in script_helper).  If we do keep the timeout, comments explaining why would be a good idea, because it is completely un-obvious why the test is doing what it is doing...
msg138955 - (view) Author: Francis MB (francismb) * Date: 2011-06-24 15:49
I've attached an alternative test case.

I'm not sure if there is a more robust way to test:
self.assert_('SyntaxError' in err.decode('ascii', 'ignore'))

Due the use of 'SyntaxtError' directly as string. I would prefer something like str(SyntaxtError) (or just the name of the exception
without extra text)

Review is welcome
msg138957 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-24 16:07
I think that self.assertRegex(err.decode('ascii', 'ignore'), 'SyntaxError') would be fine.  We are extremely unlikely to change the string representation of the name of SyntaxError or omit it from the error message, so I think this is a reliable test.  If you really want to be paranoid, you could use SyntaxError.__name__ as the argument to assertRegex, but there are a number of other places in the test suite where we hardcode the exception names, so I don't think it is necessary.
msg138960 - (view) Author: Francis MB (francismb) * Date: 2011-06-24 16:21
Just attaching the review patch
msg138961 - (view) Author: Francis MB (francismb) * Date: 2011-06-24 16:30
On 06/24/2011 06:07 PM, R. David Murray wrote:
> self.assertRegex(err.decode('ascii', 'ignore'), 'SyntaxError')

I understand that's the standard way to check if a given failure 
happened in the command line or there is also a helper for that case 
(maybe something: assert_python_raises('-c', "'", SyntaxError))

Thanks !

Francisco
msg138967 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-06-24 17:28
New changeset 2a4764376c51 by R David Murray in branch '3.2':
#10206: add test for previously fixed bug.
http://hg.python.org/cpython/rev/2a4764376c51

New changeset 5ec95f46bac5 by R David Murray in branch 'default':
Merge #10206: add test for previously fixed bug.
http://hg.python.org/cpython/rev/5ec95f46bac5
msg138968 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-24 17:30
Thanks, Petri and Francisco.  Eric, I'm closing this since we now have a minimal test.  If you still want to go back and add more tests based on a deeper understanding of what was broken, feel free to reopen the issue.
History
Date User Action Args
2011-06-24 17:30:00r.david.murraysetstatus: open -> closed
versions: + Python 3.3
messages: + msg138968

resolution: fixed
stage: test needed -> resolved
2011-06-24 17:28:35python-devsetnosy: + python-dev
messages: + msg138967
2011-06-24 16:30:47francismbsetmessages: + msg138961
2011-06-24 16:21:31francismbsetfiles: + issue10206v2.patch

messages: + msg138960
2011-06-24 16:07:21r.david.murraysetmessages: + msg138957
2011-06-24 15:49:32francismbsetfiles: + issue10206.patch
nosy: + francismb
messages: + msg138955

2011-06-24 01:10:13r.david.murraysetmessages: + msg138884
2011-06-23 07:36:38petri.lehtinensetfiles: + issues10206_test.patch

messages: + msg138859
2011-06-23 06:58:45petri.lehtinensetmessages: + msg138854
2011-06-23 06:55:07skrahsetnosy: + skrah
messages: + msg138853
2011-06-23 06:28:47petri.lehtinensetnosy: + petri.lehtinen
messages: + msg138852
2010-11-02 19:47:23eric.araujosetnosy: + benjamin.peterson
2010-10-29 13:16:57eric.smithsetfiles: - issue10206.diff
2010-10-29 13:15:50eric.smithsetpriority: critical -> normal

messages: + msg119900
stage: patch review -> test needed
2010-10-28 11:46:19eric.smithsetmessages: + msg119773
2010-10-28 11:43:54eric.smithsetfiles: + issue10206.diff
messages: + msg119772

assignee: eric.smith
components: + Interpreter Core
keywords: + patch
stage: needs patch -> patch review
2010-10-27 17:46:06eric.smithsetnosy: + eric.smith
messages: + msg119723
2010-10-26 20:01:27r.david.murraycreate