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

Add support for inline completions (WIP) #2552

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jwortmann
Copy link
Member

Experimental / work in progress

Implementation details are open for discussion and it would be nice to gather some opinions how inline completions should work.

  • For now in this PR they are only requested when you manually trigger autocomplete. And then they are immediately hidden when you either type something or move the caret. It would be nice to show the inline completions while typing, but I think the phantom behavior is too buggy for that (Make phantoms work better with inline(ghost) text sublimehq/sublime_text#5536). Another problem is that the regular completions popup often hides a big part of the inline completion phantom. But that is independent of whether it would be on typing or on manual trigger.
  • The server can return multiple inline completion items, but currently only the first one is shown and the other ones are ignored. Maybe there should be some way to switch between items, like in the signature help popup.
  • Maybe add a setting to configure the display mode; full or only single line (see Sublime-LSP Local Code-Assistant Support #2520 (comment))?
  • We can't provide SelectedCompletionInfo because it is not available from ST.

It can be tested with Tabby; for setup see #2520 or the docs description which is added in this PR. Tabby requires a compatible GPU.

For this PR I hijacked the Alt + Enter key binding which is currently used for "insert completion with opposite insert mode", because there aren't really any good key bindings left (everything with Ctrl or Shift is already taken in the ST default key bindings). This was mostly for testing and is up for discussion I guess.

I haven't really looked at the implementations in e.g. LSP-copilot yet. Perhaps there are some improvements that we can adopt.

Copy link

netlify bot commented Nov 11, 2024

Deploy Preview for sublime-lsp ready!

Name Link
🔨 Latest commit 3063b62
🔍 Latest deploy log https://app.netlify.com/sites/sublime-lsp/deploys/6738d0f11449380008461ae0
😎 Deploy Preview https://deploy-preview-2552--sublime-lsp.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

session = self.best_session(self.capability, point)
if not session:
return
if self.view.settings().get('mini_auto_complete', False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hahahaha I was wondering why this PR doesn't work for me, it was due to this check.
I had mini_auto_complete enabled.

// {"key": "auto_complete_visible"}
// ]
// },
// Insert Inline Completion
{
"keys": ["alt+enter"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of using tab for committing an inline completion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware that Tab is used for it in VSCode and I think also in other IDEs like IntelliJ. But I doubt that it would be a good choice in Sublime; personally I use Tab to commit regular auto-complete ("auto_complete_commit_on_tab": true) and there are probably many users who do the same. I think we should use a key binding which is definitely not already in use by Sublime Text. However Alt + Enter is not the most ergonomic to use though; here I used it mostly for testing but we can discuss whether we find something better. Or maybe it should be left without any key binding by default and require users to set up an own key binding for it.

@predragnikolic
Copy link
Member

We can't provide SelectedCompletionInfo because it is not available from ST.

This is true.

listener.inline_completion.position = position
listener.inline_completion.render_async(0)

# filter_text = item.get('filterText', insert_text) # ignored for now
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

return False
return listener.inline_completion.visible

def run(self, edit: sublime.Edit, event: dict | None = None, point: int | None = None) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

point is unused

"LspNextDiagnosticCommand",
"LspNextInlineCompletionCommand",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how to test the LspNextInlineCompletionCommand

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently I trigger the autocomple,
that also triggers the inline completions request,
and I will see the inline completion phantom,
When I trigger the LspNextInlineCompletionCommand, I expect to see a different inline completion phantom, but I still see the same phantom. Do you see a different phantom when triggering the LspNextInlineCompletionCommand?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experiments, Tabby always provides only a single inline completion item. So it's expected that currently nothing happens when you trigger that command.

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

Successfully merging this pull request may close these issues.

2 participants