Eglot integration (snippet expansion)

[KaranAhlawat]

While using corfu, and having yas-minor-mode enabled, I was able to jump precisely through the fields of the snippet provided by eglot (the language server, to be more precise). However, now that I'm trying out company-mode, when I autocomplete some LSP suggestion that is also a snippet, the cursor just ends up skipping over the snippet's field.

as seen in the image.
Is there some way to fix that?

[KaranAhlawat]

1. I do not have TNG mode enabled.
2. The language server is metals.
3. I was unable to find a fix for any similar issues. (or even similar issues, [un]fortunately).

[KaranAhlawat]

It half worked. In the first placeholder, it did what you'd expect it to. In the second one however, it doesn't really replace the text (although the placeholders aren't just $0 etc in this case).

[dgutov]

I think the second placeholder isn't supposed to be shown in that case -- it's just the last position for point.

Here's a video with me expanding that snippet, and with using a snippet expansion in a function call too: https://imgur.com/a/nItyrqG

[KaranAhlawat]

For me it goes like this -

Screencast.from.2023-10-16.23-39-26.webm

And the other one goes like tihs (what prompted me to make the issue) -
Screencast from 2023-10-16 23-40-58.webm

[dgutov]

Yeah, the text expression shouldn't stay around.

You can try upgrading to the latest version of eglot/company/yasnippet, and if it doesn't help.

Or try reproducing the problem starting with emacs -Q (to verify the absence of any conflicting settings and packages).

Failing all that, Eglot has specific instructions for reporting a bug (listing all current versions et cetera).

[joaotavora]

I couldn't reproduce: joaotavora/eglot#1308 (reply in thread)

[KaranAhlawat]

I tried with emacs -Q, still running into the same issue. And I somehow broke my PGTK build upgrading my packages XD

[dgutov]

I suppose a really far-out hypothesis could be that the PGTK build is interfering somehow. Do you build Emacs from source?

Any chance you can switch to lucid (the result should run okay through XWayland) and compare?

[dgutov]

And you're sure that when using Corfu you didn't have any such problems?

[KaranAhlawat]

I suppose a really far-out hypothesis could be that the PGTK build is interfering somehow. Do you build Emacs from source?

Any chance you can switch to lucid (the result should run okay through XWayland) and compare?

Yes, I do build from source (debian repos are on 28). I don't see how it'd interfere, but I can find some time to build a lucid build.

And you're sure that when using Corfu you didn't have any such problems?

Well, I went back to Corfu after our discussions yesterday, and it was doing the same thing now, so, it did have those problems as well I suppose 😄 Although I now think it's a Yasnippet thing rather than corfu or company.

I can 100% say corfu and yasnippet worked as expected before I switched to company, because I remember using that feature just the day before while working on a Scala project.

[KaranAhlawat]

Doesn't look like I'm the only one who ran into this either as per this issue. As at least we know it's not entirely unique, although the symbol-overlay package isn't used by me so that solution was a dead-end.

[dgutov]

That issue mentions Emacs 28.1, and there was no pgtk back then :/

[dgutov]

Tangentially from the bug, it seems like $0 is not so useful in these snippets.

For example, return expands into return {$0:expression};, meaning at the end the point remains before the semicolon. That seems less useful than being able to TAB again moving point to eol and pressing RET to open the next line.

But VS Code behaves this too, so I suppose this is a choice made by the language servers (at least clangd).

[KaranAhlawat]

Yeah I think it's actually a decision that iirc was decided in the Microsoft languag-server-protocol repository.

[dgutov]

Was there a discussion when to use the $0 placeholder? As opposed to $1, etc.

[KaranAhlawat]

Ah no, looking it up, it was about something else (signature help while using snippets), sorry.

[KaranAhlawat]

Okay! Lots of new information, which will hopefully put this issue to rest.

1. Upgrading to clangd-15 also gives me the same completions as @joaotavora, with the last tab stop as $0 instead of ${0:expression}.
2. Apparently for metals, the issue was me modify the client capabilities and setting dynamicRegistration to true? Or maybe in general just modifying the client capabilities (which shouldn't break, but maybe it's just the dynamicRegistration part). Anyhow, if I set that, the server doesn't include the includeTextFormat field in the completion item it sends back, and then eglot doesn't treat it as a snippet. Not setting it, however, does make the server indicate the completion is a snippet which eglot can handle properly.

So, I think the only "bug" would be that ${0:text} doesn't replace the text part, as in joaotavora/yasnippet#1141. But that is a yasnippet.

[joaotavora]

I can't reproduce that either. I had asked you already if you can successfully expand the following in your *scratch* buffer after M-x yas-minor-mode:

(yas-expand-snippet "hello ${0:world}")

I can, flawlessly, in both Emacs 30 and Emacs 29. The decision whether to completely delete the "world" after the first typed character is controlled by delete-selection-mode (defaults to nil, but I think most people set it to t).

[KaranAhlawat]

Alright, that works. Thank you both very much!

[dgutov]

Aha! Now I get it too.

@joaotavora Should we do something more about this issue? It came up before (another example: emacs-lsp/lsp-dart#130). The starter packs like Doom help have their own workarounds.

I guess in the past having a default value for the $0 stop wasn't very popular (there are no such examples in the yasnippet-snippets collection), but with snippets coming from LSP it now happens more. And there is no visual indication that this kind of "final" field is different from the other template fields (that it is, in fact, just active region after point). The wrong default value of delete-selection-mode affects what looks like the expected behavior.

We could add a mention to the docs. But most don't read them until they really have to. Maybe we should really change yasnippet's handling of $0? When there is a default text, it would add an overlay just like in other cases, with the only difference that the last TAB won't move point later. Curiously, TAB currently works as expected there (aside from saying "Indenting the region" in the echo area 😝).

[joaotavora]

In my testing, even with delete-selection-mode set to nil I still see the region overlay, it's just not automatically deleted. Which is the expected behavior.

I'm not sure I like the overlay idea. It used to be an overlay but I more or less distinctly deciding it should just be a region. My reasoning was that yasnippet has done its job by that point, the snippet has exited, no more navigation possible etc. If there was this overlay we would need to clean it up somehow. The visual region machinery should take care of that.

[dgutov]

In my testing, even with delete-selection-mode set to nil I still see the region overlay, it's just not automatically deleted. Which is the expected behavior.

The region overlay at the end looks just like any other template field, so the difference in behavior (when it happens) is puzzling. How long did we take to understand the source of this complaint here?

My reasoning was that yasnippet has done its job by that point, the snippet has exited, no more navigation possible etc. If there was this overlay we would need to clean it up somehow.

We would clean it up like the others. Like the last overlay in a template without $0, e.g. "hello ${1:beautiful} ${2:world}".

Again, here's a comment in that other issue from the DartCode author and Analysis Server developer: emacs-lsp/lsp-dart#130 (comment)

Perhaps $0 itself is special, in that the "point moved away" hooks are less likely to fire soon, so the deletion of the overlay is predicated on the user typing something (increasing the odds of something going wrong before that), but wherever that decision was made (some discussion, perhaps, or another bug report) seems worth revisiting.

Maybe ideally that variable's default would change, but that seems unlikely to happen in any near future.