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

fix: expose semicolons, remove generation warnings #159

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

ribru17
Copy link
Contributor

@ribru17 ribru17 commented Nov 11, 2024

This commit exposes semicolon terminators as anonymous nodes that can be queried, and removes the following warnings upon generating the parser:

Warning: rule _pair_operator is just a seq or choice rule with a single element. This is unnecessary.
Warning: rule _lazy_or_operator is just a seq or choice rule with a single element. This is unnecessary.
Warning: rule _lazy_and_operator is just a seq or choice rule with a single element. This is unnecessary.
Warning: rule _pipe_right_operator is just a seq or choice rule with a single element. This is unnecessary.
Warning: rule _pipe_left_operator is just a seq or choice rule with a single element. This is unnecessary.
Warning: rule _rational_operator is just a seq or choice rule with a single element. This is unnecessary.
Warning: rule _tilde_operator is just a seq or choice rule with a single element. This is unnecessary.

@savq
Copy link
Collaborator

savq commented Nov 12, 2024

Thanks for the PR.

Currently semicolons are parsed as a single token because it's more convenient for multidimentional arrays. Though I don't see any problem with parsing it as multiple tokens.

Is there a specific reason to make it visible?

@ribru17
Copy link
Contributor Author

ribru17 commented Nov 12, 2024

Is there a specific reason to make it visible?

Basically just so that, e.g. in nvim-treesitter, the semicolons can be given a highlight. Currently they cannot be queried since grammar regexes are not exposed

@ribru17
Copy link
Contributor Author

ribru17 commented Nov 12, 2024

Also I should mention that in other nodes in the grammar, there exists anonymous ";" nodes so I thought it would make sense for these terminators to be exposed in the same way for proper highlighting

@savq
Copy link
Collaborator

savq commented Nov 12, 2024

Also I should mention that in other nodes in the grammar, there exists anonymous ";" nodes

Yeah. It's very annoying that some places require a single semicolon and other don't.

One thing I noticed is that this would allow non-consecutive semicolons like [foo ; ; ; bar]. We'd probably want something like seq(';', repeat(token.immediate(';'))).

@ribru17
Copy link
Contributor Author

ribru17 commented Nov 12, 2024

Ah, good point. Is ; ; ; not valid julia?

@savq
Copy link
Collaborator

savq commented Nov 12, 2024

Ah, good point. Is ; ; ; not valid julia?

It's valid when delimiting expressions but not valid when delimiting matrix rows, and we're using _terminator for both.

I'd keep whichever rule produces less states (not that it makes a difference in our huge parser).

@ribru17
Copy link
Contributor Author

ribru17 commented Nov 13, 2024

This change gives 19334, 9490 for state count and large state count (respectively), and prior to this change it was 19618, 9476

@savq
Copy link
Collaborator

savq commented Nov 16, 2024

Writing just token.immediate will cause semicolons after whitespace to break. The first semicolon cannot be immediate.

@ribru17
Copy link
Contributor Author

ribru17 commented Nov 16, 2024

Ah apologies, I thought that was a mistake

This commit exposes semicolon terminators as anonymous nodes that can be
queried, and removes the following warnings upon generating the parser:

Warning: rule _pair_operator is just a `seq` or `choice` rule with a single element. This is unnecessary.
Warning: rule _lazy_or_operator is just a `seq` or `choice` rule with a single element. This is unnecessary.
Warning: rule _lazy_and_operator is just a `seq` or `choice` rule with a single element. This is unnecessary.
Warning: rule _pipe_right_operator is just a `seq` or `choice` rule with a single element. This is unnecessary.
Warning: rule _pipe_left_operator is just a `seq` or `choice` rule with a single element. This is unnecessary.
Warning: rule _rational_operator is just a `seq` or `choice` rule with a single element. This is unnecessary.
Warning: rule _tilde_operator is just a `seq` or `choice` rule with a single element. This is unnecessary.
@savq savq merged commit e01c928 into tree-sitter:master Nov 16, 2024
3 checks passed
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