-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Update some grammars to a commit where the license file is included #8691
Conversation
There are also some grammars that don't have any license files at all. I created PR for those and will file a PR (here) each time one of those is merged. (I am unsure myself why I went as far as filing PRs for missing license files but here we are :D) |
In general when updating a language you need to update the commit hash but also look through the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for updating these!
name = "beancount" | ||
source = { git = "https://github.com/polarmutex/tree-sitter-beancount", rev = "4cbd1f09cd07c1f1fabf867c2cf354f9da53cc4c" } | ||
source = { git = "https://github.com/polarmutex/tree-sitter-beancount", rev = "f3741a3a68ade59ec894ed84a64673494d2ba8f3" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beancount had quite a few changes but it's all refactoring to make the grammar and nodes nicer to work with. It doesn't look like the queries we use need any updates.
name = "comment" | ||
source = { git = "https://github.com/stsewd/tree-sitter-comment", rev = "5dd3c62f1bbe378b220fe16b317b85247898639e" } | ||
source = { git = "https://github.com/stsewd/tree-sitter-comment", rev = "a37ca370310ac6f89b6e0ebf2b86b2219780494e" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No changes here to the grammar
name = "llvm" | ||
source = { git = "https://github.com/benwilliamgraham/tree-sitter-llvm", rev = "3b213925b9c4f42c1acfe2e10bfbb438d9c6834d" } | ||
source = { git = "https://github.com/benwilliamgraham/tree-sitter-llvm", rev = "e9948edc41e9e5869af99dddb2b5ff5cc5581af6" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just adds the license file
name = "ungrammar" | ||
source = { git = "https://github.com/Philipp-M/tree-sitter-ungrammar", rev = "0113de880a58ea14f2a75802e9b99fcc25003d9c" } | ||
source = { git = "https://github.com/Philipp-M/tree-sitter-ungrammar", rev = "a7e104629cff5a8b7367187610631e8f5eb7c6ea" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, just the license
name = "astro" | ||
source = { git = "https://github.com/virchau13/tree-sitter-astro", rev = "5f5c3e73c45967df9aa42f861fad2d77cd4e0900" } | ||
source = { git = "https://github.com/virchau13/tree-sitter-astro", rev = "947e93089e60c66e681eba22283f4037841451e7" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here too, just the license
In the meantime some of my PRs were merged, here is another set of language updates :D. |
It seems that the generation of the docs for the |
i seems the sql queries will need to be updated |
For sql I think we should be able to copy the changes to the queries upstream: DerekStride/tree-sitter-sql@eeab724...25be0b8#diff-d485a982e458ef8da2cc203585065b7542665cb80b78d230b1e8f77ea25825d4 |
I updated the |
Funny how each time someone approves, a PR of mine gets merged and I have to update another language :D |
The conflict stems from #8712. That PR updates the grammar but uses a fork which seems very weird because the original repository is still maintained? The last commit is a merge commit from the owner of the new fork. Very weird 🤔 |
It looks to me like the author of the new repo is working on upstreaming their changes to the original: https://github.com/Maskhjarna/tree-sitter-purescript/pulls?q=is%3Apr+sort%3Aupdated-desc+is%3Aclosed+author%3Apostsolar - they may have swapped us to their fork if they have a bunch of changes that are waiting on review / effort to upstream. If they merge their main branch with Maskhjarna's they should get the license file change: postsolar/tree-sitter-purescript@main...Maskhjarna:tree-sitter-purescript:main |
Yeah it's pretty much what Mike said, I made the swap because I don't expect the original repo owner to review as often as I commit and needed some extra velocity. They contacted me about it and we agreed on both repos staying open for now. Expect mine fork to have more frequent updates. I just added a LICENSE file to resolve the situation, so you can pin to the latest commit @blinxen. There were no breaking changes but there's a bugfix so please update the queries as well. You can just copy them because they're Helix-compatible. |
This PR is blocked until Maskhjarna/tree-sitter-purescript#30 is solved / merged. Also see postsolar/tree-sitter-purescript#5. |
Note that for Fedora packaging you're also able to exclude or include grammars in the language config, so you should be able to skip any unlicensed grammars |
The referenced PR has been merged. |
Rebased to master and updated purescript to the latest version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small changes for the new queries but otherwise this looks good 👍
runtime/queries/sql/highlights.scm
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, could you reset the changes here except for the additions to the @keyword
pattern and the changes to the @operator
pattern? The queries upstream are written for nvim so they differ a little from our capture names (https://docs.helix-editor.com/master/themes.html#syntax-highlighting) and use nvim-specific features like #lua-match?
. The current highlights other that those keyword and operator highlights shouldn't need any changes I don't think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed all changes except the ones you told me to keep.
@@ -117,8 +117,9 @@ | |||
|
|||
(row_field (field_name) @variable.other.member) | |||
(record_field (field_name) @variable.other.member) | |||
(record_accessor (variable) @variable.other.member) | |||
(exp_record_access (variable) @variable.other.member) | |||
(record_field (field_pun) @field) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(record_field (field_pun) @field) | |
(record_field (field_pun) @variable.other.member) |
nvim's @field
is @variable.other.member
in helix capture names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it' a typo. Just pushed the change :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to the latest commit
This PR is kind of tricky because I am unsure how you guys handle language / grammar updates.
Problem I am trying to solve here: We need to include license files for all grammars. Some grammars that are used in
languages.toml
use an old commit where the license file is not included. I updated the grammars to a commit that includes the license file.