Skip to content

Conversation

@nkrkv
Copy link
Collaborator

@nkrkv nkrkv commented Nov 27, 2022

Close #178

I’m tempted to generalize pipe expression as seen in #176 and #177. But to me, they are too wide, producing extraneous grammar conflicts for constructs not legal actually in ReScript.

So, as an alternative proposal, can we have a nailed-down addition to fix the bug, but not go too wide?

/cc @aspeddro

),
choice('->', '|>'),
choice(
$.value_identifier,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove all rules on rhs and use only choice($.primary_expression, $.block)?

There will be minor changes in testing. But I think it's more consistent.

Example:

1 -> #A("foo")

Is parsed as:

yarn run v1.22.19
$ /home/pedro/Desktop/projects/tree-sitter-rescript/node_modules/.bin/tree-sitter parse ../../test-filetypes/rescript_print.res
(source_file [0, 0] - [1, 0]
  (expression_statement [0, 0] - [0, 14]
    (call_expression [0, 0] - [0, 14]
      function: (pipe_expression [0, 0] - [0, 7]
        (number [0, 0] - [0, 1])
        (polyvar_identifier [0, 5] - [0, 7]))
      arguments: (arguments [0, 7] - [0, 14]
        (string [0, 8] - [0, 13]
          (string_fragment [0, 9] - [0, 12]))))))
Done in 0.47s.

With this change:

yarn run v1.22.19
$ /home/pedro/Desktop/projects/tree-sitter-rescript/node_modules/.bin/tree-sitter parse ../../test-filetypes/rescript_print.res
(source_file [0, 0] - [1, 0]
  (expression_statement [0, 0] - [0, 14]
    (pipe_expression [0, 0] - [0, 14]
      (number [0, 0] - [0, 1])
      (polyvar [0, 5] - [0, 14]
        (polyvar_identifier [0, 5] - [0, 7])
        (arguments [0, 7] - [0, 14]
          (string [0, 8] - [0, 13]
            (string_fragment [0, 9] - [0, 12])))))))
Done in 0.05s.

Copy link
Collaborator

@aspeddro aspeddro Nov 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually in another PR. See #182

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Fair point, thank you for the PR!

@aspeddro
Copy link
Collaborator

So, as an alternative proposal, can we have a nailed-down addition to fix the bug, but not go too wide?

Sure. Looks great.

@nkrkv nkrkv merged commit 072e0ab into main Dec 4, 2022
@nkrkv nkrkv deleted the fix-pipe-block-on-lhs branch December 4, 2022 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: block on lhs of pipe_expression

3 participants