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.

Sign up for GitHub

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

Jump to bottom

Fix lookahead of soft keywords in the PEG parser #20436

Merged
merged 1 commit into from
May 26, 2020
Merged

Fix lookahead of soft keywords in the PEG parser #20436

merged 1 commit into from
May 26, 2020

Conversation

pablogsal
Copy link
Member

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

Automerge-Triggered-By: @gvanrossum

@pablogsal pablogsal requested a review from gvanrossum May 26, 2020 18:08
@pablogsal
Copy link
Member Author

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

@pablogsal pablogsal force-pushed the fix_lookahead branch 2 times, most recently from a4c9940 to caeaa31 Compare May 26, 2020 19:46
@lysnikolaou
Copy link
Contributor

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

@pablogsal
Copy link
Member Author

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

#20370

pablogsal
pablogsal commented May 26, 2020
View reviewed changes
Lib/_s.py Outdated Show resolved Hide resolved
gvanrossum
gvanrossum reviewed May 26, 2020
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

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.

Lib/_s.py Outdated Show resolved Hide resolved
gvanrossum
gvanrossum approved these changes May 26, 2020
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks!

@lysnikolaou
Copy link
Contributor

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

#20370

Nice.

@gvanrossum
Copy link
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
Copy link
Member Author

pablogsal commented May 26, 2020 •
edited
Loading

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
Copy link
Contributor

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

@miss-islington miss-islington merged commit 404b23b into python:master May 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] Backport pythonGH-20370 and pythonGH-20436: Soft keywords
d315e95
pablogsal pushed a commit that referenced this pull request May 27, 2020
@lysnikolaou
[3.9] Backport GH-20370 and GH-20436: Soft keywords (GH-20458)
1bfe659
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees
No one assigned
Labels
skip issue skip 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