bpo-30377: Simplify handling of COMMENT and NL in tokenize.py #1607
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
|
@albertjan, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tiran, @serhiy-storchaka and @1st1 to be potential reviewers. |
put both new lines and comments back into one branch and fall though to adding the newline if it's a comment.
|
Due to a new release of Sphinx, we had to fix the documentation to build on Travis again. Please do a merge to get these changes to help get Travis passing on your PR. |
|
Apparently @the-knights-who-say-ni don't agree with bpo 😄 as bpo now says that I've signed the CLA. |
|
Finally we convinced him. 😄 |
| COMMENT '# NEWLINE' (3, 17) (3, 26) | ||
| NEWLINE '\\n' (3, 26) (3, 27) | ||
| DEDENT '' (4, 0) (4, 0) | ||
| NL '\\n' (3, 4) (3, 5) |
There was a problem hiding this comment.
@serhiy-storchaka, this looks invalid to me. Why are there two newlines now? Now the tokenizer claims there's 5 lines in the snippet. There's 4 lines in the snippet.
There was a problem hiding this comment.
Because there are two lines between the comment an "True". One ends the comment, and other ends the blank line (spaces are ignored).
Look closer on line numbers. "if" is at line 1, the comment is at line 2, "True" is at line 4. Previously the tokenizer reported "True" at line 3.
The fifth line contains only DEDENT. 4 newlines divide the whole text on 5 parts. If you write the sample text in a file and open it with a text editor for sure you could move the cursor on the line 5, just after the 4th newline.
There was a problem hiding this comment.
Yes, I was confused by the DEDENT. Thank you for clarification!
There was a problem hiding this comment.
I was confused myself too. It looked to me that this change also fixed a bug in line numbering, but I was not able to find a bug in the old code. Later I figured out that the input data for this test was changed, therefore both old and new codes are correct. This PR just cleans up the code, as was claimed.
While porting tokenize.py to javascript for skulpt I ran into this bit of code. It check's if a line is a comment more then necessary, this addresses it and makes the code a bit more readable.
Should I also do a PR for this agains 2.7?