Skip to content

Drop Keyword suffix from TokenSpecSet #1668

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

Merged
merged 1 commit into from
May 26, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented May 16, 2023

Since we started modelling keywords as a single RawTokenKind there hasn’t been an ifKeyword etc. anymore. Instead we always refered to them as Keyword.if or .if everywhere. We should be consistent about this in TokenSpecSet as well, which also shouldn’t have a Keyword suffix on the cases.

@ahoppen ahoppen requested a review from bnbarham May 16, 2023 02:41
@ahoppen
Copy link
Member Author

ahoppen commented May 16, 2023

@swift-ci Please test

Copy link
Contributor

@kimdv kimdv left a comment

Choose a reason for hiding this comment

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

Nice!
A bit cleaner to read

case `do`
case `fallthrough`
case `for`
case forget // NOTE: support for deprecated _forget
Copy link
Contributor

Choose a reason for hiding this comment

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

_forget?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Thanks. I missed that

@ahoppen ahoppen force-pushed the ahoppen/drop-keyword-suffix branch from fb41a22 to 0a96021 Compare May 16, 2023 22:02
@ahoppen
Copy link
Member Author

ahoppen commented May 16, 2023

@swift-ci Please test

Since we started modelling keywords as a single `RawTokenKind` there hasn’t been an `ifKeyword` etc. anymore. Instead we always refered to them as `Keyword.if` or `.if` everywhere. We should be consistent about this in `TokenSpecSet` as well, which also shouldn’t have a `Keyword` suffix on the cases.
@ahoppen ahoppen force-pushed the ahoppen/drop-keyword-suffix branch from 0a96021 to d3316f9 Compare May 25, 2023 15:08
@ahoppen
Copy link
Member Author

ahoppen commented May 25, 2023

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge May 25, 2023 15:08
@ahoppen
Copy link
Member Author

ahoppen commented May 26, 2023

@swift-ci Please test macOS

@ahoppen
Copy link
Member Author

ahoppen commented May 26, 2023

@swift-ci Please test Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants