renzmann / treesit-auto

Automatic installation, usage, and fallback for tree-sitter major modes in Emacs 29
GNU General Public License v3.0
353 stars 27 forks source link

Doesn't work with yasnippet #76

Open aspiers opened 6 months ago

aspiers commented 6 months ago

@monnier just announced that he's patched yasnippet to honor major-mode-remap-alist, so that for example if it contains the cons cell (typescript-mode . typescript-ts-mode), then activating typescript-ts-mode will also activate all the snippets for typescript-mode.

However, as I noted in my reply to his comment, it won't work out of the box with treesit-auto yet because the latter is only setting major-mode-remap-alist locally not globally. I don't yet understand why it was coded in this way, so I don't have a good solution to suggest yet, but hopefully someone here will :grin:

aspiers commented 6 months ago

Actually, as noted here, I suspect the problem is not so much that it's being set via setq-local, but instead that after the file is loaded, major-mode-remap-alist somehow reverts to nil.

aspiers commented 6 months ago

In https://github.com/joaotavora/yasnippet/issues/1169#issuecomment-1877387431, @monnier has advised that resetting local variables is expected behaviour for activation of any major mode.

aspiers commented 6 months ago

See https://github.com/joaotavora/yasnippet/issues/1169#issuecomment-1877713541 for a recommended way forward here using derived-mode-add-parents.

renzmann commented 6 months ago

... the latter is only setting major-mode-remap-alist locally not globally. I don't yet understand why it was coded in this way ...

The motivation here (and I agree that it's probaby a bit more convoluted than it needs to be) is so that we can revert any changes made by turning global-treesit-auto-mode, which is in keeping with the first ask in the GNU Emacs Lisp coding conventions. Since we're augmenting the major mode Emacs chooses for a buffer, this counts as "changing Emacs' editing behavior." I like to stick to these conventions as best we can for inclusion in MELPA. If we were to modify major-mode-remap-alist globally, there's no guarantee that we can revert our changes if you modify that alist and then turn off global-treesit-auto-mode later.

Whether or not we got the implementation right is definitely up for debate, since this is my first try at it and there's a lot I don't know about how Emacs works under the hood 🤷

I liked your snippet here:

(setq major-mode-remap-alist (treesit-auto--build-major-mode-remap-alist))

since this effectively does what I originally wanted this package to do: just set the damn major-mode-remap-alist. My current advice would be to continue using this if you don't care about the "revert" behavior I mentioned above. I haven't seen this derived-mode-add-parents before, since it looks like it might be for Emacs 30? My hope is that treesit-auto won't be needed for very long, and we get better built-in support for what I've hacked around in this package, but I can't quite tell from that conversation whether this is resolved or a solid workaround exists.

monnier commented 6 months ago

The motivation here (and I agree that it's probaby a bit more convoluted than it needs to be) is so that we can revert any changes made by turning global-treesit-auto-mode,

FWIW, your code made this clear in the comments, indeed, thanks. It has its pros and cons, but I can't suggest a solution that's universally better 🙁

I haven't seen this derived-mode-add-parents before, since it looks like it might be for Emacs 30?

Yes, it's new.

My hope is that treesit-auto won't be needed for very long, and we get better built-in support for what I've hacked around in this package,

Agreed.

but I can't quite tell from that conversation whether this is resolved or a solid workaround exists.

It's still not solved, no. It's probably going to take some time.