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

feat: add tree-sitter-make #1433

Merged
merged 2 commits into from
Jan 8, 2022

Conversation

EricCrosson
Copy link
Contributor

This commit adds syntax highlighting for GNU Make1 makefiles
via tree-sitter-make2.

Footnotes

  1. https://www.gnu.org/software/make/

  2. https://github.com/alemuller/tree-sitter-make

@EricCrosson
Copy link
Contributor Author

Hey guys! Helix is an awesome project, I've only found it in the last few days but I love what I've seen so far. Thanks so much for your hard work! What a great idea to build an editor from the ground up with LSP and tree-sitter in mind.

I've used GNU Emacs for 10 years with some thousands of lines of configuration. Lately I've been using neovim, where I have ~100 lines of config. I'm happy with helix with all of 5 lines of config, and LSP works better and faster with helix than any of my other editors. This team is on to something!

I tried editing a GNU Makefile today and found there is no syntax highlighting. This PR imports the preexisting highlights.scm file from tree-sitter-make, as well as the tree-sitter-make submodule. I am also pretty new to tree-sitter so I didn't try my hand at writing the indents.toml, injections.scm, or the textobjects.scm files. Hopefully those are optional or we can iterate to flush out the functionality over time.

I'm not sure if you have any conventions for tracking existing scm files that are preferred over copy/paste, so just let me know if there are any changes requested. Cheers!

Copy link
Member

@dead10ck dead10ck left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

languages.toml Outdated
[[language]]
name = "make"
scope = "source.make"
file-types = ["Makefile"]
Copy link
Member

@dead10ck dead10ck Jan 4, 2022

Choose a reason for hiding this comment

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

Could we also add the other variants, makefile and GNUmakefile? And also could we throw in justfile and .justfile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good thinking!

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, I thought there is tree-sitter-just? https://github.com/IndianBoy42/tree-sitter-just

Copy link
Member

Choose a reason for hiding this comment

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

Oh interesting. Is it good to use? It mentions it's a work in progress and doesn't have tests yet.

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Thank you for the kind words!

The PR looks good but highlights.scm needs some changes to follow the list of scopes we use: https://docs.helix-editor.com/themes.html#syntax-highlighting

"if"
"or" ; boolean functions are conditional in make grammar
"and"
] @conditional
Copy link
Member

Choose a reason for hiding this comment

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

@keyword.control.conditional

"and"
] @conditional

"foreach" @repeat
Copy link
Member

Choose a reason for hiding this comment

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

@keyword.control.repeat

"include"
"sinclude"
"-include"
] @include
Copy link
Member

Choose a reason for hiding this comment

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

@keyword.control.import

"error"
"warning"
"info"
] @exception
Copy link
Member

Choose a reason for hiding this comment

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

@keyword.control.exception

Hmm, that one looks like it's missing from the list, can you add it here?

- `conditional` - `if`, `else`

Comment on lines 121 to 125
(variable_assignment
name: (word) @constant)

(variable_reference
(word) @constant)
Copy link
Member

Choose a reason for hiding this comment

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

Seems to me that these should be @variable?


(comment) @comment

((word) @clean @string.regex
Copy link
Member

Choose a reason for hiding this comment

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

@string.regexp


(function_call
function: "error"
(arguments (text) @text.danger))
Copy link
Member

Choose a reason for hiding this comment

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

@error

Copy link
Contributor Author

@EricCrosson EricCrosson Jan 4, 2022

Choose a reason for hiding this comment

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

I don't see @error on this list -- is this a scope that should also be added to themes.md?

Oh I see it, under the Interface heading 👍


(function_call
function: "warning"
(arguments (text) @text.warning))
Copy link
Member

Choose a reason for hiding this comment

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

@warning


(function_call
function: "info"
(arguments (text) @text.note))
Copy link
Member

Choose a reason for hiding this comment

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

@info

EricCrosson added a commit to EricCrosson/helix that referenced this pull request Jan 4, 2022
@EricCrosson
Copy link
Contributor Author

The PR looks good but highlights.scm needs some changes to follow the list of scopes we use: https://docs.helix-editor.com/themes.html#syntax-highlighting

Thanks for the pointer, and the complete review! I've updated the scopes and added exception to the themes documentation

[[language]]
name = "make"
scope = "source.make"
file-types = ["Makefile", "makefile", "justfile", ".justfile"]
Copy link
Contributor

Choose a reason for hiding this comment

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

"justfile" will cover files with justfile as their name or as their extension.

Suggested change
file-types = ["Makefile", "makefile", "justfile", ".justfile"]
file-types = ["Makefile", "makefile", "justfile"]

Copy link
Member

Choose a reason for hiding this comment

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

Does that include .justfile by itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that is what this is meant to cover, my mistake.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove just, there is https://github.com/IndianBoy42/tree-sitter-just

EricCrosson added a commit to EricCrosson/helix that referenced this pull request Jan 4, 2022
EricCrosson added a commit to EricCrosson/helix that referenced this pull request Jan 6, 2022
This commit adds syntax highlighting for GNU Make[^1] makefiles
via tree-sitter-make[^2].

[^1]: https://www.gnu.org/software/make/
[^2]: https://github.com/alemuller/tree-sitter-make
@archseer
Copy link
Member

archseer commented Jan 8, 2022

Thanks! 🎉

@archseer archseer merged commit 5b45bdd into helix-editor:master Jan 8, 2022
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.

5 participants