Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for inlineCompletionProvider #4581

Open
Konubinix opened this issue Oct 8, 2024 · 14 comments
Open

Support for inlineCompletionProvider #4581

Konubinix opened this issue Oct 8, 2024 · 14 comments

Comments

@Konubinix
Copy link
Contributor

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
Copy link
Member

kiennq commented Nov 6, 2024

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
Copy link
Contributor

kassick commented Nov 6, 2024

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
Copy link
Contributor

kassick commented Nov 6, 2024

@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
Copy link
Contributor

kassick commented Nov 7, 2024

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
Copy link
Member

kiennq commented Nov 8, 2024

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
Copy link
Contributor

kassick commented Nov 18, 2024

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
Copy link
Contributor Author

Konubinix commented Nov 19, 2024 via email

@kassick
Copy link
Contributor

kassick commented Nov 19, 2024

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
Copy link
Contributor

kassick commented Nov 19, 2024

@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
Copy link
Member

kiennq commented Nov 20, 2024

@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
Copy link
Contributor Author

@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
Copy link
Contributor

kassick commented Nov 21, 2024

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.

Footnotes

  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
Copy link
Contributor

kassick commented Nov 21, 2024

@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!

@kassick
Copy link
Contributor

kassick commented Nov 27, 2024

@kiennq the PR includes a minor mode (on my default) that displays completions on idle.

I also tried to make company and the online completion overlay to play nice with each other, specially with the minor mode on. It seems to be working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants