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

feat: support non-indented cases in Scala3 match expression #436

Closed
wants to merge 2 commits into from

Conversation

susliko
Copy link
Collaborator

@susliko susliko commented Nov 25, 2024

Addresses #264

@@ -1157,7 +1162,7 @@ module.exports = grammar({
optional($.inline_modifier),
field("value", $.expression),
"match",
field("body", choice($.case_block, $.indented_cases)),
field("body", choice($.case_block, $.indented_cases, $.non_indented_cases)),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should non_indented_cases be aliased to indented_cases here to preserve compatibility for any queries that may not expect nodes other than case_block/indented_cases, wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 to this.

@susliko
Copy link
Collaborator Author

susliko commented Nov 25, 2024

Looks like CI ran against an old version, without changes in grammar.js. Strange 🤔

@@ -850,8 +866,8 @@ class C {
a
.b
// comment1
/*
comment2
/*
Copy link
Collaborator Author

@susliko susliko Nov 25, 2024

Choose a reason for hiding this comment

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

Changes here and below are caused by changes in the behavior of tree-sitter test -u, which now removes trailing spaces

@eed3si9n
Copy link
Collaborator

Looks like CI ran against an old version, without changes in grammar.js. Strange 🤔

Previously, we were running generate explicitly only on Linux, but at some point CI started testing on macOS as well? I fixed the CI in #437

Copy link
Collaborator

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

LGTM

@eed3si9n
Copy link
Collaborator

Checking syntax complexity: npm exec -c 'tree-sitter generate --report-states-for-rule compilation_unit' 2>&1 >/dev/null
Report written to report-complexity.txt
tuple_type 	1857
Error: complexity of the most complex definition tuple_type: 	1857, higher than the allowed ceiling 1400

I guess PREC.right adds to the complexity a bit? I'm ok with bumping this up.

@susliko
Copy link
Collaborator Author

susliko commented Nov 27, 2024

While working on this PR, I realized that the conflict between $.match_expression and $._block is slowing down the generation process by about 9 times. I haven't found a good solution yet, so I'm going to close the PR for now.

@susliko susliko closed this Nov 27, 2024
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