TerminalFi / LSP-copilot

GitHub Copilot support for Sublime Text LSP plugin provided through Copilot.vim.
625 stars 25 forks source link

Copilot completes on tab despite rebinding copilot_accept_completion #73

Closed davidaurelio closed 3 months ago

davidaurelio commented 1 year ago

Thanks for working on this package, and bringing copilot to Sublime Text!

In my Default.sublime-keymap I have bound copilot_accept_completion like this:

  {
    "keys": ["primary+enter"],
    "command": "copilot_accept_completion",
    "context": [
      {
        "key": "setting.copilot.completion.is_visible"
      }
    ]
  },

Despite that, the default key binding using "tab" still exists and takes precedence over other LSP autocompletion.

How do I disable that? Sublime Text now has 4 other bindings for tab, do I have to copy all of them into my key bindings (b/c that seems to work)? That’s inconvenient and might break functionality introduced in the future.

jfcherng commented 1 year ago

Despite that, the default key binding using "tab" still exists and takes precedence over other LSP autocompletion.

Oops. Do you mean that if you press "tab", the Copilot suggestion will be commited? If yes, I think that's the original goal: to mimic how it works in VSCode. But that doesn't work on my side so I add that Pressing Tab commits autocompletion rather than Copilot's suggestion FAQ.


So eventually, different people have different expectation for what should happen when pressing tab. Commit autocompletion or commit Copilot suggestion?

timfjord commented 1 year ago

Theoretically, we could add a setting to control that(and make it true by default).

So by default we will mimic VSCode, but leave an option to disable this functionality for the cases like this one

davidaurelio commented 1 year ago

In general I think the initial setup is good, because it works out of the box.

However, I really like using other LSP autocompletions 70% of the time, and copilot 30% of the time—they are each good at different things.

@jfcherng Yeah, I thought I’d be able to rebind completely. That’s a misunderstanding on my part.

@timfjord your suggestion of having a way to disable the default behaviour sounds good.

For now, I copied all completions bound to "tab" from the defaults to my key bindings, but I feel that’s not maintainable long term.

jfcherng commented 1 year ago

Theoretically, we could add a setting to control that(and make it true by default).

I don't know how we can implement this. Can't figure out a simple way.

And thus I prefer to unbound https://github.com/TerminalFi/LSP-copilot/blob/f41608198f00ccaf69c60ed6d6c05969476e78f5/Default.sublime-keymap#L2-L12. If a user wants it, then he binds that to whichever key prefered.

davidaurelio commented 1 year ago

That’s an understandable perspective. I haven’t fooled around with ST extensions in a while. Would it be possible to create a context value that can be influenced by a setting? That could allow to create a configurable key binding.

jfcherng commented 1 year ago

Would it be possible to create a context value that can be influenced by a setting? That could allow to create a configurable key binding.

Ah, yes. This is it!

timfjord commented 1 year ago

Cannot we simply use setting.copilot.<my_setting>?
I thought it should work Otherwise, yeah, we would need a custom context

jfcherng commented 1 year ago

setting means view settings iirc. I.e., setting.copilot.<my_setting> is view.settings().get('copilot.<my_setting>').

timfjord commented 1 year ago

You can access project settings and general settings(Preferences.sublime-settings) use the setting. notation, but, unfortunately, you cannot access settings defined in a different file(e.g. LSP-copilot.sublime-settings).

So we could either do a custom context or use Preferences.sublime-settings to control that and have something like

{ 
     "keys": [ 
         "tab" 
     ], 
     "command": "copilot_accept_completion", 
     "context": [ 
         {
            "key": "setting.copilot_disable_default_keybindings", 
            "operand": false
         },
         { 
             "key": "copilot.is_on_completion" 
         }
     ] 
 }
timfjord commented 1 year ago

If we really want to unbound, we should add a note to the README, because after the upgrade, the default keybinding won't work anymore that might confuse people

jfcherng commented 1 year ago

You can access project settings and general settings(Preferences.sublime-settings) use the setting. notation, but, unfortunately, you cannot access settings defined in a different file(e.g. LSP-copilot.sublime-settings).

Yes, because those are merged into view settings but a plugin's settings won't be. Can be verified by view.settings().to_dict() in the console.


copilot_disable_default_keybindings is ambiguous imo since obviously we don't disable all default keybindings. Maybe use another name?

jfcherng commented 1 year ago

https://github.com/TerminalFi/LSP-copilot/blob/e61efe877d50dfe13f970e18f68926232f9e3011/Default.sublime-keymap#L13-L23 suffers from the same issue (cancel copilot suggestion v.s. cancel autocompletion) too but people may less care about it.

timfjord commented 1 year ago

I would add copilot_disable_default_keybindings to all default bindings.

Alternatively, we could have copilot_disable_vscode_keybindings and use it only for esc and tab

jfcherng commented 1 year ago

@TerminalFi what do you think?

TerminalFi commented 1 year ago

I agree with the former. I don't think there is a need to specify VSCode settings.

It is a bummer that this would be considered a breaking change, but I also understand that it may be a UX improvement. This makes me feel like we need to start adding release notes

KyleKing commented 1 year ago

I want to bump this issue, since I have also had trouble with copilot providing incorrect suggestions and overriding the correct one that I attempted to select from autocompletion on tab. For now, I've turned off "auto_ask_completions"

jfcherng commented 1 year ago

I want to bump this issue, since I have also had trouble with copilot providing incorrect suggestions and overriding the correct one that I attempted to select from autocompletion on tab. For now, I've turned off "auto_ask_completions"

Do you mean you actually want ST's autocompletion but Copilot's suddenly pops up so your tab committed Copilot's?

KyleKing commented 1 year ago

Yeah, that's correct

TerminalFi commented 1 year ago

I've also experienced completion on Copilot even when Copilot isn't displaying a completion. This may be a ST issue. Need to further investigate

Mr-HaleYa commented 6 months ago

Has any progress on this been made?

Do you mean you actually want ST's autocompletion but Copilot's suddenly pops up so your tab committed Copilot's?

This happens a lot and being able to debind/rebind the key would be very nice.

Unrelated but could also be helpful, I've seen others talk about a keybind that could toggle auto_ask_completions. Being able to switch off the suggestions when not wanted and then turn them back on could reduce the need to rebind the tab behavior

demeralde commented 4 months ago

Is there going to be a fix for this soon?

jfcherng commented 4 months ago

I would say just use a different key binding.