-
Notifications
You must be signed in to change notification settings - Fork 13
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 repository and add modeline support #125
Conversation
corpus-files: |- | ||
examples/neovim/runtime/doc/* | ||
# FIXME: these files should not have errors | ||
invalid-files: |- |
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.
Presumably the "errors" are the ones ignored by gen_help_html.lua
: https://github.com/neovim/neovim/blob/bbb68e2a034ad3aaea99178c09301ca458ee8dff/scripts/gen_help_html.lua#L329-L331
Likely the best solution is to fix the help files themselves.
@@ -0,0 +1,47 @@ | |||
// swift-tools-version:5.3 | |||
import PackageDescription |
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.
wtf is swift doing here? does treesitter require rust, npm, C, and now swift?
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.
Not require, but support. These bindings are boilerplate that every parser gets so C, JS, Rust, Swift, Python projects can use tree-sitter, including parsers.
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.
Can all these things live in one place, instead of the top level? It's hard to tell what is generated and what isn't. That makes the project look way more overgrown than it is.
This project is literally just a single code-file and a bunch of tests. Yet it looks like a giant mess.
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.
You have a decision to make: do you want this to be a real parser that takes part in the tree-sitter ecosystem, or something internal that is only used for Neovim? (I personally think being considerate and standardize -- which upstream has started to do -- cuts both ways. )
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.
Can all these things live in one place, instead of the top level?
Some, yes, but not all. It depends on the language.
It's hard to tell what is generated and what isn't.
Look at .gitattributes
.
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.
I'll revert this PR anyway since it massively broke stuff. Next PR should be more targeted and not add unrelated things in a single PR.
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.
The only unrelated thing is the modeline and that was for the tests.
It's fine to revert that part but you will also need to remove the file parsing.
This PR updates the repository to use the new bindings and official actions (and bumps the version).
I also added a modeline node to increase the success rate of the parser to 50% from 20%.