lifepillar / vim-mucomplete

Chained completion that works the way you want!
MIT License
913 stars 18 forks source link

Adding the value 'noselect' to the option 'completeopt' breaks undo sequence #40

Closed lacygoill closed 7 years ago

lacygoill commented 7 years ago

Hello,

If I write the following text inside /tmp/vimrc.vim:

set cot=noselect,menu,menuone
set rtp+=~/.vim/plugged/vim-mucomplete

" people_A
" people_B
" people_C

" hello world
" hello world
" hello world

And then if I run Vim from the shell like this:

vim -Nu /tmp/vimrc.vim /tmp/vimrc.vim

I put the cursor on the word world and hit * to populate the search register. I hit cgn to change the next occurrence of the search register (here world). I insert pe and hit Tab to complete. The plugin inserts people_A, and the menu suggests people_A, people_B and people_C. It doesn't select any entry, respecting the value noselect.

If I then hit escape to go back to normal mode, then the dot command to repeat the last change, the next occurrence of world is replaced with pe instead of people_A like I expected.

The problem comes from the function s:act_on_pumvisible(), and more specifically from this line:

 \ : get(s:select_entry, s:compl_methods[s:i], "\<c-n>\<up>")

Its purpose is to hit C-n Up after the pop-up menu has been opened, if 'completeopt' contains the value 'noselect', in order to force the insertion of the first or last entry from the menu.

I think the keys <Up> and <Down> are special, insofar as they break the undo sequence, as described in :h ins-special-special. Breaking the undo sequence means that the dot register discards the text that was inserted so far, and records only from the current point. It can be checked in the current case by looking at the output of :reg..

Usually, when you want the dot command to repeat the last edit entirely, and it contains a special key, you prefix the latter with C-g U which preserves the undo sequence (see :h i_^gU). But here it doesn't seem to work. Hitting C-g makes us leave the pop-up menu.

I must admit I don't really understand the bug, for 2 different reasons. First, if <Up> or <Down> break the undo sequence, then, in the last example, the dot command shouldn't repeat pe but ople_A. Second, why do they break the undo sequence, when 'completeopt' contains the value noselect and s:act_on_pumvisible() hit C-n Up, but they don't seem to break it when 'completeopt' does NOT contain noselect but DOES contain noinsert and s:act_on_pumvisible() hit Up C-n?

To be clear, this isn't a bug in the plugin. It seems to be a standard behavior from Vim, but I don't understand it. And it causes the plugin to break the undo sequence when 'completeopt' contains noselect. I don't know if it deserves a fix, and if it does, I have no idea how to write one.

You could replace C-n Up and C-p Down with simply C-n and C-p. It would preserve the undo sequence, but it wouldn't respect the user setting set completeopt+=noselect, because then an entry from the menu would be selected, regardless of the option's value.

lacygoill commented 7 years ago

Here's a gif showing what happens when 'cot' contains noselect:

http://imgur.com/a/5rhFZ

And here's what happens when 'cot' doesn't contain noselect, but does contain noinsert:

http://imgur.com/a/cfIi5

In the first case, the repetition with the dot command is only partial. In the second case, it's complete.

lifepillar commented 7 years ago

I think that the problem is that, after pressing a cursor key, Vim in “state 2” (:h ins-completion-menu), that is, it considers the match as not inserted. I don't know whether the fact that the text is there, and it stays there after pressing <esc> is by design.

It might make sense to ignore noselect in manual completion, which would solve this problem.

lacygoill commented 7 years ago

I agree with you (ignoring noselect), that's what I do anyway, I don't add noselect because of this. noinsert is enough. Besides, it would make the code inside s:act_on_pumvisible() simpler.