Wrong completion entry selection after backspace

[Olivier Teuliere]

Hi,

There seems to be a small bug in the completion popup behavior.
Here is a simple way to reproduce it (I used vim 7.3-798 on Linux):

1) Start vim with "vim -u NONE -c "set nocp"

2) Enter the following text:
foobar
foobaz
foofoo1
foofoo2
foo

3) In insert mode, with the cursor at the end of the last word, type
<c-n><c-p> to open the completion menu and remove the inserted text

4) Type <Down> to highlight "foobar" again

4) Type 'f': the completion list shrinks to "foofoo1" and "foofoo2" only

5) Type <bs>: the list is restored, and "foofoo1" is still selected
(and typing <CR> now would insert it)

6) Type <down>: instead of selecting "foofoo2" as expected, the
selection is now on "foobaz", as if there was still a reference to the
previous "foobar" selection somehow. This is even more visible when
the completion list has more items.

I don't know if it's preferable to have the selection back to "foobar"
after <bs>, or to have it _really_ on "foofoo1", but the current
situation is wrong IMHO.

Best regards,
--
Olivier

[Christian Brabandt]

Hi Olivier!

Check this patch:

diff --git a/src/edit.c b/src/edit.c
--- a/src/edit.c
+++ b/src/edit.c
@@ -3441,6 +3441,7 @@
}

compl_enter_selects = !compl_used_match;
+ compl_shown_match = compl_curr_match = compl_first_match;

/* Show the popup menu with a different set of matches. */
ins_compl_show_pum();


regards,
Christian
--
Letzte Worte eines Chemikers:
"La� das Reagenzglas bitte in _Ruhe_ stehen."

[Olivier Teulière]

Hi Christian,

On Saturday, February 16, 2013 10:03:05 PM UTC+1, Christian Brabandt wrote:
> Check this patch:
> [...]

Your patch solves the problem, but maybe not in the best way. Now, after typing 'f' to shrink the list, the selection is lost in the popup. This means that I cannot simply type Enter to insert the selected entry and close the popup at the same time, which was very practical.

I think it would make more sense to keep the selection, and to return to one of the following states after <bs>:
- foofoobar1 is selected (both visually and "internally"), because it was selected before typing <bs>
- foobar is selected (both visually and "internally"), because it was selected before typing the letter erased by <bs>
Whichever is easier to implement :) Currently, it seems to be in the first state visually, and in the second one internally.

Best regards,
--
Olivier

[Christian Brabandt]

Hi Olivier!

The problem is, hitting backspace changes the leader and on the next
invocation of ins_complete() compl_curr_match() will be set wrongly
because ins_compl_next is called.

This patch should work better:

diff --git a/src/edit.c b/src/edit.c
--- a/src/edit.c
+++ b/src/edit.c

@@ -93,6 +93,7 @@
static compl_T *compl_first_match = NULL;
static compl_T *compl_curr_match = NULL;
static compl_T *compl_shown_match = NULL;
+static int did_bs = FALSE;

/* After using a cursor key <Enter> selects a match in the popup menu,
* otherwise it inserts a line break. */
@@ -3380,6 +3381,7 @@
if (compl_leader != NULL)
{
ins_compl_new_leader();
+ did_bs = TRUE;
return NUL;
}
return K_BS;
@@ -5363,7 +5365,10 @@
* Find next match (and following matches).
*/
save_w_wrow = curwin->w_wrow;
- n = ins_compl_next(TRUE, ins_compl_key2count(c), ins_compl_use_match(c));
+ /* don't add completions, after hitting backspace and the leader changed */
+ if (!did_bs)
+ n = ins_compl_next(TRUE, ins_compl_key2count(c), ins_compl_use_match(c));
+ did_bs = FALSE;

/* may undisplay the popup menu */
ins_compl_upd_pum();


Mit freundlichen Gr��en
Christian
--
Was man erfindet, tut man mit Liebe, was man gelernt hat, mit
Sicherheit.
-- Goethe, Maximen und Reflektionen, Nr. 1068

[Olivier Teuliere]

Hi Christian,

Sorry, but there doesn't seem to be any difference.
And there is a compilation warning:
edit.c: In function ‘ins_complete’:
edit.c:5377:16: warning: ‘n’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
edit.c:4944:9: note: ‘n’ was declared here

Best regards,
Olivier

On Tue, Feb 26, 2013 at 11:10 PM, Christian Brabandt <cbl...@256bit.org> wrote:
> Hi Olivier!
>

> Mit freundlichen Grüßen

> Christian
> --
> Was man erfindet, tut man mit Liebe, was man gelernt hat, mit
> Sicherheit.
> -- Goethe, Maximen und Reflektionen, Nr. 1068
>

> --
> --
> You received this message from the "vim_dev" maillist.
> Do not top-post! Type your reply below the text you are replying to.
> For more information, visit http://www.vim.org/maillist.php
>
> ---
> You received this message because you are subscribed to the Google Groups "vim_dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+u...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>



--
Olivier

[Christian Brabandt]

Hi Olivier!

On Di, 26 Feb 2013, Olivier Teuliere wrote:

> Hi Christian,
>
> Sorry, but there doesn't seem to be any difference.
> And there is a compilation warning:
> edit.c: In function ‘ins_complete’:
> edit.c:5377:16: warning: ‘n’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> edit.c:4944:9: note: ‘n’ was declared here

Yes. n needs to be initialized to -1. It fixes the problem you described
in your first message for me.

regards,
Christian

[Olivier Teuliere]

Hi Christian,

On Tue, Feb 26, 2013 at 11:56 PM, Christian Brabandt <cbl...@256bit.org> wrote:
> Yes. n needs to be initialized to -1. It fixes the problem you described
> in your first message for me.

Initializing n doesn't fix the initial problem either.
I don't know if I was supposed to revert your first patch before
applying the second one, but neither way works for me:
- if I apply only the second one, the behaviour described in my first
email is still there
- if I apply both, it's the same as when applying only the first one:
the selection is lost after typing the 'f'

Did you use the latest sources from hg for your tests?

Best regards,
--
Olivier

[Christian Brabandt]

Hi Olivier,
I used something like 7.3.840 patch yesterday night to test it and as
I said, it works for me, but let me look into it again later today.

regards,
Christian