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

Fail to parse if keyword is not longest identifier #742

Merged
merged 1 commit into from
Nov 9, 2022
Merged

Fail to parse if keyword is not longest identifier #742

merged 1 commit into from
Nov 9, 2022

Conversation

Kijewski
Copy link
Collaborator

@Kijewski Kijewski commented Nov 9, 2022

This PR makes e.g. {% leta = b %} a parsing error. To the reader it would appear that leta should be a meaningful expression in Askama, which it is not. Before this PR, leta was tokenized as let + a. This PR makes the parser try to find the longest identifier at a parsing positions and only compare the outcome against the expected keyword.

This is potentially a breaking change, because code that should always have been invalid will now fail to compile, when it was accepted before.

This PR makes e.g. `{% leta = b %}` a parsing error. To the reader it
would appear that `leta` should be a meaningful expression in Askama,
which it is not. Before this PR, `leta` was tokenized as `let` + `a`.
This PR makes the parser try to find the longest identifier at a parsing
positions and only compare the outcome against the expected keyword.

This is potentially a breaking change, because code that should always
have been invalid will now fail to compile, when it was accepted before.
@Kijewski Kijewski changed the title Fail to parse if keyword is not longest ident Fail to parse if keyword is not longest identifier Nov 9, 2022
@djc
Copy link
Collaborator

djc commented Nov 9, 2022

Ohh, good call, thanks!

@djc djc merged commit 58d3938 into rinja-rs:main Nov 9, 2022
@Kijewski Kijewski deleted the pr-keyword branch November 9, 2022 12:18
@djc djc mentioned this pull request Feb 26, 2023
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.

2 participants