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

add tree-sitter-git-config #1426

Merged

Conversation

the-mikedavis
Copy link
Member

@the-mikedavis the-mikedavis commented Jan 3, 2022

Since I spend so much time looking at .gitmodules I thought it should have some highlighting 😄

git-modules

Luckily git uses this config syntax for virtually all config, so this grammar applies to things like ~/.gitconfig as well.

git-config

I think this is a good grammar to start asking questions about expanding the file-types config in languages.toml. Ideally this grammar would apply for .git/config, ~/.config/git/config, and other config files under a git directory (for example: .git/modules/helix-syntax/languages/tree-sitter-git-config/config). I'm fine leaving off changes to language detection in this PR but it's something to think about.

Playground for the grammar: https://the-mikedavis.github.io/tree-sitter-git-config/

@the-mikedavis the-mikedavis force-pushed the md-tree-sitter-git-config branch 3 times, most recently from efc0ebb to dcac147 Compare January 6, 2022 16:59
@pickfire
Copy link
Contributor

pickfire commented Jan 7, 2022

Playground for the grammar: https://the-mikedavis.github.io/tree-sitter-git-config/

Wow \o/

name = "git-config"
scope = "source.gitconfig"
roots = []
file-types = [".gitmodules", ".gitconfig"]
Copy link
Contributor

@pickfire pickfire Jan 7, 2022

Choose a reason for hiding this comment

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

Is this a good idea? It could be .git/config as well. Not sure how we can do it here.

@archseer I think we might have to add support to check for the path as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm yeah I think if file-types could be a regex or a glob that'd work? We'd want to detect git-config for a few files where the containing directory/ies are important

Copy link
Contributor

Choose a reason for hiding this comment

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

For git it's actually a little more complicated. Git repositories don't have to be called .git. That is just the default for working trees. There many other possible variations. e.g bare repositories, submodules and git with multiple working trees (see git-worktree).

.git/modules/helix-syntax/languages/tree-sitter-c/config for example is probably a git config file in your helix working tree.

The best way afaik to detect git config files is to check for other items in the same directory. objects and refs for example.

I'm pretty sure other file types will be in a similar situation.

bat seems to have this figured out. Perhaps would be worth looking to see how they do it?

Copy link
Contributor

@pickfire pickfire Jan 8, 2022

Choose a reason for hiding this comment

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

I am thinking that's better done as plugin or some sort of check then. But right now the simplest would be regex on full path.

@pickfire
Copy link
Contributor

pickfire commented Jan 8, 2022

@the-mikedavis Do you think it's good to merge this now? I think the leftover can be done separately, maybe can just leave a todo comment saying to improve that.

@the-mikedavis
Copy link
Member Author

sounds good to me! let me just push a commit with the todo comment 👍

@pickfire pickfire merged commit e0a99ae into helix-editor:master Jan 9, 2022
@the-mikedavis the-mikedavis deleted the md-tree-sitter-git-config branch January 10, 2022 15:19
hovsater pushed a commit to hovsater/helix that referenced this pull request Jan 14, 2022
* add tree-sitter-git-config

* add todo comment for improving filetype check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants