Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-23894: Make rb'' strings work in lib2to3 #1724

Merged
merged 1 commit into from
May 22, 2017
Merged

Conversation

ambv
Copy link
Contributor

@ambv ambv commented May 22, 2017

This partially solves bpo-23894. A separate PR coming later to support f-strings.

@ambv ambv added type-bug An unexpected behavior, bug, or error needs backport to 3.6 labels May 22, 2017
@ambv ambv requested a review from ned-deily May 22, 2017 18:46
@ambv ambv changed the title Make rb'' strings work in lib2to3 bpo-23894: Make rb'' strings work in lib2to3 May 22, 2017
@ambv ambv force-pushed the bpo-23894-rb branch 2 times, most recently from 76b0e9d to d330004 Compare May 22, 2017 19:04
# Single-line ' or " string.
String = group(r"[uU]?[rR]?'[^\n'\\]*(?:\\.[^\n'\\]*)*'",
r'[uU]?[rR]?"[^\n"\\]*(?:\\.[^\n"\\]*)*"')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"String" didn't previously include b but this is suspicious to me. Why wouldn't it? That way b'' literals were only PseudoTokens (via ContStr), not actual Tokens (via String).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

Copy link
Member

@ned-deily ned-deily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the test: LGTM. As far as the tokenizer changes, I'm not familiar enough with lib2to3 to provide a useful review. Assuming other reviewers have no objections, I'm fine for this going into 3.6.2rc.

@ambv
Copy link
Contributor Author

ambv commented May 22, 2017

Tests are failing because I used an f-string in the test and lib2to3 is parsing that test file as part of fixers. f-strings are not supported yet in lib2to3. Switching to using a classic str.format fixes it. I'm updating the PR now.

UPDATE: as expected, tests pass now.

This partially solves bpo-23894.
@ambv ambv requested review from vstinner and gvanrossum May 22, 2017 19:42
@ambv ambv merged commit 0c4aca5 into python:master May 22, 2017
ambv added a commit to ambv/cpython that referenced this pull request May 22, 2017
This partially solves bpo-23894.
(cherry picked from commit 0c4aca5)
ambv added a commit that referenced this pull request May 22, 2017
This partially solves bpo-23894.
(cherry picked from commit 0c4aca5)
@vstinner
Copy link
Member

I was requested for a review, but after I completed my review, I see that the change was already merged :-D It's fine, don't worry. I just want to give my post-commit LGTM! Nice change. Thanks @ambv for taking care of 2to3 ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants