Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(12)

#16806: col_offset is -1 for multiline string expressions resembling docstrings

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years ago by carsten.klein
Modified:
7 years ago
Reviewers:
benjamin
CC:
terry.reedy, haypo, Benjamin Peterson, inada.naoki, carsten.klein, Aivar.Annamaa, ivailo_karamanolev.com, asottile, Anthony Sottile, pablogsal
Visibility:
Public.

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_ast.py View 1 chunk +19 lines, -0 lines 1 comment Download
Parser/parsetok.c View 2 chunks +12 lines, -3 lines 2 comments Download
Parser/tokenizer.c View 1 chunk +7 lines, -0 lines 0 comments Download
Parser/tokenizer.h View 2 chunks +5 lines, -0 lines 2 comments Download

Messages

Total messages: 1
Benjamin Peterson
7 years ago #1
The patch looks good; I just have a few nits.

http://bugs.python.org/review/16806/diff/6932/Lib/test/test_ast.py
File Lib/test/test_ast.py (right):

http://bugs.python.org/review/16806/diff/6932/Lib/test/test_ast.py#newcode510
Lib/test/test_ast.py:510: def
test_multi_line_docstring_col_offset_and_lineno_issue16806(self):
Please put this in the AST_Tests class.

http://bugs.python.org/review/16806/diff/6932/Parser/parsetok.c
File Parser/parsetok.c (right):

http://bugs.python.org/review/16806/diff/6932/Parser/parsetok.c#newcode213
Parser/parsetok.c:213: (cf. issue 16806) */
Please explain what about their handling is different rather than just saying
that it is.

http://bugs.python.org/review/16806/diff/6932/Parser/parsetok.c#newcode215
Parser/parsetok.c:215: line_start = type == STRING ? tok->multi_line_start :
tok->line_start;
Split these statements into if (type == STRING) .. else ... blocks.

http://bugs.python.org/review/16806/diff/6932/Parser/tokenizer.h
File Parser/tokenizer.h (right):

http://bugs.python.org/review/16806/diff/6932/Parser/tokenizer.h#newcode40
Parser/tokenizer.h:40: int first_lineno;   /* First line of a single line or
multi line string
It seems this would be more appropriately called string_literal_lineno or
something referring to the fact that it's used for strings.

http://bugs.python.org/review/16806/diff/6932/Parser/tokenizer.h#newcode63
Parser/tokenizer.h:63: const char* multi_line_start; /* pointer to start of
first line of
Same here.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+