Skip to content

Fix lookahead of soft keywords in the PEG parser - #20436

Merged
miss-islington merged 1 commit into
python:masterfrom
pablogsal:fix_lookahead
May 26, 2020
Merged

Fix lookahead of soft keywords in the PEG parser#20436
miss-islington merged 1 commit into
python:masterfrom
pablogsal:fix_lookahead

Conversation

@pablogsal

@pablogsalpablogsal commented May 26, 2020•
edited by miss-islington
Loading

Copy link
Copy Markdown
Member

Automerge-Triggered-By: @gvanrossum

@pablogsal
pablogsal requested a review from gvanrossumMay 26, 2020 18:08
@pablogsal

pablogsal commented May 26, 2020

Copy link
Copy Markdown
MemberAuthor

I was playing a bit with the feature and realized that we missed the lookahead code.

@pablogsal
pablogsalforce-pushed the fix_lookahead branch 2 times, most recently from a4c9940 to caeaa31CompareMay 26, 2020 19:46
@lysnikolaou

lysnikolaou commented May 26, 2020

Copy link
Copy Markdown
Member

When were soft keywords added? I can't remember us implementing that.

@pablogsal

pablogsal commented May 26, 2020

Copy link
Copy Markdown
MemberAuthor

When were soft keywords added? I can't remember us implementing that.

#20370

pablogsal
pablogsal commented May 26, 2020
View reviewed changes
Comment threadLib/_s.py Outdated
gvanrossum
gvanrossum reviewed May 26, 2020
View reviewed changes

@gvanrossumgvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay, everything else LGTM. I now remember noticing this (during my hack to add a print statement) and thinking "oh, I need to add _PyPegen_lookahead_with_string" -- and then forgetting about it.

Comment threadLib/_s.py Outdated
gvanrossum
gvanrossum approved these changes May 26, 2020
View reviewed changes

@gvanrossumgvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@lysnikolaou

lysnikolaou commented May 26, 2020

Copy link
Copy Markdown
Member

When were soft keywords added? I can't remember us implementing that.

#20370

Nice.

@gvanrossum

gvanrossum commented May 26, 2020

Copy link
Copy Markdown
Member

There's one caveat: we need to remind people to use 'single' quotes for keywords, not "double" quotes. This happens to be the current convention but it may be surprising to people that it matters, and some folks may just automatically type "double" quotes.

@pablogsal

pablogsal commented May 26, 2020•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

There's one caveat: we need to remind people to use 'single' quotes for keywords, not "double" quotes. This happens to be the current convention but it may be surprising to people that it matters, and some folks may just automatically type "double" quotes.

That could be on the "new parser guide" on the dev guide. I was preparing something for that, but I am currently waiting to see how we want to approach #19969 first.

@miss-islington

miss-islington commented May 26, 2020

Copy link
Copy Markdown
Contributor

@pablogsal: Status check is done, and it's a success ✅ .

@miss-islington
miss-islington merged commit 404b23b into python:masterMay 26, 2020
@pablogsal
pablogsal deleted the fix_lookahead branch May 26, 2020 23:16
lysnikolaou added a commit to lysnikolaou/cpython that referenced this pull request May 27, 2020
@lysnikolaou
[3.9] BackportpythonGH-20370andpythonGH-20436: Soft keywords
d315e95
pablogsal pushed a commit that referenced this pull request May 27, 2020
@lysnikolaou
[3.9] BackportGH-20370andGH-20436: Soft keywords (GH-20458)
1bfe659
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossumgvanrossumgvanrossum approved these changes

Assignees

No one assigned

Labels

skip issueskip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pablogsal@lysnikolaou@gvanrossum@miss-islington@the-knights-who-say-ni@bedevere-bot