-
Notifications
You must be signed in to change notification settings - Fork 19
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
Which files and directories are maintained and important is unclear #38
Comments
If the bindings haven't caused any problems while maintaining this package, is it worth removing them and potentially breaking something? There could be someone out there using the rust or node bindings for this |
I think if we leave them in we should state that we don't test them and won't necessarily offer support for them. I think it's misleading to have them sit here with no statement. I think the Rust bindings may be used by lapce, but I think they depend on a fork. Hmm, may be they can look after the Rust bindings... |
Just a technical note: I think it's slightly messy to keep just the Rust bindings. IIUC, it's easy to get both the Node.js and Rust bindings by just doing To look after the Rust bindings only, one would remove the Node.js bindings which I think involves getting rid of:
if it's not there. So that would need to be removed if it got added. To do things properly I think one would then look after the existing or generated To not get either set of bindings, from tree-sitter 0.19.4, I believe one can do Arranging for removal of just the Node.js bindings could probably be automated and possibly the relevant files and directories could be added to It might be possible to generate both sets of bindings once, remove the Node.js binding info and then just look after the Rust things, ever after using I wonder whether binding removal is being considered along with generated source removal as mentioned here (search for "Mergeable Git Repos"). [1] There is also the option In theory, other programs might do the same at some point... |
So the only reason to keep package.json around is for the tree-sitter entry? |
I think there are a few more reasons:
[1] I think I saw this happen at the command line while testing and one can also see references for checking from |
Cool, I'm glad they're not doing any additional introspection. |
As part of the work on #47, I put together some documentation that is supposed to make it clear(er) what files and directories are provided by the repository and why. Also included is information about what I consider less stable and subject to change and removal. |
Regarding whether the The tldr is that I have seen some commits like CLI: Determine language symbol from grammar, not package.json (though that particular one is pretty old) or Allow dynamically loading grammars w/ no package.json file, so perhaps there's been an effort to reduce dependence on At the end of this comment is a rough pseudo "call-graph"-ish collection of info. The first bit covers subcommand entry points in These three functions are defined in Though it reads Interestingly, after that bit of code, there's a bit that does some processing if a My impression from looking at this code (see below for a brief summary based on testing) is that Does it matter if Note that the I moved So it looks like having
|
Commits relevant to this issue can now be seen at: |
This document attempts to cover our thinking around the time of the v0.0.12 release. |
Due to ignorance and history, the repository may contain files and/or directories that are not being maintained and/or tested.
Probably it's a good idea to work on spelling out more clearly which files and directories there are, whether they are relevant, and why.
When development started, there were no users of the grammar and lack of sufficient understanding on the part of yours truly resulted in various files / directories being committed to the repository.
Now though, I think there are some users and I know a bit better, so I'll work on spelling things out.
Perhaps I'll try to keep a summary in this first post.
What Is Likely To Be Kept
Currently, at least the following files / directories are important for some existing users and/or for proper functioning of the tree-sitter cli:
corpus
grammar.js
package.json
queries/highlights.scm
src/parser.c
corpus
contains test files. Although I think these are not adequate for testing, they can be helpful for light consistency checks as well as for becoming familiar with parse results. I am thinking to move it though (test/corpus
to be precise).grammar.js
andpackage.json
are necessary for the proper functioning of some of the subcommands of the tree-sitter cli.package.json
is consulted for at least the bit that looks like this:IIUC vscode-parse-tree retrieves this repository using
yarn
and thuspackage.json
must exist and have at least this bit:According to some docs:
Not sure what "publish" means here, but when I tried to replicate vscode-parse-tree's build steps, leaving out the fields
name
andversion
resulted in failure.src/parser.c
can be generated using an appropriate version of the tree-sitter cli (possibly with options like--abi
). However, Emacs 29+ and nvim-treesitter usesrc/parser.c
for compiling a dynamic library for their own uses. ATM, many grammar repositories provide this file (though that might change in the future -- see "Mergeable Git Repos" at tree-sitter/tree-sitter#930).queries/highlights.scm
is used by the difftastic project.Things I'm Thinking Of Removing
binding.gyp
bindings
Cargo.toml
package-lock.json
package.json
-- e.g.dependencies
anddevDependencies
binding.gyp
is related to Node.js bindings which I do not use nor test. I have not heard of anyone using those bindings and this file is generated by the tree-sitter cli anyway, so if someone needs it, they can create it.bindings
contains Node.js and Rust bindings which I do not use nor test. I have not heard of anyone using these bindings and this directory and its content are generated by the tree-sitter cli anyway, so... :)Cargo.toml
is for the Rust bindings which...see above :)package-lock.json
is generated bypackage.json
, but is optional and is used bynpm
when installing. As I'm leaning in the direction of not usingnpm
for tree-sitter-clojure development, this file is a candidate for removal.There are parts of
package.json
which do not need to exist ifnpm
is not being used for development purposes. Some such parts include:The following bits may also be removed:
Regarding
main
, I found this:In our case it seems irrelevant and testing with it removed seemed fine (at least when locally trying for vscode-parse-tree's case).
The text was updated successfully, but these errors were encountered: