Skip to content
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

Pattern matching #193

Merged
merged 8 commits into from
Nov 17, 2021
Merged

Pattern matching #193

merged 8 commits into from
Nov 17, 2021

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Nov 5, 2021

This pull request adds pattern matching syntax to the Ruby grammar. This syntax looks as follows:

case expr
  in pattern1 then  ...
  in pattern2 then ...
  else ...
end

See also: https://docs.ruby-lang.org/en/3.0.0/doc/syntax/pattern_matching_rdoc.html

This pull request does not yet add the one-line pattern matching syntax (expr in pattern and expr => pattern). While the addition to the grammar just a few lines, it causes a serious increase in the generated C code. Therefore, I think this is best left as a follow-up pull request where we can discuss what causes the blow-up and how to address it. See also: aibaars#1

This pull request makes some minor changes to the existing grammar:

  • _expression is added to the supertypes list to avoid a lot of duplication in the node-types file
  • A new intermediate rule _literal is extracted for the various types of literals allowing it to be used for expressions as well as patterns.
  • The unary literal rule only allowed float and integer values, this has been corrected to also allow the other numeric literals (e.g. complex and rational)

Checklist:

  • All tests pass in CI.
  • There are sufficient tests for the new fix/feature.
  • Grammar rules have not been renamed unless absolutely necessary.
  • The conflicts section hasn't grown too much.
  • The parser size hasn't grown too much (check the value of STATE_COUNT in src/parser.c).

@aibaars aibaars force-pushed the pattern-matching branch 3 times, most recently from ff1d2f4 to 31abc7f Compare November 12, 2021 11:13
@aibaars aibaars force-pushed the pattern-matching branch 3 times, most recently from 85a588d to 176f3a9 Compare November 16, 2021 17:51
@aibaars aibaars marked this pull request as ready for review November 17, 2021 12:33
Copy link
Contributor

@dcreager dcreager left a comment

Choose a reason for hiding this comment

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

This looks good, thanks @aibaars! Do the syntax highlighting queries need to be updated to highlight patterns correctly?

@aibaars
Copy link
Contributor Author

aibaars commented Nov 17, 2021

This looks good, thanks @aibaars! Do the syntax highlighting queries need to be updated to highlight patterns correctly?

@dcreager Possibly, but I don't know how to check that. I suspect that things might "just work" already because the pattern syntax is quite similar to expressions and re-uses the same keywords.

@dcreager
Copy link
Contributor

The easiest way would be to run tree-sitter highlight against some Ruby files containing patterns and make sure they look "right". You can also add some additional tests to the tests/highlight directory if you wanted to verify it more rigorously (and in a way that would prevent regressions).

(encoding) @constant.builtin

(hash_pattern_norest
"**" @operator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dcreager Ideally the highlight of ** would be none, however, I don't know how to express that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I actually don't know if that can be done! That's an interesting thing that we should support. 👍 with this workaround for now.

(encoding) @constant.builtin

(hash_pattern_norest
"**" @operator
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I actually don't know if that can be done! That's an interesting thing that we should support. 👍 with this workaround for now.

@dcreager dcreager merged commit f1ff027 into tree-sitter:master Nov 17, 2021
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.

2 participants