emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.81k stars 890 forks source link

Support for inlineCompletionProvider #4581

Open Konubinix opened 1 month ago

Konubinix commented 1 month ago

Is your feature related or already mentioned on the wishlist?

Describe your feature here.

When getting completion, I try to use only the built-in capfs mechanism. The fact that lsp-mode integrates with it is great. Unfortunately, copilot.el does not and I have capfs and the ad-hoc copilot.el stuff so far.

I just realized that under the hood, copilot.el uses an lsp server : copilot-node-server, so I tried creating the lsp-copilot.el client.

But this server provides completion using the inlineCompletionProvider capability, not the completionProvider one.

I tried using https://github.com/kassick/lsp-inline-completions there are a few glitches, but it basically works. But it still does not use the capfs. So I'm back to square one.

So, I'm wondering if we could deal with completion using inlineCompletionProvider in lsp-mode, so that simply M-TAB would work seamlessly.

Is there already some discussion on the topic?

kiennq commented 2 weeks ago

inlineCompletions is official in 3.18 lsp spec, we should support it if possible. I've also created an issue in this kassick/lsp-inline-completions#1 to see if it's okay to integrate the lsp-inline-completions to the current lsp-mode. @yyoncho @jcs090218

kassick commented 2 weeks ago

so I tried creating the lsp-copilot.el

@Konubinix I did the same here https://github.com/kassick/copilot-lsp ! Let me know if you want to use some of it or contribute -- it was created as a POC at the time, and then I switched to copilot.el because I did not have much time back then ...

kassick commented 2 weeks ago

@kiennq as I mentioned in the lsp-inline-completions repo, if the maintainers think it makes sense to upstream the code, sure!

There is at least one glitch I'm aware of -- not sure if it is the same one @Konubinix ran into

kassick commented 2 weeks ago

Regarding CAPF for inlineCompletions:

One aspect of inline completions is that they can span multiple lines.

With copilot, I did not see much of an issue -- I always get one-liners, not sure if it ever responds with longer text. On gitlab-lsp, though, some prompts resulted in multi-line code snippets (e.g. # write a unit test for this file would bring a test function) , and some would even result in changes in other points of the text.

That's why I wrote a separate UI for inline completions instead of using company, even taking care to display the overlay in the correct place.

When I had the completions from gitlab-lsp displayed in company, it would often look rather weird -- a very long company tooltip with a single candidate displayed on top of the preview overlay.

Helm or Vertico as frontends for capf were no better.

IMHO inlineCompletions either requires some tweaking in company (or any other CAPF frontend) or a separate UI alltogether.

kiennq commented 2 weeks ago

I think the inlineCompletions should be independent from capf and closer to the inlay hint. Just that the inlay hint is just displaying without allow to select the next/prev suggestion and commit it. The UI is fairly close.

kassick commented 4 days ago

I've update the code in my repo and soon I'll open a PR to lsp-mode

@Konubinix If you can recall what were the issues you ran into, I could work on them before

Konubinix commented 3 days ago

Rodrigo Kassick @.***> writes:

I've update the code in my repo and soon I'll open a PR to lsp-mode

@Konubinix If you can recall what were the issues you ran into, I could work on them before

I will try again your repo and provide a feedback by friday. Is that ok ?

-- Konubinix GPG Key : 7439106A Fingerprint: 5993 BE7A DA65 E2D9 06CE 5C36 75D2 3CED 7439 106A

kassick commented 3 days ago

I will try again your repo

@Konubinix Actually, could you try lsp-mode from this fork ?

I've extracted a lsp-inline-completion-get-items function that handles making the request and parsing the response. If you prefer to have these completions as capf, you can write a wrapper to add to completion-at-point-functions.

Otherwise, using it is simply calling the display function.

  (define-key lsp-mode-map
              (kbd "C-*") '("Inline Completions" . lsp-inline-completion-display))

... provide a feedback by friday. Is that ok ?

Sure is!

I'll also be testing it here in my setup; let's see if its behaving better now

kassick commented 3 days ago

@kiennq I've included the overlay UI in this fork, but I wonder if lsp-mode should contain only the lsp-inline-completion-get-items and open a separate PR to lsp-ui with the user interface.

What do you think?

kiennq commented 2 days ago

@kiennq I've included the overlay UI in this fork, but I wonder if lsp-mode should contain only the lsp-inline-completion-get-items and open a separate PR to lsp-ui with the user interface.

What do you think?

The overlay UI (default) should be in lsp-mode. More advanced UI can be put to lsp-ui. You can take a look at lsp-inlay-hints-mode which is also using overlay and stay inside lsp-mode.

Basically, we will want to have lsp-inline-completion-mode as well and let the user toggle on the experience as they see fit. The default can be t for ease of feature discoverability though.

On another note, is the entry point for inline completion mode lsp-inline-completion-display? I think we can put that to idle hook as well so it can be fetch automatically when idle.

Konubinix commented 2 days ago

@kassick : I wanted to try your code, but I cannot manage anymore to create a connection with copilot-node-server that provides inline completion. When I look at the exposed capabilities (using lsp-describe), I can only see this

image

I created a simple lsp-client that runs the code the same way than copilot.el does

(lsp-register-client
 (make-lsp-client :new-connection (lsp-stdio-connection '("node" "/home/sam/.emacs.d/.cache/copilot/lib/node_modules/copilot-node-server/copilot/dist/agent.js" "--stdio"))
                  :activation-fn (lambda (_ _) t)
                  :server-id 'copilot))

IIRC, at the time I wrote this issue, I could see inlineCompletion in the registered capabilities. I suppose it is linked to a recent update of the server. Therefore, I am unable to try your code. Sorry.

kassick commented 1 day ago

The overlay UI (default) should be in lsp-mode.

Ack

More advanced UI can be put to lsp-ui. You can take a look at lsp-inlay-hints-mode which is also using overlay and stay inside lsp-mode.

Will do, but overall the overlay in my fork is behaving as expected.

Basically, we will want to have lsp-inline-completion-mode as well and let the user toggle on the experience as they see fit. The default can be t for ease of feature discoverability though.

On another note, is the entry point for inline completion mode lsp-inline-completion-display? I think we can put that to idle hook as well so it can be fetch automatically when idle.

Having the function be called on idle looks like an expected behaviour for an active lsp-inline-completion-mode , I'll add this minor mode as suggested [^1].

[^1]: My personal preference is having completions shown only upon request, so I did not consider an idle timer at first. Good thing that both explicit and implicit calling can co-exist ;)

kassick commented 1 day ago

@Konubinix That's weird -- I'm on version 1.41.0 of the server (the latest version, as of this reply) and I can see the expected capabilities image

I think I've seen something similar when the server was partially initialized (network issues; expired token, etc).

If you manage to work around this server issue, let me know if the implementation in the fork is working as expected for you!