-
Notifications
You must be signed in to change notification settings - Fork 34
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
quint/grammar: add support for optional leading hashbang line #1522
Conversation
/cc @bugarela for review. I decided to follow JS approach and only carve a niche in the grammar for the first line, see https://github.com/tc39/proposal-hashbang for details. |
57e8700
to
644e0a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for taking the time and linking that proposal!
I just left a comment on the grammar. Also, it would be good to add an integration tests that calls ./testFixture/SuperSpec.qnt
to test the overall hashbang behavior. It should be simple to add that to cli-tests.md
, and you can run the integration tests with npm run integration
- lmk if you need assistance with that or would rather have me take on from here.
quint/src/generated/Quint.g4
Outdated
@@ -313,6 +314,7 @@ CAP_ID : ([A-Z][a-zA-Z0-9_]*|[_][a-zA-Z0-9_]+) ; | |||
DOCCOMMENT : '///' .*? '\n'; | |||
|
|||
// comments and whitespaces | |||
HASHBANG_LINE : '#!' .*? '\n' -> skip ; // only used for the first line of a file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using skip where tells ANTLR that we want to skip this token, wherever it appears, as it is the case with comments and whitespaces. However, in this case, we only want to allow this in the very first line, so we shouldn't skip
it. I've tested this locally and confirmed that, in the current state, this will allow hasbangs in the middle of a file.
We can simply remove the -> skip
here and move it closer to the other regular tokens (and away from the skipped ones).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I'm not overly familiar with ANTLR grammars so I didn't realize that skip
had such a side-effect.
I did the changes you suggested and also added a few unit tests to ensure that the hashbang rule only applies to the first line of the file.
644e0a3
to
ca5083a
Compare
Fixed grammar rules and rebased to latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks again
This updates Quint grammar to support a leading hashbang (
#!
) line in spec files.It allows using a spec file as a self-running script on Unix systems, as shown in
SuperSpec.qnt
.CHANGELOG.md
for any new functionalityCloses #1500