-
Notifications
You must be signed in to change notification settings - Fork 11
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 tree-sitter-openscad
#35
Comments
Your pull request for tree-sitter-openscad contains too many changes, making it difficult to review. It would be better to break it down into several smaller PRs. Additionally, it seems some editor config files and symbol files have been included in the commit, should these be removed? |
If the maintainer has not merged your code for a long time, I think it is ok to use your repository directly. |
Maybe you could ask the openscad org if they work like to adopt your fork into the org, and then it could become the official repository for the treesitter grammar. It seems a fairly foundational library that makes sense to have in the org to ensure it stays maintained. |
@kintel do you have any thoughts on the matter for @Lenbok's suggestion? There's some changes (more context in the issue desc.) for syntax highlighting, formatting, and LSP error messaging held back by lack of movement in the current tree-sitter-openscad repo, would the tree-sitter grammar do well to live under github.com/openscad? Perhaps github.com/tree-sitter-grammars is also a good location for the grammar to move to. EDIT: one of the problems is that nobody here other than the repository owner can transfer the repository. It would essentially have to be a hard fork and tree-sitter-grammars is not accepting any community grammars at this moment. |
I'm not familiar with tree-sitter, but we could easily host an openscad-specific grammar under the OpenSCAD github org. Someone would need to care for the repo of course. Let me know what I can do to facilitate that. |
Hi @kintel, thanks for the quick response. Most of the relevant changes were under my fork of tree-sitter-openscad here:
There's a quick intro here but tree-sitter essentially allows a lot of operations to be done on the grammar of the language just through the AST including syntax highlighting (many of grammars on github use tree-sitter for highlighting): |
I've sent @bollian an email directly in case he's not getting GitHub notifications to see if he's open to the idea of transferring the original repo. Let's see if there's any response in the next few days. |
Sounds good, give me the word, and I can initiate a hard fork/transfer from whatever source we want to use. |
There's been no reply. I suggest that @mkatychev work with @kintel to set up the hard fork into the openscad org along with whatever other changes need to happen so that the crate can get published in a way that downstream packages can start using it. |
Agreed, I had one PR that was an easy to review change, however it did not make sense to make further PRs if the code is not going to get reviewed. The commits themselves are self-contained so it would not be hard to make pull requests out of them. |
New repo has been made here: https://github.com/openscad/tree-sitter-openscad/ @kintel has been a huge help! |
In a prelude to #33 I've added many more grammar cases to the
tree-sitter-openscad
grammar however the repo is possibly unmaintained.The changes needed to add the edge cases that are correct in the openscad AST would required a newer version of the tree-sitter-openscad crates.io crate that I have no control over.
In the meantime, adding the coverage from the proposed 0.6 changes would required the dep below to change to the git dependency or a crates.io fork:
openscad-LSP/Cargo.toml
Line 20 in e4ac57e
tree-sitter-openscad.git = https://github.com/mkatychev/tree-sitter-openscad
tree-sitter-openscad = { package = "tree-sitter-openscad-fork", version = "0.6" }
The git dependency change means that a newer version of
openscad-lsp
won't be able to be published to crates.io but a fork may also be undesirable. Option #3 requires the most work and I'm not for it, just providing an alternative.Let me know your throughts @Leathong
The text was updated successfully, but these errors were encountered: