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

Treesit.el Support #48

Closed
AndrewSwerlick opened this issue Jan 4, 2023 · 41 comments
Closed

Treesit.el Support #48

AndrewSwerlick opened this issue Jan 4, 2023 · 41 comments

Comments

@AndrewSwerlick
Copy link

Hey @jcs090218 thanks for this great package, I think its a hugely important feature set in the emacs tree-sitter ecosystem. I'm curious what your plans are with regards to the new [built in treesit.el](https://github.com/emacs-mirror/emacs/blob/emacs-29/lisp/treesit.el) that is shipping with Emacs 29? Are you interested in having this package work with that?

I started experimenting with what that might mean in this PR and found the process of integrating with treesit.el to be pretty straight forward, but I wanted to understand how you were thinking about this before going any further.

If you're interested, I can go further with the PR. There are some unanswered questions, like would you want older versions to support the legacy emacs-tree-sitter package, and new versions to only support treesit.el, or one version that could swap between them etc. Interested to hear your thoughts.

@jcs090218
Copy link
Member

jcs090218 commented Jan 4, 2023

Hi, Andrew! Thanks for liking this package! ❤️ Yes, I am interested to see things going with built-in tree-sitter support!

I am thinking of creating another package for built-in treesit support since ts-fold is heavily designed with emacs-tree-sitter. It depends on the complexity of switching between the two APIs! If it were easy to implement and wouldn't burden maintainers, then it should be good to build together!

If we go for the first route, it will be more straightforward since we would only have one abstraction layer regarding treesit. If we decide to build together, it's better to support both tree-sitter abstraction layers. The ultimate goal is to move from legacy emacs-tree-sitter to built-in treesit, so it's fine to drop the legacy support at some point. We should wait until treesit becomes popular, or consider stable.

Thank you for working on this. I might wait until Emacs 29 is out before I start working on this! Feel free to continue working on this! 😄

@AndrewSwerlick
Copy link
Author

Cool, what I'll probably do is keep playing with my prototype to see if I can easily support both approaches in a single version. That will make it easier to merge changes from upstream too.

@garyo
Copy link

garyo commented Feb 1, 2023

I'm also super interested in this! Would be willing to help.

@AndrewSwerlick
Copy link
Author

AndrewSwerlick commented Feb 1, 2023

@garyo I have a fork working here that I'm using daily with both ruby and js. It seems to be working well.

I explored trying to get something that could easily toggle between built in tree sitter and the package, and I couldn't do it without basically writing a big whole compatibility later, which didn't seem worth it.

In general the transformation was just a find and replace though, with a few minor complications

@garyo
Copy link

garyo commented Feb 2, 2023

I'm trying your fork now (the andrew-sw/treesit-el-support branch of that fork). I had to modify the ts-fold-range-alist to add the new -ts- major modes in the latest emacs, e.g. c++-ts-mode, but then it works OK.

Perhaps an alternative to doubling the size of that alist would be to look up using symbols that remove the "-ts", something like (intern (replace-regex-in-string "-ts-mode" "-mode" (symbol-value major-mode))?

The indicator mode isn't working yet but I can look into that.

@garyo
Copy link

garyo commented Feb 2, 2023

I also found that ts-fold-close-all needs this small patch:

@@ -307,7 +319,7 @@ If the current node is not folded or not foldable, do nothing."
   (interactive)
   (ts-fold--ensure-ts
     (let* ((node (treesit-buffer-root-node))
-           (patterns (mapconcat (lambda (fold-range) (concat "(" (car fold-range) ") " "@name"))
+           (patterns (mapconcat (lambda (fold-range) (concat "(" (symbol-name (car fold-range)) ") " "@name"))
                                  (alist-get major-mode ts-fold-range-alist) " "))
            (query (treesit-query-compile (treesit-node-language node) patterns))
            (nodes-to-fold (treesit-query-capture node query nil nil t)))

@garyo
Copy link

garyo commented Feb 2, 2023

@AndrewSwerlick patches sent to your fork. Thanks for that!

@AndrewSwerlick
Copy link
Author

I'm trying your fork now (the andrew-sw/treesit-el-support branch of that fork). I had to modify the ts-fold-range-alist to add the new -ts- major modes in the latest emacs, e.g. c++-ts-mode, but then it works OK.

Perhaps an alternative to doubling the size of that alist would be to look up using symbols that remove the "-ts", something like (intern (replace-regex-in-string "-ts-mode" "-mode" (symbol-value major-mode))?

It seems we could remove the non ts-mode items if we aren't going to try to support both versions at the same time.

@jcs090218 jcs090218 pinned this issue Feb 27, 2023
@LazerJesus
Copy link

what is the status of this? is it considered safe to use ts-fold with emacs@29?

@jcs090218
Copy link
Member

jcs090218 commented Mar 10, 2023

Hi, I will work on this after April, when 29.1 is released. I've tried 29.0.60 but couldn't get it to run smoothly. I will investigate more once I better understand treesit.el.

To answer your question, yes, it's safe to use this package in 29. However, it does not use the built-in treesit module. Hope the built-in tree-sitter support will be soon in the near future!

@garyo
Copy link

garyo commented Mar 10, 2023

I am using Andrew's fork plus some patches (see above), my patches at AndrewSwerlick#1, with emacs built from trunk a few days ago, with built-in treesit module. There are still some things to iron out with treesit-based major modes but it is basically functional.

@AndrewSwerlick
Copy link
Author

I'll add I use my fork on my daily driver for both ruby and typescript with minimal issues. I haven't tested extensively with other languages though.

@jcs090218
Copy link
Member

jcs090218 commented Mar 16, 2023

Thanks for all the information! One major issue is I use windows pretest snapshot, so it's not always on the latest master branch. Emacs-devel may have made some changes, so it causes API to be incompatible (it's normal). I will make sure I go deeper next time I try it out! ;)

@abougouffa
Copy link

FOR INFORMATION
I've started a fork of this package to port it to treesit for my personal use. I've got a usable treesit-fold package that I'm using in my Emacs installation.

https://github.com/abougouffa/treesit-fold

@garyo
Copy link

garyo commented Nov 17, 2023

@abougouffa this looks good; how does it compare to @AndrewSwerlick 's version I wonder?

@abougouffa
Copy link

@garyo I was using @AndrewSwerlick's fork, but since he didn't update it from a while, I've tried to port ts-fold from the current master branch. I did inspire from Andrew's fork though.

I think it needs more testing, and I need to learn more about tree-sitter to be able to maintain it.

@jcs090218
Copy link
Member

Feel free to open PRs for this. As long it works on both tree-sitter.el and treesit.el, I am happy to merge it! :)

@AndrewSwerlick
Copy link
Author

@garyo I was using @AndrewSwerlick's fork, but since he didn't update it from a while

Yeah I'm not proactively maintaining my fork, but I am still using it, and will fix things that affect me or others as time allows. I haven't updated anything in a while because everything has just worked from my personal perspective.

@guibor
Copy link

guibor commented Jan 12, 2024

Hi! Is there still an intention of merging fs-fold into build-in treesit.el?

@jcs090218
Copy link
Member

Hi! Is there still an intention of merging fs-fold into build-in treesit.el?

Yes, we are! However, I haven't had a chance to use treesit.el, therefore, it won't be integrated any time soon.

@ChauhanT
Copy link

ts-fold was a key package in my emacs 28 config. Is there a workaround to make it work in emacs 29 ?

@jcs090218
Copy link
Member

I think you can use one of the forks from @AndrewSwerlick and @abougouffa. As stated above, I haven't had time to try the built-in treesit.el, but I am open for PRs if anyone wants to contribute to this package! :)

@DevelopmentCool2449
Copy link

DevelopmentCool2449 commented Apr 13, 2024

Hello, thank you maintainers for creating and improve this wonderful package, i tried to see how treesit.el library can be included in this package, however ts-fold has grown and matured enough that including support for treesit.el can complicate the development and maintenance of this package, i might suggest on making a standalone fork for supporting treesit.el based on @abougouffa one, and submit it to MELPA/NonGNU ELPA (and if possible to GNU ELPA too), i can help in making a fork of @abougouffa one and make little changes.

@jcs090218
Copy link
Member

I don't think is that hard since we only need to handle translations between tree-sitter.el nodes and treesit.el nodes. Andrew has an early implementation in #47. 🤔 TBH, we only need a flag to let users decide what backend they want.

Another approach is to have a standalone package that requires this package as a dependency. That way, we don't have to rewrite the logic in ts-fold and only focus on the translation part. WDYT?

@abougouffa
Copy link

Hi @DevelopmentCool2449

It would be nice to do a standalone fork. My fork was just a first try, and being not an expert on how Tree-sitter works, I didn't manage to get the fork to work correctly on all cases (I remember having Emacs crashing when I try to fold/unfold some complex C/C++ files.

As for now, I'm using treesitter-context-fold-mode from treesitter-context which provide a basic support for code folding using built-in treesit.el.

@DevelopmentCool2449
Copy link

DevelopmentCool2449 commented Apr 14, 2024

I don't think is that hard since we only need to handle translations between tree-sitter.el nodes and treesit.el nodes. Andrew has an early implementation in #47. 🤔 TBH, we only need a flag to let users decide what backend they want.

You are right, watching that PR i think i can do an experimental treesit.el support based in that
for availables parsers/modes in ts-fold.
If it were to work I would like to send you a recipe for MELPA (i don't know about Non-GNU ELPA).

Another approach is to have a standalone package that requires this package as a dependency. That way, we don't have to rewrite the logic in ts-fold and only focus on the translation part. WDYT?

It seems like a good idea to me, however i don't know if it would require install tree-sitter.el,
A fork would allow only focusing in treesit.el and avoiding maintaining both implementation in 1 package.

@jcs090218
Copy link
Member

It seems like a good idea to me, however i don't know if it would require install tree-sitter.el,
A fork would allow only focusing in treesit.el and avoiding maintaining both implementation in 1 package.

You are right. 👍 One thing worth mentioning is that tree-sitter.el supports non -ts modes, but treesit.el does. So, it covers both cases. 🤔

@abougouffa
Copy link

You are right. 👍 One thing worth mentioning is that tree-sitter.el supports non -ts modes, but treesit.el does. So, it covers both cases. 🤔

By default, treesit.el is activated on -ts modes, however, we can use treesit functionalities on classical non -ts modes just by running:

;; Create `treesit` grammar for a XML file when in `nxml-mode'
(when (and (treesit-available-p) (treesit-language-available-p 'xml))
  (treesit-parser-create 'xml))

I use this regularly to use treesit based packages on non ts modes.

@DevelopmentCool2449
Copy link

DevelopmentCool2449 commented Apr 14, 2024

Well, then i think i may start in the fork, i don't know if implementing it to ts-fold would require using or installing tree-sitter.el which would install all the parsers.

That would generate garbage for users that only want using treesit.el

If not, then i could start a PR instead forking.

@jcs090218
Copy link
Member

By default, treesit.el is activated on -ts modes, however, we can use treesit functionalities on classical non -ts modes just by running:

👍

It makes more sense to have a new fork. :)

@jcs090218
Copy link
Member

@DevelopmentCool2449 If you tend to create a fork, it's probably better to make it under this org so I can help maintain the core logic. However, I don't have permission to do so. Let me ping the owner of this org, and let’s see what he thinks.

cc @ubolonton WDYT?🤔

@DevelopmentCool2449
Copy link

DevelopmentCool2449 commented Apr 23, 2024

I think it's better that I open a PR for implement this when I have time, since there is no response from the owner.

@ubolonton
Copy link

@jcs090218 That's strange. According to settings, any member can create public repository.

Can you double-check? If you still cannot, let me know:

  • The desired name of the new repo.
  • Whether it should be empty, or be a fork of this one.

@jcs090218
Copy link
Member

@ubolonton Thank you! I think I can create the repo, but I'm not sure if I have the correct permission! 🤔

The repo name should probably be treesit-fold; empty should be good! :)

cc @DevelopmentCool2449

@ubolonton
Copy link

I've created it here: https://github.com/emacs-tree-sitter/treesit-fold

@DevelopmentCool2449
Copy link

Hi @ubolonton thank you for creating the repo,
as soon as I can I will start creating the fork.

@jcs090218
Copy link
Member

as soon as I can I will start creating the fork.

@DevelopmentCool2449 I will first push everything from this repo; then, we can start working on the transition! :)

@DevelopmentCool2449
Copy link

as soon as I can I will start creating the fork.

@DevelopmentCool2449 I will first push everything from this repo; then, we can start working on the transition! :)

TY.

@abougouffa
Copy link

@DevelopmentCool2449 Any news about this?

@jcs090218
Copy link
Member

People can now open PRs at https://github.com/emacs-tree-sitter/treesit-fold. I'll start merging them, but I'll need people to verify it works. 🤔

@jcs090218
Copy link
Member

Move discussion to emacs-tree-sitter/treesit-fold#2.

agzam added a commit to agzam/.doom.d that referenced this issue Jun 16, 2024
- it's improper to use evil-insert for switching Evli to insert mode,
you need to use evil-insert-state

- consult-yasnippet replaces yas-insert-snippet, because it allows to
search for both - key and the snippet name

- adds anki-editor-ui

- treesit-fold, I think the issue with folding is fixed
emacs-tree-sitter/ts-fold#48

- mpv stuff has it's own file now
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

No branches or pull requests

9 participants