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

change: register inline completion provider for files and notebooks only #1114

Merged
merged 2 commits into from
Sep 20, 2023

Conversation

abeatrix
Copy link
Contributor

Close #1112

The inline completion provider was previously registered for all languages ('*') at all places.

This change updates the registration to explicitly include notebook and text documents with file scheme to ensure autocompletion only works in places that we currently support.

Test plan

  • Completions should work the same on saved text documents and notebook cells
  • Completions should not work in source control input box or any other places that are not editor files / notebook files

Before

image

After

limited.to.file.scheme.and.notebook.mp4

@abeatrix abeatrix requested review from a team and philipp-spiess September 19, 2023 17:22
@abeatrix abeatrix merged commit c4489fb into main Sep 20, 2023
9 checks passed
@abeatrix abeatrix deleted the bee/inline-provider branch September 20, 2023 06:41
@philipp-spiess
Copy link
Contributor

Nice! But I think this regresses autocomplete on new unsaved files, did you try that?

@philipp-spiess
Copy link
Contributor

#1120

@abeatrix
Copy link
Contributor Author

@philipp-spiess I mentioned in the description that it would only work in saved text doc and notebook cells:

Completions should work the same on saved text documents and notebook cells

I thought the quality will not be good like in scm when working in an unsaved file because we won't be able to fetch relevant context for them / no language support so I didn't include that.

If that's something we want to continue support I can add the untitled scheme to the provider!

@abeatrix
Copy link
Contributor Author

Saw you already did lol thanks!

@philipp-spiess
Copy link
Contributor

Ohh gotcha... Yeah I added this on purpose because a user expected it since Copilot does it.

I guess what we can do is support it but only if a language id is specified (so we exclude plaintext which according to our analytics does also have a very bad acceptance rate)

@philipp-spiess
Copy link
Contributor

Forget that I just tested and Copilot does not do this (anymore?) so I think your intuition is correct

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.

Autocomplete: Disable on scminput
3 participants