microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.93k stars 29.18k forks source link

InlineCompletion Tab acceptance #206811

Open marrej opened 8 months ago

marrej commented 8 months ago

Problem statement

InlineCompletion can in most cases be accepted by Tab, but can't be accepted when there is Tab in between the cursor and the completion. This causes issues e.g. when interacting with multiline completions, which can be accepted by line.

  1. Show multiline completion
  2. Accept first line (the cursor lands on the start of the next line without tabbing applied)
  3. User wants to accept the full completion by pressing Tab
  4. Only Tab is added, and the completion can refresh

This model of interaction causes frustration for users, who expect that the acceptance would actually apply the completion. And creates unwanted space for errors.

To replicate try here (try accepting line and fully accepting completion from the line 8)

Possible solution

I understand that there might be some internal reasons to why this was introduced, so I wonder whether we can at least allow the specific completions (Completion Providers) to decide whether this should be applied or not. The same way as enableForwardStability is a inlineCompletion flag.

enableTabPrefixAcceptance would be a flag that would allow extensions to have a better control over the acceptace of their provided completions.

hediet commented 7 months ago

@isidorn what do you think?

isidorn commented 7 months ago

I like the points @marrej is making. And this feels like a bug to me. If we decide to tackle this, I would suggest we start without a flag. And just have the behaviour @marrej proposses for everyone, and in case we hear complaints, then we can look into adding a flag.

Thoughts?

hediet commented 7 months ago

Copilot explicitly asked for this behavior. I think it should be easy to override it, by configuring a custom keybinding:

https://github.com/microsoft/vscode/blob/a9d09f9fbbb43b0398478f13856b5198cfc5afaa/src/vs/editor/contrib/inlineCompletions/browser/commands.ts#L155

marrej commented 7 months ago

I'm not sure a custom keybinding is exactly the best way forward. That introduces unneccessary mental load for the users who would have to either explicitly change the keybinding or learn to work with InlineSuggest in a different manner. (not the muscle memory Tab)

That is why I originally proposed for Completions to actually controll this via additional data (like done with the enableForwardStability), so that Providers can choose to ignore this if neccessary, without introducing toil for the user.