joaotavora / yasnippet

A template system for Emacs
http://joaotavora.github.io/yasnippet/
2.81k stars 311 forks source link

Emacs Snippets not working anymore with Tree Sitter Major Modules #1169

Closed nicemaker closed 8 months ago

nicemaker commented 1 year ago

I started to use only the new build in tree-sitter for my languages. So for example instead of typescipt-mode I am now using typescipt-ts-mode. Since I switched yasnippets are not showing up anymore. I guess yasnippet doesn't understand that typescript-ts-mode needs the same snippets as typescript mode.

I was basically following instructions here: https://www.masteringemacs.org/article/how-to-get-started-tree-sitter

I was hoping that the major-mode-remap list would take care of that mapping, but that doesn't seem to work for yasnippet. Any ideas?

(setq major-mode-remap-alist
 '((typescript-mode . typescript-ts-mode)
   (python-mode . python-ts-mode)))
cybergrind commented 1 year ago

For you you can get in working by adding (yas-activate-extra-mode 'python-mode) to python-ts-mode-hook

twlz0ne commented 1 year ago

My workaround:

(advice-add 'yas--modes-to-activate :around
 (defun yas--get-snippet-tables@tree-sitter (orig-fn &optional mode)
   (funcall orig-fn
            (or (car (rassq (or mode major-mode) major-mode-remap-alist))
                mode))))
jvillasante commented 11 months ago

@twlz0ne This is not working for me, at least for C++. Any other workaround?

jvillasante commented 11 months ago

In case somebody needs a workaround to work today, I found this that seems to work

twlz0ne commented 11 months ago

@jvillasante Don't know why not working for you, the advice above just tells yas to use /path/to/snippets/lang-mode instead of /path/to/snippets/lang-ts-mode. Have you checked the major-mode-remap-alist and snippet dirs?

(car (rassq 'c++-ts-mode major-mode-remap-alist))
;; => c++-mode

(and (yas--get-snippet-tables 'c++-mode) t)
;; => t
aspiers commented 10 months ago

In case somebody needs a workaround to work today, I found this that seems to work

Thanks a lot @jvillasante, yasnippet-treesitter-shim looks like a great workaround - although IMHO really this should work out of the box with yasnippet rather than needing a separate package as a bandaid :disappointed:

aspiers commented 10 months ago

I got the shim working with use-package and straight.el and submitted https://github.com/fbrosda/yasnippet-treesitter-shim/pull/1 to document this so that others can install it more easily.

monnier commented 10 months ago

looks like a great workaround - although IMHO really this should work out of the box with yasnippet rather than needing a separate package as a bandaid :disappointed:

I just pushed a patch to master which should hopefully help. Can someone confirm it fixes the problem for them?

aspiers commented 10 months ago

@monnier Thanks so much for working on this! I have just tested, and can confirm it works 🚀

BTW I tested it by setting major-mode-remap-alist with the help of treesit-auto.el, although I note that that package only sets major-mode-remap-alist locally by advising set-auto-mode-0:

https://github.com/renzmann/treesit-auto/blob/07a8f924cd4f020a2eb32b45d8543af9556f355d/treesit-auto.el#L466-L484

So I had to set it globally via:

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

This means that while your commit is working as intended, it won't work out of the box with treesit-auto.el yet. I don't yet understand why the latter is only setting major-mode-remap-alist locally not globally, so I'm not sure what the best full solution would be, but I guess the responsibility for fixing that is more likely to lie with treesit-auto.el rather than yasnippet.

aspiers commented 10 months ago

I wrote:

This means that while your commit is working as intended, it won't work out of the box with treesit-auto.el yet. I don't yet understand why the latter is only setting major-mode-remap-alist locally not globally, so I'm not sure what the best full solution would be, but I guess the responsibility for fixing that is more likely to lie with treesit-auto.el rather than yasnippet.

I've filed https://github.com/renzmann/treesit-auto/issues/76 to track this.

monnier commented 10 months ago

This means that while your commit is working as intended, it won't work out of the box with treesit-auto.el yet.

Hmm... I wonder why my code isn't seeing the buffer-local value that treesit-auto sets for major-mode-remap-alist.

AFAICT the yasnippet code should be run inside the buffer and after treesit-auto intervened, so it should see the relevant value.

Could you M-x trace-function RET on the following functions:

yas--modes-to-activate
yas-minor-mode
yas--get-snippet-tables
yas--load-pending-jits
treesit-auto--set-major-remap
set-auto-mode-0

and then see what the trace looks like?

aspiers commented 10 months ago

I will do that when I get a moment (how did I use emacs for 30 years and only just hear about trace-function?!), but I have already seen a weird thing which might explain this:

Somehow treesit-auto correctly executes the setq-local when the file is first visited (proven via edebug), but then when the buffer is opened, major-mode-remap-alist somehow reverts to nil.

aspiers commented 10 months ago

I've commented the same point in https://github.com/renzmann/treesit-auto/issues/76#issuecomment-1877070544, so perhaps that would be a better place to continue this conversation for now, since it currently looks like more of an issue with treesitter-auto rather than yasnippet.

monnier commented 10 months ago

Where do you see that it has value nil? Since it's a buffer-local setting, the buffer in which you are when you look at the value can make all the difference.

You might also want to use M-x debug-on-variable-change on major-mode-remap-alist (which reminds me that we should also add trace-variable to trace.el).

aspiers commented 10 months ago

Yep, I appreciate the difference and did check that carefully: I see it as nil in the buffer of the (Typescript) file where I want yasnippet activated. To clarify, this is what I did:

It's strange, but perhaps something else is resetting the variable to nil. Or more likely, I'm just being an idiot somehow.

Anyhow, I will try debug-on-variable-change which is another great trick I somehow never discovered before! Perhaps I should find more issues so that I can continue learning from you ;-)

aspiers commented 10 months ago

OK, this is interesting and seems relevant! I tried debug-on-variable-change. Sure enough, it triggered the debugger as expected:

Debugger entered--setting major-mode-remap-alist in buffer Options.tsx to ((yaml-mode . yaml-ts-mode) (typescript-mode . typescript-ts-mode) (js2-mode . js-ts-mode) (javascript-mode . js-ts-mode) (js-mode . js-ts-mode) (sh-mode . bash-ts-mode)): 
  debug--implement-debug-watch(major-mode-remap-alist ((yaml-mode . yaml-ts-mode) (typescript-mode . typescript-ts-mode) (js2-mode . js-ts-mode) (javascript-mode . js-ts-mode) (js-mode . js-ts-mode) (sh-mode . bash-ts-mode)) set #<buffer Options.tsx>)
  set(major-mode-remap-alist ((yaml-mode . yaml-ts-mode) (typescript-mode . typescript-ts-mode) (js2-mode . js-ts-mode) (javascript-mode . js-ts-mode) (js-mode . js-ts-mode) (sh-mode . bash-ts-mode)))
  treesit-auto--set-major-remap(tsx-ts-mode nil)
  apply(treesit-auto--set-major-remap (tsx-ts-mode nil))
  set-auto-mode-0(tsx-ts-mode nil)
  set-auto-mode--apply-alist((("\\(\\.js[mx]\\|\\.har\\)\\'" . js-ts-mode) ("\\.ya?ml\\'" . yaml-ts-mode) ("\\.ts\\'" . typescript-ts-mode) ("\\.tsx\\'" . tsx-ts-mode) ("\\.toml\\'" . toml-ts-mode) ("\\.rs\\'" . rust-ts-mode) ("\\(?:\\.\\(?:rbw?\\|ru\\|rake\\|thor\\|jbuilder\\|rabl\\|ge..." . ruby-ts-mode) ("\\.py[iw]?\\'" . python-ts-mode) ("\\.json\\'" . json-ts-mode) ("\\.js\\'" . js-ts-mode) ("\\.java\\'" . java-ts-mode) ("go\\.mod\\'" . go-mod-ts-mode) ("\\.go\\'" . go-ts-mode) ("\\Dockerfile\\'" . dockerfile-ts-mode) ("\\.css\\'" . css-ts-mode) ("\\.cpp\\'" . c++-ts-mode) ("\\.cmake\\'" . cmake-ts-mode) ("\\.cs\\'" . csharp-ts-mode) ("\\.c\\'" . c-ts-mode) ("\\.sh\\'" . bash-ts-mode) ("\\.odc\\'" . archive-mode) ("\\.odf\\'" . archive-mode) ("\\.odi\\'" . archive-mode) ("\\.otp\\'" . archive-mode) ("\\.odp\\'" . archive-mode) ("\\.otg\\'" . archive-mode) ("\\.odg\\'" . archive-mode) ("\\.ots\\'" . archive-mode) ("\\.ods\\'" . archive-mode) ("\\.odm\\'" . archive-mode) ("\\.ott\\'" . archive-mode) ("\\.odt\\'" . archive-mode) ("\\(?:\\(?:\\.\\(?:b\\(?:\\(?:abel\\|ower\\)rc\\)\\|json\\(?:l..." . json-mode) ("\\.scss\\'" . sass-mode) ("\\.sass\\'" . sass-mode) ("\\.haml\\'" . haml-mode) ("\\.\\(phtml\\|tpl\\.php\\|jsp\\|as[cp]x\\|erb\\|mustache\\|..." . web-mode) ("\\.json\\'" . json-mode) ("\\.ya?ml\\'" . yaml-mode) ("\\.\\(e?ya?\\|ra\\)ml\\'" . yaml-mode) ("\\.feature\\'" . feature-mode) ("/\\.zsh\\(env\\|rc\\|/functions/\\)\\|\\.stp$" . sh-mode) ("\\.sc\\'" . sclang-mode) ("\\.gem\\'" . tar-mode) ("\\(\\.\\(e?rb\\|rjs\\|rake\\)\\|Rakefile\\|Guardfile\\)\\'" . ruby-mode) ("\\.py\\'" . python-mode) ("\\.php\\'" . php-mode) ("\\.\\(?:php[s345]?\\|phtml\\)\\'" . php-mode-maybe) ("\\.\\(?:php\\.inc\\|stub\\)\\'" . php-mode) ("/\\.php_cs\\(?:\\.dist\\)?\\'" . php-mode) ...) nil nil)
  #<subr set-auto-mode>()
  so-long--set-auto-mode(#<subr set-auto-mode>)
  apply(so-long--set-auto-mode #<subr set-auto-mode> nil)
  set-auto-mode()
  normal-mode(t)
  after-find-file(nil nil t nil nil)

However after continuing from this, the debugger reactivated again with this:

Debugger entered--killing local value of major-mode-remap-alist in buffer Options.tsx: 
  debug--implement-debug-watch(major-mode-remap-alist nil makunbound #<buffer Options.tsx>)
  prog-mode()
  typescript-ts-base-mode()
  tsx-ts-mode()
  #<subr set-auto-mode-0>(tsx-ts-mode nil)
  apply(#<subr set-auto-mode-0> (tsx-ts-mode nil))
  set-auto-mode-0(tsx-ts-mode nil)
  set-auto-mode--apply-alist((("\\(\\.js[mx]\\|\\.har\\)\\'" . js-ts-mode) ("\\.ya?ml\\'" . yaml-ts-mode) ("\\.ts\\'" . typescript-ts-mode) ("\\.tsx\\'" . tsx-ts-mode) ("\\.toml\\'" . toml-ts-mode) ("\\.rs\\'" . rust-ts-mode) ("\\(?:\\.\\(?:rbw?\\|ru\\|rake\\|thor\\|jbuilder\\|rabl\\|ge..." . ruby-ts-mode) ("\\.py[iw]?\\'" . python-ts-mode) ("\\.json\\'" . json-ts-mode) ("\\.js\\'" . js-ts-mode) ("\\.java\\'" . java-ts-mode) ("go\\.mod\\'" . go-mod-ts-mode) ("\\.go\\'" . go-ts-mode) ("\\Dockerfile\\'" . dockerfile-ts-mode) ("\\.css\\'" . css-ts-mode) ("\\.cpp\\'" . c++-ts-mode) ("\\.cmake\\'" . cmake-ts-mode) ("\\.cs\\'" . csharp-ts-mode) ("\\.c\\'" . c-ts-mode) ("\\.sh\\'" . bash-ts-mode) ("\\.odc\\'" . archive-mode) ("\\.odf\\'" . archive-mode) ("\\.odi\\'" . archive-mode) ("\\.otp\\'" . archive-mode) ("\\.odp\\'" . archive-mode) ("\\.otg\\'" . archive-mode) ("\\.odg\\'" . archive-mode) ("\\.ots\\'" . archive-mode) ("\\.ods\\'" . archive-mode) ("\\.odm\\'" . archive-mode) ("\\.ott\\'" . archive-mode) ("\\.odt\\'" . archive-mode) ("\\(?:\\(?:\\.\\(?:b\\(?:\\(?:abel\\|ower\\)rc\\)\\|json\\(?:l..." . json-mode) ("\\.scss\\'" . sass-mode) ("\\.sass\\'" . sass-mode) ("\\.haml\\'" . haml-mode) ("\\.\\(phtml\\|tpl\\.php\\|jsp\\|as[cp]x\\|erb\\|mustache\\|..." . web-mode) ("\\.json\\'" . json-mode) ("\\.ya?ml\\'" . yaml-mode) ("\\.\\(e?ya?\\|ra\\)ml\\'" . yaml-mode) ("\\.feature\\'" . feature-mode) ("/\\.zsh\\(env\\|rc\\|/functions/\\)\\|\\.stp$" . sh-mode) ("\\.sc\\'" . sclang-mode) ("\\.gem\\'" . tar-mode) ("\\(\\.\\(e?rb\\|rjs\\|rake\\)\\|Rakefile\\|Guardfile\\)\\'" . ruby-mode) ("\\.py\\'" . python-mode) ("\\.php\\'" . php-mode) ("\\.\\(?:php[s345]?\\|phtml\\)\\'" . php-mode-maybe) ("\\.\\(?:php\\.inc\\|stub\\)\\'" . php-mode) ("/\\.php_cs\\(?:\\.dist\\)?\\'" . php-mode) ...) nil nil)
  #<subr set-auto-mode>()
  so-long--set-auto-mode(#<subr set-auto-mode>)
  apply(so-long--set-auto-mode #<subr set-auto-mode> nil)
  set-auto-mode()
  normal-mode(t)
  after-find-file(nil nil t nil nil)

I can't figure out why prog-mode would cause the local value of major-mode-remap-alist to be killed, but I'm guessing you'll know the reason immediately :)

monnier commented 10 months ago

However after continuing from this, the debugger reactivated again with this:


Debugger entered--killing local value of major-mode-remap-alist in buffer Options.tsx: 
  debug--implement-debug-watch(major-mode-remap-alist nil makunbound #<buffer Options.tsx>)
  prog-mode()

Ah, yes, of course: the first thing that a major mode does is kill "all" buffer-local variables (basically, to "reset" the settings installed by any previous major mode).

aspiers commented 10 months ago

Hrm, so does this mean that treesit-auto's approach is fundamentally flawed?

monnier commented 10 months ago

Hrm, so does this mean that treesit-auto's approach is fundamentally flawed?

Don't know, but it does mean that Yasnippet can't get treesit-auto's information via major-mode-remap-alist for free.

    Stefan
aspiers commented 10 months ago

OK thanks, so for now the workarounds are either to use https://github.com/fbrosda/yasnippet-treesitter-shim or my hack of setting it globally:

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

I think any further discussion on the treesit-auto side of things belongs in https://github.com/renzmann/treesit-auto/issues/76.

monnier commented 10 months ago

OK thanks, so for now the workarounds are either to use https://github.com/fbrosda/yasnippet-treesitter-shim or my hack of setting it globally:

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

Yes. treesit-auto (and major-mode-remap-alist) is designed to redirect FOO-mode to FOO-ts-mode, whereas YASnippet needs something different, which is to know that FOO-ts-mode is a "kind of" FOO-mode. This should be used both for the users that want to redirect FOO-mode to FOO-ts-mode and for those that don't do that but which invoked FOO-ts-mode some other way (such as manually).

So I think the canonical way should be for the major mode to use derived-mode-add-parents to declare its relationship. I just pushed to master a patch which makes YASnippet use derived-mode-all-parents so as to obey such settings.

Since this functionality is new in Emacs-30, the code still tries to use major-mode-remap-alist, but that's just a crutch which will cover only some configs 🙁.

BTW, instead of having "dummy" snippets with .yas-parents pointing to the real snippets, like yasnippet-treesitter-shim does, maybe we should allow snippets to have not only .yas-parents but also .yas-children.

aspiers commented 10 months ago

Yep, that all makes sense.