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

WIP: Add treesit font-lock support #495

Closed
wants to merge 11 commits into from

Conversation

wkirschbaum
Copy link
Contributor

@wkirschbaum wkirschbaum commented Oct 17, 2022

Add "native" tree-sitter font-lock support.

This PR is for the feature/tree-sitter emacs branch when it is merged in, but should not break for older versions of emacs, so should be safe to merge.

The emacs starter guide for treesit:
https://github.com/emacs-mirror/emacs/blob/feature/tree-sitter/admin/notes/tree-sitter/starter-guide

Known issues:

  • There are some small issues regarding docstrings when fontification is out of sync, which can be fixed with C-x x f
  • String interpolation does not show correctly ( This seems to be an issue with the emacs build at the moment, but not sure )

For the near future:

  • Add indentation support
  • Add navigation support

To figure out

  • Add heex support

Additional implementation can be found here: https://github.com/wkirschbaum/elixir-mode/blob/main/elixir-mode.el. I will copy over when it feels stable enough using it daily.

@jsmestad
Copy link
Contributor

@wkirschbaum have you seen if treesit branch supports injections?

@wkirschbaum
Copy link
Contributor Author

@jsmestad I am not sure yet, but seems like there is a possibility as it is possible to add more than one language for the treesit-settings. I want to refine indentation and navigation and will have a look in more details if no-one else beats me to it.

@victorolinasc
Copy link
Contributor

@wkirschbaum this is really awesome and thans a lot for contributing :)

I think this must hold on for a bit though. The guides are a moving target for now. This is awesome to be tested internally by us but I'd hold off at least until the branch is in master. This is because from what I can follow from the mailing list, there are subtle changes happening that would make the work here easier. For example the detection if the feature is available or if the language is installed.

Wdyt?

@wkirschbaum
Copy link
Contributor Author

wkirschbaum commented Oct 18, 2022 via email

@victorolinasc
Copy link
Contributor

Awesome! Thanks a lot once again and I'll certainly test it more broadly once it is in master too. This is a most welcome addition and is exactly in line with what we were discussing in #465

Please keep this work going :)

@wkirschbaum wkirschbaum changed the title Add treesit font-lock support WIP: Add treesit font-lock support Oct 19, 2022
@wkirschbaum
Copy link
Contributor Author

wkirschbaum commented Oct 23, 2022

I updated to the latest changes and also added imenu indexing, which gives which-function support as well. For know I want to keep it basic and leave refinements to this for later iterations once everything else is in place.

image

Yuan also fixed a bug I reported regarding navigation, so now we can attempt getting nicer navigation going. Indentation is in place, but it is time consuming to tweak it and using my weekly coding to get a feel for what is still missing.

@wkirschbaum
Copy link
Contributor Author

@wkirschbaum have you seen if treesit branch supports injections?

I see there is a function to allow embedded languages for treesit, but failed to get it to work. I am waiting a response from emacs-devel on this.

The latest commit should work on emacs master as tree-sitter got merged yesterday if anyone wants to give it a try. There are some known issues, but will start revisiting them as things are becoming a bit more stable.

@wkirschbaum
Copy link
Contributor Author

@jsmestad @victorolinasc I am thinking about closing this PR and submit the elixir-ts-mode and heex-ts-mode to emacs core? Any thoughts?

Since the convention now seems to keep the mode's separate for those without a tree-sitter emacs compilation to just use the "normal" elixir-mode it makes sense to not merge this in, so want to close this PR.

@jsmestad
Copy link
Contributor

jsmestad commented Dec 7, 2022

I think it makes sense to host them together if for no other reason then discoverability. If I saw elixir-mode.el then elixir-ts-mode.el in the same repo, it's easier to know they both exist then using GH search.

Just my 2c

@victorolinasc
Copy link
Contributor

With the elixir-ts-mode in core I think we have the following scenario:

  • People with Emacs 29+ would use the default grammar maintained by the community and it would be an out-of-the-box experience
  • It would need to be maintained there separately from where we keep the other mode which could give us somehow a burden on feature parity (although it will always be different from the feature parity standpoint)
  • Major changes to treesitter would probably carry those to elixir "automatically" which is a good thing
  • The contribution model is not the most common (no PRs or things like that if I understand it properly)
  • All the code would be GPL all the way which I believe is a good thing

So, I think it would be good, though I'd collect a bit more feedback here and on other major modes.

@wkirschbaum
Copy link
Contributor Author

I am closing this PR for now as it won't make sense to merge this into the same package at this time, but perhaps later we can see how it will work.

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.

None yet

4 participants