tree-sitter / tree-sitter-julia

Julia grammar for Tree-sitter
MIT License
93 stars 32 forks source link

Fix `argument_list` and `tuple_expression` with semicolon #75

Closed wangl-cc closed 1 year ago

wangl-cc commented 1 year ago

This PR added implicit_named_field in a argument_list and tuple_expression after semicolon ;, close #65.

The implicit_named_field can be a identifier or a field_expression, see https://github.com/JuliaLang/julia/blob/master/doc/src/manual/functions.md?plain=1#L871-L873.

Besides, this PR also changed some syntax about Tuple:

Sorry about that I'm not familiar with tree-sitter, the tree-sitter told me a conflict between implicit_named_field and keyword_parameters, and I add a conflict for them following its suggestion`.

savq commented 1 year ago

This PR added implicit_named_field in a argument_list and tuple_expression

I was doing something similar in #72 (only for arguments), but I didn't add a rule, because it doesn't seem to add new information. Though we could keep it to make it more obvious that keyword arguments and named tuples are related. Either way, I'd prefer if it was a hidden rule: _implicit_named_field.

This rule causes a conflict because while parsing (;x) tree-sitter doesn't know if it'll parse an arrow function like (;x) -> x+1 or just a tuple. You can see most of the other conflicts are related to parameters and arguments. So I guess it's fine for now.

Besides, this PR also changed some syntax about Tuple

These changes are much more valuable, but it needs a couple of fixes. It's not parsing some singletons correctly:

(; a)
(; a = 1)
(source_file [0, 0] - [2, 0]
  (ERROR [0, 0] - [1, 9]
    (parameter_list [0, 0] - [0, 5]
      (keyword_parameters [0, 1] - [0, 4]
        (identifier [0, 3] - [0, 4])))
    (parameter_list [1, 0] - [1, 9]
      (keyword_parameters [1, 1] - [1, 8]
        (optional_parameter [1, 3] - [1, 8]
          (identifier [1, 3] - [1, 4])
          (integer_literal [1, 7] - [1, 8]))))))

In the last case, you can change choice(…); repeat1(…) for sep(',', …). That also removes the need for a separate case for (;). Then you can add these two examples to the tests.

Also, could you place the comments on top of each case? (instead of after the parenthesis).

wangl-cc commented 1 year ago

Thanks for your review, I have updated with your suggestions. The only difference is that I change choice(…); repeat1(…) for sep1(',', …), instead of sep(',', …). Because which will match (; ,),

savq commented 1 year ago

Great! thank you.