feat: add phantom completion

[timfjord]

• [x] hide phantoms on_selection_modified_async
• [x] handle multi-line display text
• [x] make the completion method configurable
• [x] clean up the phantoms dict when a view is closed
• [x] try line-height: 0 for the empty block phantom we use to keep the cursor from the left

TODOs:

• [ ] ~update the currently displayed phantom(instead of making a new request) when typing characters that match the beginning of the currently displayed phantom (this should work even when auto_ask_completions is set to false)~ https://github.com/TheSecEng/LSP-copilot/issues/52
• [ ] ~split the display text into regions and display with multiple phantoms~ https://github.com/TheSecEng/LSP-copilot/issues/53
• [ ] ~fix corner cases when the cursor jumps when the completion at the end of the line~ https://github.com/TheSecEng/LSP-copilot/issues/54
• [ ] ~do not ask for completion when saving a view with lsp_format_on_save~ https://github.com/TheSecEng/LSP-copilot/issues/55
• [ ] ~[optional] override the background colour for the block phantom and make it the same as the background colour of the editor (try this workaround to make it full width), this way it will be different from the line_highlight colour~ https://github.com/TheSecEng/LSP-copilot/issues/56

[jfcherng]

@TheSecEng has something similar or superior iirc

[TerminalFi]

@timfjord this is what I was working on in a local branch.

Is this something that is worth pursuing?

[timfjord]

Is this something that is worth pursuing?

Absolutely @TheSecEng

I just started on the implementation and there are a lot of things to do to make the integration really nice.
So if you have something please share it, so we could then combine some ideas

[timfjord]

I've updated the integration a bit, so it now handles multi-line strings and properly ident everything with   symbols.

The cursor still jumps in some cases and for now, I am not sure how to fix it(my workaround with an empty block doesn't work in all cases) but in general, it is now a way more reliable

[TerminalFi]

I've updated the integration a bit, so it now handles multi-line strings and properly ident everything with   symbols.

The cursor still jumps in some cases and for now, I am not sure how to fix it(my workaround with an empty block doesn't work in all cases) but in general, it is now a way more reliable

I wonder if we just reset cursor position after drawing the phantom would work.

[TerminalFi]

Also, I am assuming we plan to put this behind a setting? So the user can select popup vs phantom?

I see the task

[timfjord]

I wonder if we just reset cursor position after drawing the phantom would work.

I haven't tried this, but the problem with phantom cursor jumping is that the position doesn't really change.
I think it is basically how Sublime handles phantom rendering.
I am going to play with it a bit more, but most likely we need to ask someone from the Sublime team (I might try in the discord chat if I won't be able to find a solution).

So as far as I understand, you are also experiencing these problems with the cursor in your implementation, right?

[TerminalFi]

I wonder if we just reset cursor position after drawing the phantom would work.

I haven't tried this, but the problem with phantom cursor jumping is that the position doesn't really change. I think it is basically how Sublime handles phantom rendering. I am going to play with it a bit more, but most likely we need to ask someone from the Sublime team (I might try in the discord chat if I won't be able to find a solution).

So as far as I understand, you are also experiencing these problems with the cursor in your implementation, right?

Yup, and there is no rhyme or reason to when it happens.

I experienced this and an old plugin as well.

[TerminalFi]

For settings, we should really access session.config.settings over view.settings() as the session holds the most true settings for each LSP session

Atleast when accessing copilot settings

[TerminalFi]

@timfjord Perhaps this is what we experience https://github.com/sublimehq/sublime_text/issues/2871

[timfjord]

I think that is exactly it, let me add my comment there.
A label was added on May'31, so at least it was acknowledged by the Sublime team

[TerminalFi]

There is also https://github.com/sublimehq/sublime_text/issues/4575 which might be relevant

[timfjord]

It was tagged on the same day as the other one, so maybe there is chance it will be implemented

[timfjord]

@jfcherng thanks to your latest debounce improvements the cursor doesn't jump anymore 👍🏻
I think the main issue with the cursor was that we sent too many requests(because of the old debouncer implementation) and the phantoms were shown and hidden multiple times that was causing the cursor to jump.

There might be still problems with cursor jumping but at least it is way more stable now

[jfcherng]

Personally, I have "line_padding_bottom": 2, in my preferences.

There is a similar issue to https://github.com/facelessuser/ColorHelper/pull/122.

https://user-images.githubusercontent.com/6594915/182443500-8319948d-228b-4715-8f77-f0fb2a7d1a3c.mp4

[timfjord]

Personally, I have "line_padding_bottom": 2, in my preferences.

There is a similar issue to facelessuser/ColorHelper#122.

dasf.mp4

We add an empty block phantom(even for single line completion) to stabilize the cursor and it causes the current line to be a bit bigger.
I haven't tried line-height: 0, but I will do

[timfjord]

@jfcherng @TheSecEng all the edge cases for the phantom completion are taking longer than planned so I would like to propose the following.

I've addressed the most critical TODOs so to avoid maintaining a long-running branch I would send this one to review. As for the remaining TODOs, I would create separate issues and tackle them separately. What do you think?

I've also done minor refactoring as part of this PR. Basically, I've made the Completion classes more independent and removed ViewCompletionManager from its dependencies.

[timfjord]

Ok, I think it is ready to go.
Once we merge I will convert all pending TODOs to issues

[timfjord]

Hey @TheSecEng Have you got a chance to look at this PR?