Skip to content

Conversation

@ambv
Copy link
Contributor

@ambvambv commented May 22, 2017

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

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

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-deilyned-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
ContributorAuthor

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.
@ambvambv requested review from gvanrossum and vstinnerMay 22, 2017 19:42
@ambvambv merged commit 0c4aca5 into python:masterMay 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bugAn unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@ambv@vstinner@gvanrossum@ned-deily@Mariatta@the-knights-who-say-ni