patch 9.1.1714: completion: wildmode=longest:full selects wrong item

Problem:  completion: wildmode=longest:full selects wrong item
          (zeertzjq)
Solution: Fix issue, refactor ex_getln.c slightly
          (Girish Palya)

fixes: #18102
closes: #18125

Signed-off-by: Girish Palya <girishji@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

src/cmdexpand.c

@@ -387,8 +387,7 @@ nextwild(
 }
 
 /*
- * Create and display a cmdline completion popup menu with items from
+ * Create completion popup menu with items from 'matches'.
- * 'matches'.
  */
     static int
 cmdline_pum_create(
@@ -396,11 +395,9 @@ cmdline_pum_create(
 	expand_T	*xp,
 	char_u		**matches,
 	int		numMatches,
-	int		showtail,
+	int		showtail)
-	int		noselect)
 {
-    int		i;
+    int	prefix_len;
-    int		prefix_len;
 
     // Add all the completion matches
     compl_match_array = ALLOC_MULT(pumitem_T, numMatches);
@@ -408,7 +405,7 @@ cmdline_pum_create(
 	return EXPAND_UNSUCCESSFUL;
 
     compl_match_arraysize = numMatches;
-    for (i = 0; i < numMatches; i++)
+    for (int i = 0; i < numMatches; i++)
     {
 	compl_match_array[i].pum_text = SHOW_MATCH(i);
 	compl_match_array[i].pum_info = NULL;
@@ -426,12 +423,6 @@ cmdline_pum_create(
 	    - vim_strsize(matches[0]);
     compl_startcol = MAX(0, compl_startcol - prefix_len);
 
-    // no default selection
-    compl_selected = noselect ? -1 : 0;
-
-    pum_clear();
-    cmdline_pum_display();
-
     return EXPAND_OK;
 }
 
@@ -868,14 +859,27 @@ get_next_or_prev_match(int mode, expand_T *xp)
     }
 
     // Display matches on screen
-    if (compl_match_array)
+    if (p_wmnu)
     {
-	compl_selected = findex;
+	if (compl_match_array)
-	cmdline_pum_display();
+	{
+	    compl_selected = findex;
+	    cmdline_pum_display();
+	}
+	else if (vim_strchr(p_wop, WOP_PUM) != NULL)
+	{
+	    if (cmdline_pum_create(get_cmdline_info(), xp, xp->xp_files,
+			xp->xp_numfiles, cmd_showtail) == EXPAND_OK)
+	    {
+		compl_selected = findex;
+		pum_clear();
+		cmdline_pum_display();
+	    }
+	}
+	else
+	    win_redr_status_matches(xp, xp->xp_numfiles, xp->xp_files, findex,
+		    cmd_showtail);
     }
-    else if (p_wmnu)
-	win_redr_status_matches(xp, xp->xp_numfiles, xp->xp_files, findex,
-		cmd_showtail);
 
     xp->xp_selected = findex;
     // Return the original text or the selected match
@@ -1277,12 +1281,12 @@ showmatches_oneline(
 }
 
 /*
- * Show all matches for completion on the command line.
+ * Display completion matches.
- * Returns EXPAND_NOTHING when the character that triggered expansion should
+ * Returns EXPAND_NOTHING when the character that triggered expansion should be
- * be inserted like a normal character.
+ *   inserted as a normal character.
  */
     int
-showmatches(expand_T *xp, int wildmenu, int noselect)
+showmatches(expand_T *xp, int display_wildmenu, int display_list, int noselect)
 {
     cmdline_info_T	*ccline = get_cmdline_info();
     int		numMatches;
@@ -1311,12 +1315,21 @@ showmatches(expand_T *xp, int wildmenu, int noselect)
 	showtail = cmd_showtail;
     }
 
-    if (wildmenu && vim_strchr(p_wop, WOP_PUM) != NULL)
+    if (display_wildmenu && !display_list
-	// cmdline completion popup menu (with wildoptions=pum)
+	    && vim_strchr(p_wop, WOP_PUM) != NULL)
-	return cmdline_pum_create(ccline, xp, matches, numMatches,
+    {
-		showtail && !noselect, noselect);
+	int retval = cmdline_pum_create(ccline, xp, matches, numMatches,
+		showtail && !noselect);
+	if (retval == EXPAND_OK)
+	{
+	    compl_selected = noselect ? -1 : 0;
+	    pum_clear();
+	    cmdline_pum_display();
+	}
+	return retval;
+    }
 
-    if (!wildmenu)
+    if (display_list)
     {
 	msg_didany = FALSE;		// lines_left will be set
 	msg_start();			// prepare for paging
@@ -1328,10 +1341,11 @@ showmatches(expand_T *xp, int wildmenu, int noselect)
     }
 
     if (got_int)
-	got_int = FALSE;	// only int. the completion, not the cmd line
+	got_int = FALSE;  // only interrupt the completion, not the cmd line
-    else if (wildmenu)
+    else if (display_wildmenu && !display_list)
-	win_redr_status_matches(xp, numMatches, matches, noselect ? -1 : 0, showtail);
+	win_redr_status_matches(xp, numMatches, matches, noselect ? -1 : 0,
-    else
+		showtail);  // display statusbar menu
+    else if (display_list)
     {
 	// find the length of the longest file name
 	maxlen = 0;
@@ -4236,7 +4250,7 @@ wildmenu_translate_key(
 	}
     }
 
-    if (did_wild_list)
+    if (cmdline_pum_active() || did_wild_list || wild_menu_showing)
     {
 	if (c == K_LEFT)
 	    c = Ctrl_P;

src/ex_getln.c

@@ -944,23 +944,20 @@ cmdline_wildchar_complete(
 {
     int		wim_index = *wim_index_p;
     int		res;
-    int		j;
+    int		cmdpos_before;
     int		options = WILD_NO_BEEP;
-    int		noselect = p_wmnu && (wim_flags[0] & WIM_NOSELECT);
+    int		wim_noselect = p_wmnu && (wim_flags[0] & WIM_NOSELECT);
 
     if (wim_flags[wim_index] & WIM_BUFLASTUSED)
 	options |= WILD_BUFLASTUSED;
     if (xp->xp_numfiles > 0)   // typed p_wc at least twice
     {
-	// if 'wildmode' contains "list" may still need to list
+	// If "list" is present, list matches unless already listed
 	if (xp->xp_numfiles > 1
 		&& !*did_wild_list
-		&& ((wim_flags[wim_index] & WIM_LIST)
+		&& (wim_flags[wim_index] & WIM_LIST))
-		    || (p_wmnu && (wim_flags[wim_index] & WIM_FULL) != 0)))
 	{
-	    (void)showmatches(xp,
+	    (void)showmatches(xp, FALSE, TRUE, wim_noselect);
-		    p_wmnu && ((wim_flags[wim_index] & WIM_LIST) == 0),
-		    noselect);
 	    redrawcmd();
 	    *did_wild_list = TRUE;
 	}
@@ -973,6 +970,11 @@ cmdline_wildchar_complete(
     }
     else		    // typed p_wc first time
     {
+	int wim_longest = (wim_flags[0] & WIM_LONGEST);
+	int wim_list = (wim_flags[0] & WIM_LIST);
+	int wim_full = (wim_flags[0] & WIM_FULL);
+
+	wim_index = 0;
 	if (c == p_wc || c == p_wcm || c == K_WILD)
 	{
 	    options |= WILD_MAY_EXPAND_PATTERN;
@@ -983,15 +985,15 @@ cmdline_wildchar_complete(
 	    else
 		xp->xp_pre_incsearch_pos = curwin->w_cursor;
 	}
-	wim_index = 0;
+	cmdpos_before = ccline.cmdpos;
-	j = ccline.cmdpos;
+
 	// if 'wildmode' first contains "longest", get longest
 	// common part
-	if (wim_flags[0] & WIM_LONGEST)
+	if (wim_longest)
 	    res = nextwild(xp, WILD_LONGEST, options, escape);
 	else
 	{
-	    if (noselect || (wim_flags[wim_index] & WIM_LIST))
+	    if (wim_noselect || wim_list)
 		options |= WILD_NOSELECT;
 	    res = nextwild(xp, WILD_EXPAND_KEEP, options, escape);
 	}
@@ -1011,28 +1013,29 @@ cmdline_wildchar_complete(
 	    return CMDLINE_CHANGED;
 	}
 
-	// when more than one match, and 'wildmode' first contains
+	// Display matches
-	// "list", or no change and 'wildmode' contains "longest,list",
+	if (res == OK && xp->xp_numfiles > (wim_noselect ? 0 : 1))
-	// list all matches
-	if (res == OK
-		&& xp->xp_numfiles > (noselect ? 0 : 1))
 	{
-	    // a "longest" that didn't do anything is skipped (but not
+	    // If "longest" fails to identify the longest item, try other
-	    // "list:longest")
+	    // 'wim' values if available
-	    if (wim_flags[0] == WIM_LONGEST && ccline.cmdpos == j)
+	    if (wim_longest && ccline.cmdpos == cmdpos_before)
-		wim_index = 1;
-	    if ((wim_flags[wim_index] & WIM_LIST)
-		    || (p_wmnu && (wim_flags[wim_index] & (WIM_FULL | WIM_NOSELECT))))
 	    {
-		(void)showmatches(xp, p_wmnu
+		if (wim_full)
-			&& ((wim_flags[wim_index] & WIM_LIST) == 0), noselect);
+		    nextwild(xp, WILD_NEXT, options, escape);
-		redrawcmd();
+		if (wim_list || (p_wmnu && wim_full))
-		*did_wild_list = TRUE;
+		    (void)showmatches(xp, p_wmnu, wim_list, FALSE);
-		if (wim_flags[wim_index] & WIM_LONGEST)
-		    nextwild(xp, WILD_LONGEST, options, escape);
 	    }
-	    else
+	    else if (!wim_longest)
-		vim_beep(BO_WILD);
+	    {
+		if (wim_list || (p_wmnu && (wim_full || wim_noselect)))
+		    (void)showmatches(xp, p_wmnu, wim_list, wim_noselect);
+		else
+		    vim_beep(BO_WILD);
+	    }
+
+	    redrawcmd();
+	    if (wim_list)
+		*did_wild_list = TRUE;
 	}
 	else if (xp->xp_numfiles == -1)
 	    xp->xp_context = EXPAND_NOTHING;
@@ -1959,7 +1962,8 @@ getcmdline_int(
 	    c = wildmenu_translate_key(&ccline, c, &xpc, did_wild_list);
 
 	int key_is_wc = (c == p_wc && KeyTyped) || c == p_wcm;
-	if ((cmdline_pum_active() || did_wild_list) && !key_is_wc)
+	if ((cmdline_pum_active() || wild_menu_showing || did_wild_list)
+		&& !key_is_wc)
 	{
 	    // Ctrl-Y: Accept the current selection and close the popup menu.
 	    // Ctrl-E: cancel the cmdline popup menu and return the original
@@ -2118,8 +2122,7 @@ getcmdline_int(
 			    || p_wmnu))
 		{
 		    // Trigger the popup menu when wildoptions=pum
-		    showmatches(&xpc, p_wmnu
+		    showmatches(&xpc, p_wmnu, wim_flags[wim_index] & WIM_LIST,
-			    && ((wim_flags[wim_index] & WIM_LIST) == 0),
 			    wim_flags[0] & WIM_NOSELECT);
 		}
 		if (nextwild(&xpc, WILD_PREV, 0, firstc != '@') == OK
@@ -2235,7 +2238,7 @@ getcmdline_int(
 		goto cmdline_not_changed;
 
 	case Ctrl_D:
-		if (showmatches(&xpc, FALSE, wim_flags[0] & WIM_NOSELECT)
+		if (showmatches(&xpc, FALSE, TRUE, wim_flags[0] & WIM_NOSELECT)
 			== EXPAND_NOTHING)
 		    break;	// Use ^D as normal char instead
 

src/proto/cmdexpand.pro

@@ -12,7 +12,7 @@ char_u *ExpandOne(expand_T *xp, char_u *str, char_u *orig, int options, int mode
 void ExpandInit(expand_T *xp);
 void ExpandCleanup(expand_T *xp);
 void clear_cmdline_orig(void);
-int showmatches(expand_T *xp, int wildmenu, int noselect);
+int showmatches(expand_T *xp, int display_wildmenu, int display_list, int noselect);
 char_u *addstar(char_u *fname, int len, int context);
 void set_expand_context(expand_T *xp);
 void set_cmd_context(expand_T *xp, char_u *str, int len, int col, int use_ccline);

src/testdir/dumps/Test_wildmenu_pum_30.dump

@@ -1,10 +1,10 @@
-|~+0#4040ff13#ffffff0| @73
+| +0&#ffffff0@74
+|~+0#4040ff13&| @73
 |~| @73
 |~| @73
 |~| @73
 |~| @73
 |~| @73
-|:+0#0000000&|c|n| @71
+|~| @73
-|c|n|e|w|e|r| @6|c|n|f|i|l|e| @6|c|n|o|r|e|m|a|p| @40
+|~| @73
-|c|n|e|x|t| @7|c|n|o|r|e|a|b@1|r|e|v| @1|c|n|o|r|e|m|e|n|u| @39
+|:+0#0000000&|c|n> @71
-|:|c|n> @71

src/testdir/dumps/Test_wildmenu_pum_31.dump

@@ -7,4 +7,4 @@
 |:+0#0000000&|c|n| @71
 |c|n|e|w|e|r| @6|c|n|f|i|l|e| @6|c|n|o|r|e|m|a|p| @40
 |c|n|e|x|t| @7|c|n|o|r|e|a|b@1|r|e|v| @1|c|n|o|r|e|m|e|n|u| @39
-|:|c|n|s> @70
+|:|c|n> @71

src/testdir/dumps/Test_wildmenu_pum_54.dump

@@ -0,0 +1,10 @@
+| +0&#ffffff0@74
+|~+0#4040ff13&| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|c+0#0000001#ffff4012|n|e|w|e|r| +3#0000000#ffffff0@1|c|n|e|x|t| @1|c|n|f|i|l|e| @1|c|n|o|r|e|a|b@1|r|e|v| @1|c|n|o|r|e|m|a|p| @1|c|n|o|r|e|m|e|n|u| @19
+|:+0&&|c|n|e|w|e|r> @67

src/testdir/dumps/Test_wildmenu_pum_55.dump

@@ -0,0 +1,10 @@
+|~+0#4040ff13#ffffff0| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|:+0#0000000&|c|n| @71
+|c|n|e|w|e|r| @6|c|n|f|i|l|e| @6|c|n|o|r|e|m|a|p| @40
+|c|n|e|x|t| @7|c|n|o|r|e|a|b@1|r|e|v| @1|c|n|o|r|e|m|e|n|u| @39
+|:|c|n> @71

src/testdir/dumps/Test_wildmenu_pum_56.dump

@@ -0,0 +1,10 @@
+|~+0#4040ff13#ffffff0| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|:+0#0000000&|c|n| @71
+|c|n|e|w|e|r| @6|c|n|f|i|l|e| @6|c|n|o|r|e|m|a|p| @40
+|c|n|e|x|t| @7|c|n|o|r|e|a|b@1|r|e|v| @1|c|n|o|r|e|m|e|n|u| @39
+|c+0#0000001#ffff4012|n|e|w|e|r| +3#0000000#ffffff0@1|c|n|e|x|t| @1|c|n|f|i|l|e| @1|c|n|o|r|e|a|b@1|r|e|v| @1|c|n|o|r|e|m|a|p| @1|c|n|o|r|e|m|e|n|u| @19
+|:+0&&|c|n|e|w|e|r> @67

src/testdir/dumps/Test_wildmenu_pum_57.dump

@@ -0,0 +1,10 @@
+| +0&#ffffff0@74
+|~+0#4040ff13&| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|:+0#0000000&|s|i|g|n| |u|n> @66

src/testdir/dumps/Test_wildmenu_pum_58.dump

@@ -0,0 +1,10 @@
+| +0&#ffffff0@74
+|~+0#4040ff13&| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|~| @73
+|u+0#0000001#ffff4012|n|d|e|f|i|n|e| +3#0000000#ffffff0@1|u|n|p|l|a|c|e| @57
+|:+0&&|s|i|g|n| |u|n|d|e|f|i|n|e> @60

src/testdir/test_cmdline.vim

@@ -2825,11 +2825,11 @@ func Test_wildmenu_pum()
   call term_sendkeys(buf, "sign xyz\<Esc>:sign \<Tab>\<C-E>\<Up>")
   call VerifyScreenDump(buf, 'Test_wildmenu_pum_29', {})
 
-  " Check "list" still works
+  " Check that when "longest" produces no result, "list" works
   call term_sendkeys(buf, "\<C-U>set wildmode=longest,list\<CR>")
   call term_sendkeys(buf, ":cn\<Tab>")
   call VerifyScreenDump(buf, 'Test_wildmenu_pum_30', {})
-  call term_sendkeys(buf, "s")
+  call term_sendkeys(buf, "\<Tab>")
   call VerifyScreenDump(buf, 'Test_wildmenu_pum_31', {})
 
   " Tests a directory name contained full-width characters.
@@ -2929,7 +2929,32 @@ func Test_wildmenu_pum()
   call term_sendkeys(buf, "\<Esc>:set wildchazz\<Left>\<Left>\<Tab>\<C-Y>")
   call VerifyScreenDump(buf, 'Test_wildmenu_pum_53', {})
 
-  call term_sendkeys(buf, "\<C-U>\<CR>")
+  call term_sendkeys(buf, "\<Esc>:set showtabline& laststatus& lazyredraw&\<CR>")
+
+  " Verify that if "longest" finds nothing, wildmenu is still shown
+  call term_sendkeys(buf, ":set wildmode=longest:full,full wildoptions&\<CR>")
+  call term_sendkeys(buf, ":cn\<Tab>")
+  call TermWait(buf, 50)
+  call VerifyScreenDump(buf, 'Test_wildmenu_pum_54', {})
+
+  " Verify that if "longest" finds nothing, "list" is still shown
+  call term_sendkeys(buf, "\<Esc>:set wildmode=longest:list,full\<CR>")
+  call term_sendkeys(buf, ":cn\<Tab>")
+  call TermWait(buf, 50)
+  call VerifyScreenDump(buf, 'Test_wildmenu_pum_55', {})
+  call term_sendkeys(buf, "\<Tab>")
+  call TermWait(buf, 50)
+  call VerifyScreenDump(buf, 'Test_wildmenu_pum_56', {})
+
+  " Verify that if "longest" finds a candidate, wildmenu is not shown
+  call term_sendkeys(buf, "\<Esc>:set wildmode=longest:full,full wildoptions&\<CR>")
+  call term_sendkeys(buf, ":sign u\<Tab>")
+  call VerifyScreenDump(buf, 'Test_wildmenu_pum_57', {})
+  " Subsequent wildchar shows wildmenu
+  call term_sendkeys(buf, "\<Tab>")
+  call VerifyScreenDump(buf, 'Test_wildmenu_pum_58', {})
+
+  call term_sendkeys(buf, "\<C-U>\<Esc>")
   call StopVimInTerminal(buf)
 endfunc
 

src/testdir/test_ins_complete.vim

@@ -5507,7 +5507,7 @@ func Test_completetimeout_autocompletetimeout()
   set completetimeout=1
   call feedkeys("Gof\<C-N>\<F2>\<Esc>0", 'xt!')
   let match_count = len(b:matches->mapnew('v:val.word'))
-  call assert_true(match_count < 2000)
+  call assert_true(match_count < 4000)
 
   set completetimeout=1000
   call feedkeys("\<Esc>Sf\<C-N>\<F2>\<Esc>0", 'xt!')

src/version.c

@@ -724,6 +724,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1714,
 /**/
     1713,
 /**/