-
Notifications
You must be signed in to change notification settings - Fork 456
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
Parse assert
as a regular function
#6180
Parse assert
as a regular function
#6180
Conversation
Quick check before even reading the code: this still parses the old style and converts to the new style, correct? |
No, old style is a syntax error. |
That's a breaking change. |
We can convert to the new format, but it changes the precedence. // Before
assert 1 == 2
// Parsed as
(assert 1) == 2
// Now
assert(1 == 2) Currently |
I think that OK. It's going to matter in very few cases. |
Still: to be mentioned in release notes. |
Are there tests that show that old style assert is still parsed? E.g. |
res_syntax/src/res_printer.ml
Outdated
| Nothing -> doc | ||
let doc = printExpressionWithComments ~state expr cmtTbl in | ||
let expr = | ||
match expr.pexp_desc with |
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.
Not sure I understand why this code is required.
Seems oddly specific for a generic assert.
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.
Removed in ef6501e. It is in fact no longer necessary.
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.
Looking great!
Ready to merge?
👍🏾 |
It's the first time a hear about |
The thing is avoiding breaking changes. |
rescript-lang/syntax#168
TODO: