hrsh7th / nvim-cmp

A completion plugin for neovim coded in Lua.
MIT License
8.08k stars 400 forks source link

cmp.ConfirmBehavior.MatchSuffix? #1716

Open adaszko opened 1 year ago

adaszko commented 1 year ago

Hi and thank you for this fantastic plugin!

I'm looking for a middle ground between cmp.ConfirmBehavior.Replace and cmp.ConfirmBehavior.Insert. Namely, a one that would complete foo_<TAB>_baz to foo_bar_baz right away given possible completions foo_bar_baz and foo_bar_quux. It would ignore foo_bar_quux automatically by noticing it doesn't match the existing _baz suffix. This is just like zsh's complete_in_word option.

I currently use cmp.ConfirmBehavior.Insert but have to constantly re-edit the suffix after inserting something in the middle of a word.

I don't think it's currently possible with nvim-cmp so this would be a feature request. Curious what you think.

Avi-D-coder commented 6 months ago

I really want this, has anyone implemented it before I spend time on it.

wincent commented 4 months ago

I don't think it's currently possible with nvim-cmp

I was just looking into this myself as I've been annoyed by this behavior for years (not just with nvim-cmp, but with literally every autocompletion system I've ever used), and I wonder if you might be able to hack something by tapping into the confirm_done event. It would be a huge hack, but you could basically delete the redundantly inserted suffix after insertion by detecting the duplicate. My main concern with it would be that it would be clunky and error prone, so a new ConfirmBehavior would probably be way more robust.

Shougo commented 4 months ago

I don't think it's currently possible with nvim-cmp so this would be a feature request. Curious what you think.

Hi. Why you cannot use cmp.ConfirmBehavior.Replace? It seems works for me.

wincent commented 4 months ago

It seems works for me.

The difference is subtle, and I think it might be due to this code that defaults to replacing on keyword boundaries.

If you have this text in a buffer (cursor position shown with ^) and you hit <Tab>:

// Going to do autocompletion here...
FooBazzzz
   ^

// And elsewhere there's this completion candidate:
FooBarBaz

The example is contrived, of course, but hopefully it illustrates what the difference would be. Sometimes you really want to keep that zzz suffix around, because you're about to continue editing it, and removing it is a destructive operation that you can't undo without also undoing the autocompletion.

In the abstract, I think the difference between the behaviors can be described as:

Does that make sense?

Shougo commented 4 months ago

Hm.... If so, MatchSuffix behavior is to edit the suffix after insertion?

And what is the cursor behavior after insertion when MatchSuffix?

adaszko commented 4 months ago

Hm.... If so, MatchSuffix behavior is to edit the suffix after insertion?

And what is the cursor behavior after insertion when MatchSuffix?

Take a look at how zsh does it (ignore the trailing /):

match-suffix

The cursor lands at the end of the matched suffix which I think is a good choice.

Shougo commented 3 months ago

Thanks. I get it. If so, this is feature request.

wincent commented 3 months ago

Reading this thread has made me realize that there are actually two separate requests going on here.

  1. The initial request is about having nvim-cmp recognize that the suffix helps disambiguate possible completions and having it select the resulting completion immediately without forcing the user to choose. That's what's shown in the Zsh Gif above.
  2. The second request that I brought up here is about nvim-cmp replacing only the matching part of the suffix as opposed to replacing the entire 'keyword'.

They're related because they're both about what nvim-cmp should do when it does "in-word" completion, but they are actually separate requests.

I think I might have a couple of examples that do a better job at illustrating what I'm asking for than my previous attempt. These are two scenarios that I ran into today and which show why Replace doesn't quite do the right thing, but neither does Insert, motivating the desire to have a MatchSuffix option.

Example A:

Example B:

[^with]: With something like <C-[ (to leave INSERT mode) and then dFP.

I run into examples like this multiple times per day, and in my (subjective) experience:

In practice, having to delete an unwanted suffix is a bit annoying, but it's not as painful as having to restore text which was deleted undesirably, so MatchSuffix would avoid the pain that sometimes comes with Replace, but it would frequently avoid the annoyance that can come with Insert mode.

Sorry about the wall of text, but I couldn't find a more concise way to describe it! 🙇

Shougo commented 3 months ago

The initial request is about having nvim-cmp recognize that the suffix helps disambiguate possible completions and having it select the resulting completion immediately without forcing the user to choose. That's what's https://github.com/hrsh7th/nvim-cmp/issues/1716#issuecomment-2237082589.

Hm... It seems suffix matcher instead of confirming.

And it does not work for Example A.

Shougo commented 3 months ago

In this instance Insert behavior would have done what I wanted, and MatchSuffix would have also; the line would contain "(KubernetesServices → Clusters → Data centers" and I'd just have to type the trailing ") ".

Well, it works well on Insert, but it does not work well on MatchSuffix.

Because the cursor position is:

(KubenetesServices| <- here.

It is not (Kubernetes|Services.

You need to modify the code in normal mode.

Shougo commented 3 months ago

I think the cursor behavior is complex.

The cursor lands at the end of the matched suffix which I think is a good choice.

If so,

With the hypothetical MatchSuffix behavior, FooBazzzz would become FooBarBazzzz

The cursor position is:

FooBarBaz|zzz

It is true?

Please describe it for me.

adaszko commented 3 months ago

With the hypothetical MatchSuffix behavior, FooBazzzz would become FooBarBazzzz

The cursor position is:

FooBarBaz|zzz

It is true?

Good question. @wincent and I seem to be proposing two conflicting specifications here.

  1. @wincent's: Foo|<TAB>Bazzzz -> FooBarBaz|zzz, assuming available completion FooBarBaz.

  2. Me: Foo|<TAB>Bazzzz -> NO CHANGE! Why? Because completions are only offered if the suffix exhausts the completed keyword fully, so it would only complete if we were completing Foo|<TAB>Baz, not Foo|<TAB>Bazzzz.

So what I'm proposing is more restrictive but if you chose a completion at some point, you can be sure the keyword isn't misspelled. It also feels less surprising. What's important, such behaviour still works in both Example A and B.

wincent commented 3 months ago

In this instance Insert behavior would have done what I wanted, and MatchSuffix would have also; the line would contain "(KubernetesServices → Clusters → Data centers" and I'd just have to type the trailing ") ".

Well, it works well on Insert, but it does not work well on MatchSuffix.

Because the cursor position is:

(KubenetesServices| <- here.

It is not (Kubernetes|Services.

You need to modify the code in normal mode.

I would actually think that it would be (Kubernetes|Services; given that it didn't complete Services, it shouldn't jump to the end of that.

wincent commented 3 months ago

The cursor position is:

FooBarBaz|zzz

It is true?

That is what I'd intuitively expect.

Here's one more thing to consider: the interaction of the different behaviors with the experimental "ghost text" feature. At the moment, I believe the ghost text always shows what would be inserted with Insert behavior; it doesn't show only the part of the completion that would appear in the buffer if a Replace behavior completion was accepted, not does it show what would be inserted if we added this hypothetical MatchSuffix behavior. I don't think that's a deal breaker, but in an ideal world, ghost text would show as close as possible to what's going to be added to the buffer if you accept a completion.

wincent commented 3 months ago

So what I'm proposing is more restrictive but if you chose a completion at some point, you can be sure the keyword isn't misspelled. It also feels less surprising. What's important, such behaviour still works in both Example A and B.

Thanks for bringing this up; I hadn't noticed this detail about your proposal (and I'd have to think a while before forming an opinion about which variant is better).

Shougo commented 3 months ago

So what I'm proposing is more restrictive but if you chose a completion at some point, you can be sure the keyword isn't misspelled. It also feels less surprising. What's important, such behaviour still works in both Example A and B.

OK. So it work for Example B, but not A. Because Kubernetes not matched to Services.

I would actually think that it would be (Kubernetes|Services; given that it didn't complete Services, it shouldn't jump to the end of that.

It jumps to the end when it is matched only the suffix is matched? It is OK.

The example is contrived, of course, but hopefully it illustrates what the difference would be. Sometimes you really want to keep that zzz suffix around, because you're about to continue editing it, and removing it is a destructive operation that you can't undo without also undoing the autocompletion.

But this is wrong?

@adaszko : Complete candidate only matched to suffix @wincent : Insert candidate if not matched to suffix, Replace the candidate if it is matched to suffix

Hm... They are conflicted desire.

adaszko commented 3 months ago

So what I'm proposing is more restrictive but if you chose a completion at some point, you can be sure the keyword isn't misspelled. It also feels less surprising. What's important, such behaviour still works in both Example A and B.

OK. So it work for Example B, but not A. Because Kubernetes not matched to Services.

That's true. My mistake. In Example A (Kub<Tab>Services) you would have to manually insert a space before Services so that Kub<Tab> Services gets completed to Kubernetes Services.

adaszko commented 3 months ago

Here's a thought: To make Example A work, ConfirmBehavior.MatchSuffix could first try to perform a restrictive match on the suffix as I described above, and if it doesn't match any candidates, perform ConfirmBehavior.Insert as a fallback.

Shougo commented 3 months ago

Yes. It is @wincent 's idea.

adaszko commented 3 months ago

Great, so we have an agreement on the specification then?

Shougo commented 3 months ago

Here's a thought: To make Example A work, ConfirmBehavior.MatchSuffix could first try to perform a restrictive match on the suffix as I described above, and if it doesn't match any candidates, perform ConfirmBehavior.Insert as a fallback.

If the behavior is OK, Yes.

adaszko commented 3 months ago

Here's a thought: To make Example A work, ConfirmBehavior.MatchSuffix could first try to perform a restrictive match on the suffix as I described above, and if it doesn't match any candidates, perform ConfirmBehavior.Insert as a fallback.

If the behavior is OK, Yes.

Yes, such behaviour totally works for me.

FWIW, I realised zsh can be configured to behave like that too:

setopt complete_in_word
zstyle ':completion:*' completer _complete _prefix