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

fix: #3758 Allow for string being a keyword and fix go template to us… #3831

Merged
merged 3 commits into from
Aug 23, 2022
Merged

fix: #3758 Allow for string being a keyword and fix go template to us… #3831

merged 3 commits into from
Aug 23, 2022

Conversation

jimidle
Copy link
Collaborator

@jimidle jimidle commented Aug 22, 2022

fix: #3758 Consistently apply .escapedName instead of just .name for the go target. Add string to reserved words

  • The Go code generation template mostly ignored the use of escapedName, hence it would generate rules and so on
    that were keywords in go.
  • Added string as a keyword for go

…e escapedName

  - The go template was ignoring the use of escapedName in many places and was
    not consistenet with the Java version.
  - Added 'string' to the list of reserved words for the Go target

Signed-off-by: Jim.Idle <jimi@gatherstars.com>
@KvanTTT
Copy link
Member

KvanTTT commented Aug 22, 2022

Could you please add a test that covers the case?

Jim.Idle added 2 commits August 23, 2022 10:25
Signed-off-by: Jim.Idle <jimi@gatherstars.com>
….mod

Signed-off-by: Jim.Idle <jimi@gatherstars.com>
@jimidle
Copy link
Collaborator Author

jimidle commented Aug 23, 2022

The go.mod file is updated to include the go runtime extensions and the go.sum file is added to the repo, which I had not noticed was missing.

@parrt parrt added this to the 4.10.2 milestone Aug 23, 2022
@parrt parrt merged commit e51cfcc into antlr:dev Aug 23, 2022
@jimidle
Copy link
Collaborator Author

jimidle commented Aug 23, 2022

I will add a string rule to the existing keyword test. I should have done that with this PR.

@jimidle
Copy link
Collaborator Author

jimidle commented Aug 30, 2022

@parrt I thin we can close this now?

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

Successfully merging this pull request may close these issues.

3 participants