fix: identifiers in collection expressions vs attributes with targets#4
Conversation
| | `Fiel tok -> R.Case ("Fiel", | ||
| (* "field:" *) token env tok |
There was a problem hiding this comment.
Is R.Case ("Fiel" intentional?
There was a problem hiding this comment.
it's autogenerated. No idea
| * Token.t (* ":" *) | ||
| ) | ||
| type attribute_target_specifier = [ | ||
| `Fiel of Token.t (* "field:" *) |
There was a problem hiding this comment.
any reason for the renaming?
| | `Meth tok -> R.Case ("Meth", | ||
| (* "method:" *) token env tok | ||
| ) | ||
| | `Para tok -> R.Case ("Para", |
There was a problem hiding this comment.
I personally liked Param, Ret and Field better... more readable.
| attribute_target_specifier: $ => seq( | ||
| choice('field', 'event', 'method', 'param', 'property', 'return', 'type'), | ||
| ':' | ||
| choice('field:', 'event:', 'method:', 'param:', 'property:', 'return:', 'type:'), |
There was a problem hiding this comment.
I'm not proficient enough in TS to appreciate this change, could we not have wrapped the original production in token(...) ?
There was a problem hiding this comment.
nope. token parses as usual, but sticks things together only after they are parsed (I think), which would make TS go into wrong branch anyway. It would be better to have a more sensible structure of the grammar, but I tried to refactor for this PR and failed...
There was a problem hiding this comment.
I think it would work, if not a prec would help?
Anyway not important.
Fix a bug (seems to be in the original tree-sitter parser):
The parser confused collection expression that started with an identifier similar to an attribute target, e.g.,
[type, ...]etc with attributes with targets[type: ...].