elixir-editors / emacs-elixir

Emacs major mode for Elixir
446 stars 94 forks source link

WIP: Add treesit font-lock support #495

Closed wkirschbaum closed 1 year ago

wkirschbaum commented 1 year ago

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:

For the near future:

To figure out

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 commented 1 year ago

@wkirschbaum have you seen if treesit branch supports injections?

wkirschbaum commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

No, lets hold off merging till its in master. things are changing daily. I have some working version of indentation and navigation to add soon too... been using it for the last two days and already prefer it. The branch should work fine if merged, but there is absolutely no rush from my side.

On Tue, 18 Oct 2022, 23:59 Victor Oliveira Nascimento, < @.***> wrote:

@wkirschbaum https://github.com/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?

— Reply to this email directly, view it on GitHub https://github.com/elixir-editors/emacs-elixir/pull/495#issuecomment-1283053843, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAK5POYLK5Z5SDMCCKUNTXTWD4MURANCNFSM6AAAAAARHLPOHU . You are receiving this because you were mentioned.Message ID: @.***>

victorolinasc commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

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

wkirschbaum commented 1 year ago

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.