feat(lsp): completion side effects

[MariaSolOs]

Closes #25714

This PR introduces vim.lsp.completion with the goal of supporting the completion behaviours (snippet expansion, execution of commands, application of text edits) specified by LSP.

To enable completions for a given buffer and client, users should call vim.lsp.completion.enable(true, client_id, bufnr, opts). Currently the options parameter includes the autotrigger property, which when set will request completions after a trigger character has been typed.

This PR also sets new default keymaps for the snippet navigation:

• <Tab> will jump to the next tabstop if the snippet is active and jumpable forwards.
• <S-Tab> will jump to the previous tabstop if the snippet is active and jumpable backwards.

If these keys are being used in other keymaps, they will be restored once the snippet session ends.

[justinmk]

Reluctant to add "attach to client" (or "attach to completion") as yet another way to extend the LSP client. There are already so many ways: start, callbacks, protocol handlers (local and global).

Make it opt-in via a vim.lsp.completion.attach(client, bufnr, [opts]) method

If we enabled it by default, then it could be opt-out, by documenting how to delete the CompleteDone handler. Any problem with that?

Alternatively, for opt-in behavior, we could document how to define a CompleteDone handler that calls a function we provide.

[MariaSolOs]

I think I prefer opt-in, at least while we develop this feature and make sure it has everything we want/need of core LSP completion.

[MariaSolOs]

I've made the feature enabled by default, documenting how to disable it if desired. Lmk what you think @justinmk :)

[MariaSolOs]

After further discussion, I think that making the feature opt-in is better for now. That will facilitate passing options (see Mathias' comment).

[justinmk]

Reluctant to add "attach to client" (or "attach to completion") as yet another way to extend the LSP client. There are already so many ways: start, callbacks, protocol handlers (local and global).

Was that resolved? Seems like this PR is still adding a new "completion provider" concept.

[justinmk]

we decided that opt-in behavior was better for now.

But does that require exposing all these different functions, which also implies this "attach a completion provider" concept?

My suggestion for opt-in was:

Alternatively, for opt-in behavior, we could document how to define a CompleteDone handler that calls a function we provide.

are there problems with this approach? I would expect this to only require exposing 1 function. And no attach/detach lifecycle.

[clason]

I won't comment on the specific API, but don't forget that the goal here is for plugins to be able to hook into that -- both completion and snippets.

Also, we won't speedrun this into 0.10; this is getting the pieces into place for a concerted "LSP autocompletion with snippets" push early in the 0.11 cycle so we can iterate on the full picture.

[clason]

(and, yes, if we can trigger the side effects robustly through a CompleteDone autocommand, that would be the preferred way of course; contingent on #20671 and a good story how plugins can remove or override such default handlers)

[mfussenegger]

It's currently InsertCharPre, InsertLeave, TextChangedP, TextChangedI and CompleteDone.
(Although TextChangedP / TextChangedI might disappear, see comment below)

And there are two independent aspects here: auto-trigger, and snippet expansion/side-effects.

Snippet expansion should also be useable with the omnifunc triggered manually, and with having opted out of auto-trigger. So we'd need a way to control these two aspects individually.

Exposing the group name or autocmd ids feels a bit like exposing the implementation details, and might limit how we can extend the implementation later.

I'd suggest to go with the enable / is_enabled.
That also gives us the option to add configuration options to the enable call.

Or maybe I completely misunderstand the proposal here?

[clason]

(and, yes, if we can trigger the side effects robustly through a CompleteDone autocommand, that would be the preferred way of course; contingent on #20671 and a good story how plugins can remove or override such default handlers)

Which we now can \o/ @MariaSolOs rebase on master?

[echasnovski]

This function is mostly called in "scheduled" fashion (via vim.shedule() or after vim.schedule_wrap()). Usually it should be possible to do single schedule_wrap() after defining function instead of all other scheduling in place.

[MariaSolOs]

Okay the implementation is done and tests are all passing. At this point we just need to agree on whether we should enable completion by default and the structure of the public API (so just the usual bikeshedding ;)).

Not sure if we want to do that in this PR or in a separate one. Wdyt @mfussenegger @clason @justinmk @gpanders?

[justinmk]

not a blocker: did you intend to revert your supports_method changes in lsp.txt ?

[MariaSolOs]

not a blocker: did you intend to revert your supports_method changes in lsp.txt ?

Yeah I didn't want to keep the changes in that file with stuff unrelated to this PR. I plan on doing a better review of the current LSP docs later.

[tomtomjhj]

Relying on InsertLeave can be wrong, because the user can exit the insert mode with ctrl-c, which doesn't trigger InsertLeave. Example: #29581. I noticed some other runtime files use InsertLeave, and they seem to be mostly fine. But I'm not quite sure if this module's use is correct.

[6Seeks]

Rely on TextChangedI will be a better idea