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

bump tree-sitter-sql #8464

Merged
merged 3 commits into from
Oct 9, 2023
Merged

Conversation

ds-cbo
Copy link
Contributor

@ds-cbo ds-cbo commented Oct 4, 2023

Bumps to DerekStride/tree-sitter-sql@eeab724 (the commit with the compiled grammar corresponding to main commit DerekStride/tree-sitter-sql@385aff4)

Changes since previous revision (DerekStride/tree-sitter-sql@7cbac04, 19 June):

Also see: #7387

@the-mikedavis the-mikedavis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-language-support Area: Support for programming/text languages labels Oct 5, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you prefer me to make the diff easier by moving things around in the file to make the keywords align with the current highlights file, or would it be nicer to keep the order of upstream so that future changes upstream are easier to follow?

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be nicer to follow the upstream highlights closely so that when they make changes to the highlights upstream it's clear where to make changes when we update the grammar. But I don't look at SQL much so I'm not sure if the queries we have in master are significantly better than the new ones in the repo (- looks like they were added since the last time we updated). If you think the new ones look good then let's use those (with the adjustments to the Helix capture names)

@ds-cbo ds-cbo requested a review from the-mikedavis October 6, 2023 12:50
(binary_expression
operator: _ @operator)
((literal) @constant.numeric.integer
(#lua-match? @constant.numeric.integer "^%d+$"))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(#lua-match? @constant.numeric.integer "^%d+$"))
(#match? @constant.numeric.integer "^\d+$"))

#lua-match? is nvim-specific. We use just the ones tree-sitter provides in the rust bindings https://tree-sitter.github.io/tree-sitter/using-parsers#predicates (though there are some there like #any-of? that we don't implement yet because we're waiting on a tree-sitter release that adds them)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also resolved. Thanks for your patience, I'm new to this tree-sitter world so don't know many of the keywords/possibilities yet, but it looks interesting

(unary_expression
operator: _ @operator)
((literal) @constant.numeric.float
(#lua-match? @constant.numeric.float "^[-]?%d*\.%d*$"))
Copy link
Member

Choose a reason for hiding this comment

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

Same here - I think the regex from the old highlights should work in place of this one (I think the %d is lua-regex-specific)

@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 9, 2023
@pascalkuthe pascalkuthe merged commit 96bbfb7 into helix-editor:master Oct 9, 2023
@ds-cbo ds-cbo deleted the bump-tree-sitter-sql branch October 10, 2023 13:56
danillos pushed a commit to danillos/helix that referenced this pull request Nov 21, 2023
* bump tree-sitter-sql

* update highlights classes to helix flavour

* replace lua-match with match
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
* bump tree-sitter-sql

* update highlights classes to helix flavour

* replace lua-match with match
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
* bump tree-sitter-sql

* update highlights classes to helix flavour

* replace lua-match with match
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
* bump tree-sitter-sql

* update highlights classes to helix flavour

* replace lua-match with match
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-support Area: Support for programming/text languages S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants