-
Notifications
You must be signed in to change notification settings - Fork 75
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
removing strip-comments and using prettier to clean pragma versions #1063
Conversation
parser: 'slangPragma' | ||
}; | ||
|
||
const query = Query.parse( |
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.
I noticed you moved query
out of the tryToCollectPragmas
function, but you only use it there; should we move it back to its original location?
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.
this way the query is computed only once and not every time it tryToCollectPragmas is called
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.
🚀
Co-authored-by: Mattia Richetto <[email protected]>
f2b6050
to
a167be2
Compare
I think I'd rather not support very weird pragma versions, like the one in the new test, and rather check that we throw a decent error when that happens (throw "couldn't parse version pragma in file " and crash). |
I made a new PR I had prepared if this one was too ambitious and complicated maintenance |
Aware this might be an overkill but the possible issue is really difficult to solve with other means (see tests).
I have a different approach to get rid of the
strip-comments
dependencies if this gets rejected.