[issue25677] Syntax error caret confused by indentation

[Donald Stufft]

Changes by Donald Stufft :


--
pull_requests: +1080

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Martin Panter]

Changes by Martin Panter :


--
resolution:  -> fixed
stage: commit review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Roundup Robot]

Roundup Robot added the comment:

New changeset 35b50c26f780 by Martin Panter in branch '3.5':
Issue #25677: Correct syntax error caret for indented blocks.
https://hg.python.org/cpython/rev/35b50c26f780

New changeset d4effdd699de by Martin Panter in branch '3.6':
Issue #25677: Merge SyntaxError caret positioning from 3.5
https://hg.python.org/cpython/rev/d4effdd699de

New changeset 2342726ea0c0 by Martin Panter in branch 'default':
Issue #25677: Merge SyntaxError caret positioning from 3.6
https://hg.python.org/cpython/rev/2342726ea0c0

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

LGTM.

--
assignee: serhiy.storchaka -> martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Martin Panter]

Martin Panter added the comment:

Patch v6 strips the form feed, and adds an extra test.

--
Added file: http://bugs.python.org/file45844/cpython25677.v6.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Martin Panter]

Martin Panter added the comment:

(Long story short: need to strip form feeds in print_error_text(), but I agree 
that this otherwise does fix a bug.)

There is one minor regression that this patch causes IMO. Given the following 
file, where  represents a form feed character ('\014')

if True:
1 + 1 = 2

the error report now has a blank line (due to outputting the FF character) and 
extra indentation that wasn’t stripped:

  File "indent.py", line 2

1 + 1 = 2
^
SyntaxError: can't assign to operator

I think the fix for this is to strip form feed characters in 
print_error_text(), in Python/pythonrun.c.

Apart from that, I agree with Serhiy and Michael that it is okay to push this 
change. The bug that we are fixing is that SyntaxError.offset does not 
correspond with SyntaxError.text, due to indentation. Before the patch, the 
offset refers to the line without indentation removed, but the text has 
indentation removed. Here is a really bad case, where there is more indentation 
than code, print_error_text() tries to find the next line, and ends up printing 
a blank line:

>>> code = "if True:\n" + " "*16 + "1 + 1 = 2\n"
>>> with open("indent.py", "wt") as f: f.write(code)
... 
35
>>> try: compile(code, "indent.py", "exec")
... except SyntaxError as err:
... print("offset={err.offset} text={err.text!r}".format(err=err))
... raise
... 
offset=16 text='1 + 1 = 2\n'
Traceback (most recent call last):
  File "", line 1, in 
  File "indent.py", line 2

 ^
SyntaxError: can't assign to operator

In Michael’s original report, I was curious why the caret is only shifted three 
spaces, despite there being four spaces of indentation. This is because offset 
points to the start of the line, but the caret points to the character _before_ 
offset. So offset=0 and offset=1 are both represented by pointing at the first 
character on the line. This is related to Serhiy’s bug with inserting “1;”. In 
some cases (e.g. the line “1 +”), the offset is the string index _after_ the 
error. But in the case of “1;1 + 1 = 2”, offset is the index where the error 
(statement) begins.

Both pieces of indentation stripping code were added in revision 27f04d714ecb 
(year 2001). It is strange that only one stripped form feeds though. The column 
offset was only added later (revision 861c35cef7aa). So Python 2 will have some 
of its SyntaxError.text lines stripped of indentation, but it does not matter 
so much because SyntaxError.offset is not set in those cases.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

This may be just bad example that it looks contradictorily to you. Is the 
example "None = 0" looks good to you? Without patch the caret is located at the 
middle of None (depending on the size of the indentation of this line). With 
patch applied it is located at the start of None independently of the 
indentation.

The original bug is that the location of the caret depends on the indentation. 
The patch fixes this bug (and only this bug).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Berker Peksag]

Berker Peksag added the comment:

With patch applied:

  File "x.py", line 2
1 + 1 = 2
^
SyntaxError: can't assign to operator

Without patch:

  File "x.py", line 2
1 + 1 = 2
   ^
SyntaxError: can't assign to operator

The caret is located at the wrong place in both examples (which is the actual 
bug that needs to be fixed here.) This isn't a high priority bug and I think 
people can live with the current behavior until this is properly fixed. Pushing 
a premature patch will only introduce more code churn.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[STINNER Victor]

Changes by STINNER Victor :


--
nosy:  -haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The patch fixes one bug, and I'm going to push it.

There is other bug here. If add "1;" before the original example (so the line 
becomes "1;1 + 1 = 2"), the caret will be under ";". In some SyntaxErrors the 
caret points one position left before the start of incorrect expression. But 
this bug deserves separate issue.

The fact that in "1 + 1 = 2" the caret points to the first "1" instead of "=" 
can be considered as yet one different bug (but this is questionable).

--
assignee:  -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

Michael Layzell added the comment:

This patch fixes a problem, which is that the caret was not placed where the 
source location information for the node is, but instead in a random position 
based on the indentation level.

The problem that the caret is not placed under the `=` sign, but instead at the 
front of the expression, feels like a different bug related to how source 
location information is computed throughout cpython for infix operators. I 
don't think it should be fixed in this bug, but rather in a follow-up.

Specifically, this fix (to make the printing actually respect the source 
location information) will also be required in order to fix the placement of 
the caret to under the = sign.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Berker Peksag]

Berker Peksag added the comment:

FWIW, I don't think the patch fixes the actual problem. With the patch applied, 
the caret still placed at the wrong place. I'd prefer avoid pushing a patch 
that fixes a bug by introducing another bug.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

Michael Layzell added the comment:

Is there something I should be doing to push this patch forward? I am not 
familiar with the contribution process for cpython unfortunately.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

LGTM.

--
stage: patch review -> commit review
versions: +Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Chi Hsuan Yen]

Changes by Chi Hsuan Yen :


--
nosy: +Chi Hsuan Yen

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

Michael Layzell added the comment:

Oops. Should be fixed now.

--
Added file: http://bugs.python.org/file41559/cpython25677.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Martin Panter]

Martin Panter added the comment:

+script = textwrap.dedent("1 + 1 = 2\n")
You don’t need that first textwrap.dedent() call. It is only useful if the 
string has extra indentation at the start, as in the second multiline one.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[R. David Murray]

Changes by R. David Murray :


--
nosy:  -r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

Michael Layzell added the comment:

Sorry for missing the review comments earlier, I didn't notice them (I've not 
used this bug tracker before - sorry).

I've attached an updated patch which should not have the variable problem, and 
also takes some of the other review comments into account.

Berker: I agree that the current positioning of the caret is undesirable, and 
that ideally we would report the error at the site of the operator rather than 
the first character of the expression. I think that changing this behavior is a 
different bug though.

P.S: Sorry for disappearing for a month, I don't seem to get emails when there 
are changes made to the bug, and thus forgot to check up on the bug...

--
Added file: http://bugs.python.org/file41539/cpython25677.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Berker Peksag]

Berker Peksag added the comment:

With the latest patch applied, the output is

  File "z.py", line 2
1 + 2 = 3
^
SyntaxError: can't assign to operator

I think the caret is still in wrong place.

I would expect the following output:

  File "z.py", line 2
1 + 2 = 3
  ^
SyntaxError: can't assign to operator

--
nosy: +berker.peksag

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Martin Panter]

Changes by Martin Panter :


--
stage: test needed -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Martin Panter]

Martin Panter added the comment:

Michael, see the review comments if you already haven’t. There is an unused 
variable “p” in the last two patches.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

Michael Layzell added the comment:

Martin's solution seemed like the least work, so that's what I ended up using. 
I also switched over to assertRegex, and I agree that it produces better error 
messages.

I added a comment to explain the use of the ugly multiline regex, and removed 
some of its unnecessary parts.

--
Added file: http://bugs.python.org/file41169/cpython25677.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Martin Panter]

Martin Panter added the comment:

Another way to get universal newlines I have used in other cases is to use 
TextIOWrapper. It might be easier if you really need that multi-line RE search:

text = TextIOWrapper(BytesIO(stderr), "ascii").read()

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[R. David Murray]

R. David Murray added the comment:

Two options on portability, I think: either use splitlines() on the decoded 
result, or add a 'universal_newlines' keyword argument to assert_python_failure 
and friends that would get passed to Popen.  I'd favor the latter, as other 
users of script_helpers may find it useful.  On the other hand, that may 
involve a whole separate discussion, so you could use splitlines here and we 
could open a new issue for a script_helpers enhancement :)

Also, rather than a direct re, I'd use assertRegex.  That will give a more 
useful error message if the test fails.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

Michael Layzell added the comment:

Sorry for the delay, I finally got around to adding a test.

I'm mildly concerned about the portability of the check, but it seems to work 
locally. If people have suggestions about how to make the check more portable, 
let me know!

--
Added file: http://bugs.python.org/file41163/cpython25677.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Would be nice to have a test.

--
stage: patch review -> test needed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[R. David Murray]

Changes by R. David Murray :


--
nosy: +haypo, serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[R. David Murray]

Changes by R. David Murray :


--
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

Michael Layzell added the comment:

This should fix the problem. It appears as though the indentation was being 
stripped from the program text differently depending on how the error was 
produced. In the case of a syntax error from the parser, the indentation was 
maintained until it was printed (in print_error_text, correctly handling the 
offset value). In contrast, errors from the AST were created with 
PyErr_ProgramText*, which would also strip the indentation (without mutating 
any offset value). 

This patch causes the second code path to not strip whitespace. This shouldn't 
cause I problem (I think), as it seems to only be used to instantiate 
SyntaxErrors.

--
keywords: +patch
Added file: http://bugs.python.org/file41100/cpython25677.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[R. David Murray]

R. David Murray added the comment:

By the way, python3.4 also shows the behavior.  Python2 does not, because 
python2 does not print the caret.  The bug presumably crept in when the caret 
was added.  I'm marking this as easy...presumably the hardest part of fixing it 
is figuring out exactly how the message is constructed :)

--
keywords: +easy
stage:  -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[R. David Murray]

R. David Murray added the comment:

It appears as though the line is always printed without the leading whitespace, 
but the caret takes the whitespace into account (try indenting the line even 
further).  So the problem is that the caret and the printing of the source line 
in the error message are not consistent about how whitespace is handled.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25677] Syntax error caret confused by indentation

[Michael Layzell]

New submission from Michael Layzell:

It appears that syntax errors generated by checking the AST will get confused 
by indentation and place the caret incorrectly.

With no indentation:
===
1 + 1 = 2
===
  File "/Users/mlayzell/test.py", line 1
1 + 1 = 2
^
SyntaxError: can't assign to operator
===

With 4 spaces of indentation:
===
if True:
1 + 1 = 2
===
  File "/Users/mlayzell/test.py", line 2
1 + 1 = 2
   ^
SyntaxError: can't assign to operator
===

As you can see, the caret appears to be placed randomly in the middle of the 
second statement, apparently offset (probably by the 4 space indentation).

The above examples were run with Python3.5 on Mac OS X 10.10.4, though my 
somewhat-recent trunk clone exhibits the same problems.

--
components: Interpreter Core
messages: 254951
nosy: Michael Layzell
priority: normal
severity: normal
status: open
title: Syntax error caret confused by indentation
type: behavior
versions: Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com