fix(completion): improve popup window position (#26739)

src/nvim/insexpand.c

@@ -1307,26 +1307,30 @@ void ins_compl_show_pum(void)
   }
 }
 
-/// used for set or update info
+/// check selected is current match.
-void compl_set_info(int pum_idx)
+///
+/// @param selected the item which is selected.
+/// @return bool    return true when is current match otherwise is false.
+bool compl_match_curr_select(int selected)
 {
-  compl_T *comp = compl_first_match;
+  if (selected < 0) {
-  char *pum_text = compl_match_array[pum_idx].pum_text;
+    return false;
-
-  while (comp != NULL) {
-    if (pum_text == comp->cp_str
-        || pum_text == comp->cp_text[CPT_ABBR]) {
-      comp->cp_text[CPT_INFO] = compl_match_array[pum_idx].pum_info;
-
-      // if comp is current match update completed_item value
-      if (comp == compl_curr_match) {
-        dict_T *dict = ins_compl_dict_alloc(compl_curr_match);
-        set_vim_var_dict(VV_COMPLETED_ITEM, dict);
   }
+  compl_T *match = compl_first_match;
+  int selected_idx = -1, list_idx = 0;
+  do {
+    if (!match_at_original_text(match)) {
+      if (compl_curr_match != NULL
+          && compl_curr_match->cp_number == match->cp_number) {
+        selected_idx = list_idx;
         break;
       }
-    comp = comp->cp_next;
+      list_idx += 1;
     }
+    match = match->cp_next;
+  } while (match != NULL && !is_first_match(match));
+
+  return selected == selected_idx;
 }
 
 #define DICT_FIRST      (1)     ///< use just first element in "dict"

src/nvim/popupmenu.c

@@ -6,6 +6,7 @@
 #include <stdbool.h>
 #include <string.h>
 
+#include "nvim/api/buffer.h"
 #include "nvim/api/private/defs.h"
 #include "nvim/api/private/helpers.h"
 #include "nvim/api/vim.h"
@@ -13,11 +14,16 @@
 #include "nvim/autocmd.h"
 #include "nvim/buffer.h"
 #include "nvim/buffer_defs.h"
+#include "nvim/buffer_updates.h"
+#include "nvim/change.h"
 #include "nvim/charset.h"
 #include "nvim/drawscreen.h"
+#include "nvim/edit.h"
 #include "nvim/eval/typval.h"
 #include "nvim/ex_cmds.h"
 #include "nvim/ex_cmds_defs.h"
+#include "nvim/extmark.h"
+#include "nvim/extmark_defs.h"
 #include "nvim/getchar.h"
 #include "nvim/gettext_defs.h"
 #include "nvim/globals.h"
@@ -33,6 +39,7 @@
 #include "nvim/menu.h"
 #include "nvim/message.h"
 #include "nvim/move.h"
+#include "nvim/ops.h"
 #include "nvim/option.h"
 #include "nvim/option_defs.h"
 #include "nvim/option_vars.h"
@@ -665,64 +672,10 @@ void pum_redraw(void)
   }
 }
 
-/// create a floating preview window for info
-/// Autocommands are blocked for the duration of the call.
-/// @return  NULL when no enough room to show
-static win_T *pum_create_float_preview(bool enter)
-{
-  WinConfig config = WIN_CONFIG_INIT;
-  config.relative = kFloatRelativeEditor;
-  // when pum_above is SW otherwise is NW
-  config.anchor = pum_above ? kFloatAnchorSouth : 0;
-  int col = pum_col + pum_width + pum_scrollbar + 1;
-  // TODO(glepnir): support config align border by using completepopup
-  // align menu
-  config.row = pum_row;
-  int right_extra = Columns - col;
-  if (right_extra > 0) {
-    config.width = right_extra;
-    config.col = col - 1;
-  } else if (pum_col - 2 > 0) {
-    config.width = pum_col - 2;
-    config.col = pum_col - config.width - 1;
-  } else {
-    return NULL;
-  }
-  config.height = pum_height;
-  config.style = kWinStyleMinimal;
-  config.hide = true;
-
-  block_autocmds();
-
-  Error err = ERROR_INIT;
-  win_T *wp = win_new_float(NULL, true, config, &err);
-  // TODO(glepnir): remove win_enter usage
-  if (enter) {
-    win_enter(wp, false);
-  }
-
-  // create a new buffer
-  Buffer b = nvim_create_buf(false, true, &err);
-  if (!b) {
-    win_free(wp, NULL);
-    unblock_autocmds();
-    return NULL;
-  }
-  buf_T *buf = find_buffer_by_handle(b, &err);
-  set_option_direct_for(kOptBufhidden, STATIC_CSTR_AS_OPTVAL("wipe"), OPT_LOCAL, 0, kOptReqBuf,
-                        buf);
-  wp->w_float_is_info = true;
-  wp->w_p_diff = false;
-  buf->b_p_bl = false;
-  win_set_buf(wp, buf, &err);
-
-  unblock_autocmds();
-  return wp;
-}
-
 /// set info text to preview buffer.
 static void pum_preview_set_text(buf_T *buf, char *info, linenr_T *lnum, int *max_width)
 {
+  bcount_t inserted_bytes = 0;
   for (char *p = info; *p != NUL;) {
     int text_width = 0;
     char *e = vim_strchr(p, '\n');
@@ -736,6 +689,7 @@ static void pum_preview_set_text(buf_T *buf, char *info, linenr_T *lnum, int *ma
     }
     *e = NUL;
     ml_append_buf(buf, (*lnum)++, p, (int)(e - p + 1), false);
+    inserted_bytes += (bcount_t)strlen(p) + 1;
     text_width = (int)mb_string2cells(p);
     if (text_width > *max_width) {
       *max_width = text_width;
@@ -745,66 +699,78 @@ static void pum_preview_set_text(buf_T *buf, char *info, linenr_T *lnum, int *ma
   }
   // delete the empty last line
   ml_delete_buf(buf, buf->b_ml.ml_line_count, false);
+  if (strstr(p_cot, "popup") != NULL) {
+    extmark_splice(buf, 1, 0, 1, 0, 0, buf->b_ml.ml_line_count, 0, inserted_bytes, kExtmarkNoUndo);
+  }
 }
 
-/// adjust floating preview window width and height
+/// adjust floating info preview window position
-static void pum_adjust_float_position(win_T *wp, int height, int width)
+static void pum_adjust_info_position(win_T *wp, int height, int width)
 {
-  // when floating window in right and right no enough room to show
+  int col = pum_col + pum_width + pum_scrollbar + 1;
-  // but left has enough room, adjust floating window to left.
+  // TODO(glepnir): support config align border by using completepopup
-  if (wp->w_config.width < width && wp->w_config.col > pum_col) {
+  // align menu
-    if ((pum_col - 2) > width) {
+  int right_extra = Columns - col;
+  int left_extra = pum_col - 2;
+
+  if (right_extra > width) {  // place in right
     wp->w_config.width = width;
-      wp->w_config.col = pum_col - width - 1;
+    wp->w_config.col = col - 1;
+  } else if (left_extra > width) {  // place in left
+    wp->w_config.width = width;
+    wp->w_config.col = pum_col - wp->w_config.width - 1;
+  } else {  // either width is enough just use the biggest one.
+    const bool place_in_right = right_extra > left_extra;
+    wp->w_config.width = place_in_right ? right_extra : left_extra;
+    wp->w_config.col = place_in_right ? col - 1 : pum_col - wp->w_config.width - 1;
   }
-  }
+  // when pum_above is SW otherwise is NW
-  wp->w_config.width = MIN(wp->w_config.width, width);
+  wp->w_config.anchor = pum_above ? kFloatAnchorSouth : 0;
+  wp->w_config.row = pum_above ? pum_row + height : pum_row;
   wp->w_config.height = MIN(Rows, height);
   wp->w_config.hide = false;
   win_config_float(wp, wp->w_config);
 }
 
-/// used in nvim_complete_set
+/// Used for nvim_complete_set
-win_T *pum_set_info(int pum_idx, char *info)
+///
+/// @param selected the selected compl item.
+/// @parma info     Info string.
+/// @return a win_T pointer.
+win_T *pum_set_info(int selected, char *info)
 {
-  if (!pum_is_visible || pum_idx < 0 || pum_idx > pum_size) {
+  if (!pum_is_visible || !compl_match_curr_select(selected)) {
     return NULL;
   }
-  pum_array[pum_idx].pum_info = xstrdup(info);
-  compl_set_info(pum_idx);
-  bool use_float = strstr(p_cot, "popup") != NULL ? true : false;
-  if (pum_idx != pum_selected || !use_float) {
-    return NULL;
-  }
-
   block_autocmds();
   RedrawingDisabled++;
   no_u_sync++;
   win_T *wp = win_float_find_preview();
   if (wp == NULL) {
-    wp = pum_create_float_preview(false);
+    wp = win_float_create(false, true);
-    // no enough room to show
     if (!wp) {
       return NULL;
     }
   } else {
     // clean exist buffer
+    linenr_T count = wp->w_buffer->b_ml.ml_line_count;
     while (!buf_is_empty(wp->w_buffer)) {
       ml_delete_buf(wp->w_buffer, 1, false);
     }
+    bcount_t deleted_bytes = get_region_bytecount(wp->w_buffer, 1, count, 0, 0);
+    extmark_splice(wp->w_buffer, 1, 0, count, 0, deleted_bytes, 1, 0, 0, kExtmarkNoUndo);
   }
-  no_u_sync--;
-  RedrawingDisabled--;
-
   linenr_T lnum = 0;
   int max_info_width = 0;
   pum_preview_set_text(wp->w_buffer, info, &lnum, &max_info_width);
-  redraw_later(wp, UPD_SOME_VALID);
+  no_u_sync--;
+  RedrawingDisabled--;
+  redraw_later(wp, UPD_NOT_VALID);
 
   if (wp->w_p_wrap) {
     lnum += plines_win(wp, lnum, true);
   }
-  pum_adjust_float_position(wp, lnum, max_info_width);
+  pum_adjust_info_position(wp, lnum, max_info_width);
   unblock_autocmds();
   return wp;
 }
@@ -828,6 +794,13 @@ static bool pum_set_selected(int n, int repeat)
   int prev_selected = pum_selected;
 
   pum_selected = n;
+  bool use_float = strstr(p_cot, "popup") != NULL;
+  // when new leader add and info window is shown and no selected we still
+  // need use the first index item to update the info float window position.
+  bool force_select = use_float && pum_selected < 0 && win_float_find_preview();
+  if (force_select) {
+    pum_selected = 0;
+  }
 
   if ((pum_selected >= 0) && (pum_selected < pum_size)) {
     if (pum_first > pum_selected - 4) {
@@ -890,7 +863,6 @@ static bool pum_set_selected(int n, int repeat)
         && (vim_strchr(p_cot, 'p') != NULL)) {
       win_T *curwin_save = curwin;
       tabpage_T *curtab_save = curtab;
-      bool use_float = strstr(p_cot, "popup") != NULL ? true : false;
 
       if (use_float) {
         block_autocmds();
@@ -915,7 +887,7 @@ static bool pum_set_selected(int n, int repeat)
         if (wp) {
           win_enter(wp, false);
         } else {
-          wp = pum_create_float_preview(true);
+          wp = win_float_create(true, true);
           if (wp) {
             resized = true;
           }
@@ -986,7 +958,7 @@ static bool pum_set_selected(int n, int repeat)
               lnum += plines_win(curwin, lnum, true);
             }
             // adjust floating window by actually height and max info text width
-            pum_adjust_float_position(curwin, lnum, max_info_width);
+            pum_adjust_info_position(curwin, lnum, max_info_width);
           }
 
           if ((curwin != curwin_save && win_valid(curwin_save))
@@ -1046,6 +1018,11 @@ static bool pum_set_selected(int n, int repeat)
     }
   }
 
+  // restore before selected value
+  if (force_select) {
+    pum_selected = n;
+  }
+
   return resized;
 }
 

src/nvim/winfloat.c

@@ -6,7 +6,9 @@
 #include "klib/kvec.h"
 #include "nvim/api/private/defs.h"
 #include "nvim/api/private/helpers.h"
+#include "nvim/api/vim.h"
 #include "nvim/ascii_defs.h"
+#include "nvim/autocmd.h"
 #include "nvim/buffer_defs.h"
 #include "nvim/drawscreen.h"
 #include "nvim/globals.h"
@@ -17,6 +19,7 @@
 #include "nvim/mouse.h"
 #include "nvim/move.h"
 #include "nvim/option.h"
+#include "nvim/option_defs.h"
 #include "nvim/optionstr.h"
 #include "nvim/pos_defs.h"
 #include "nvim/strings.h"
@@ -354,3 +357,52 @@ win_T *win_float_find_altwin(const win_T *win, const tabpage_T *tp)
   return (tabpage_win_valid(tp, tp->tp_prevwin) && tp->tp_prevwin != win) ? tp->tp_prevwin
                                                                           : tp->tp_firstwin;
 }
+
+/// create a floating preview window.
+///
+/// @param[in] bool enter floating window.
+/// @param[in] bool create a new buffer for window.
+///
+/// @return win_T
+win_T *win_float_create(bool enter, bool new_buf)
+{
+  WinConfig config = WIN_CONFIG_INIT;
+  config.col = curwin->w_wcol;
+  config.row = curwin->w_wrow;
+  config.relative = kFloatRelativeEditor;
+  config.focusable = false;
+  config.anchor = 0;  // NW
+  config.noautocmd = true;
+  config.hide = true;
+  config.style = kWinStyleMinimal;
+  Error err = ERROR_INIT;
+
+  block_autocmds();
+  win_T *wp = win_new_float(NULL, false, config, &err);
+  if (!wp) {
+    unblock_autocmds();
+    return NULL;
+  }
+
+  if (new_buf) {
+    Buffer b = nvim_create_buf(false, true, &err);
+    if (!b) {
+      win_remove(wp, NULL);
+      win_free(wp, NULL);
+      unblock_autocmds();
+      return NULL;
+    }
+    buf_T *buf = find_buffer_by_handle(b, &err);
+    buf->b_p_bl = false;  // unlist
+    set_option_direct_for(kOptBufhidden, STATIC_CSTR_AS_OPTVAL("wipe"), OPT_LOCAL, 0, kOptReqBuf,
+                          buf);
+    win_set_buf(wp, buf, &err);
+  }
+  unblock_autocmds();
+  wp->w_p_diff = false;
+  wp->w_float_is_info = true;
+  if (enter) {
+    win_enter(wp, false);
+  }
+  return wp;
+}

test/functional/ui/popupmenu_spec.lua

@@ -1594,13 +1594,13 @@ describe('builtin popupmenu', function()
     describe('floating window preview #popup', function()
       it('pum popup preview', function()
         --row must > 10
-        screen:try_resize(30, 11)
+        screen:try_resize(40, 11)
         exec([[
           funct Omni_test(findstart, base)
             if a:findstart
               return col(".") - 1
             endif
-            return [#{word: "one", info: "1info"}, #{word: "two", info: "2info"}, #{word: "three"}]
+            return [#{word: "one", info: "1info"}, #{word: "two", info: "2info"}, #{word: "looooooooooooooong"}]
           endfunc
           set omnifunc=Omni_test
           set completeopt=menu,popup
@@ -1614,14 +1614,13 @@ describe('builtin popupmenu', function()
           autocmd CompleteChanged * call Set_info()
         ]])
         feed('Gi<C-x><C-o>')
-
         --floating preview in right
         if multigrid then
           screen:expect {
             grid = [[
           ## grid 1
-            [2:------------------------------]|*10
+            [2:----------------------------------------]|*10
-            [3:------------------------------]|
+            [3:----------------------------------------]|
           ## grid 2
             one^                                     |
             {1:~                                       }|*9
@@ -1633,11 +1632,31 @@ describe('builtin popupmenu', function()
           ## grid 5
             {s:one                }|
             {n:two                }|
-            {n:three          }|
+            {n:looooooooooooooong }|
           ]],
             float_pos = {
               [5] = { -1, 'NW', 2, 1, 0, false, 100 },
-              [4] = { 1001, 'NW', 1, 1, 15, true, 50 },
+              [4] = { 1001, 'NW', 1, 1, 19, false, 50 },
+            },
+            win_viewport = {
+              [2] = {
+                win = 1000,
+                topline = 0,
+                botline = 2,
+                curline = 0,
+                curcol = 3,
+                linecount = 1,
+                sum_scroll_delta = 0,
+              },
+              [4] = {
+                win = 1001,
+                topline = 0,
+                botline = 2,
+                curline = 0,
+                curcol = 0,
+                linecount = 1,
+                sum_scroll_delta = 0,
+              },
             },
           }
         else
@@ -1646,83 +1665,35 @@ describe('builtin popupmenu', function()
             one^                                     |
             {s:one                }{n:1info}{1:                }|
             {n:two                     }{1:                }|
-            {n:three          }{1:               }|
+            {n:looooooooooooooong }{1:                     }|
             {1:~                                       }|*6
             {2:-- }{5:match 1 of 3}                         |
           ]],
-            unchanged = true,
           }
         end
 
-        -- test nvim_complete_set_info
+        -- info window position should be adjusted when new leader add
-        feed('<C-N><C-N>')
+        feed('<C-P>o')
-        vim.uv.sleep(10)
         if multigrid then
           screen:expect {
             grid = [[
           ## grid 1
-            [2:------------------------------]|*10
+            [2:----------------------------------------]|*10
-            [3:------------------------------]|
+            [3:----------------------------------------]|
           ## grid 2
-            three^                         |
+            o^                                       |
             {1:~                                       }|*9
           ## grid 3
-            {2:-- }{5:match 3 of 3}               |
+            {2:-- }{8:Back at original}                     |
           ## grid 4
-            {n:3info}|
+            {n:1info}|
             {n:     }|
           ## grid 5
             {n:one            }|
-            {n:two            }|
-            {s:three          }|
           ]],
             float_pos = {
               [5] = { -1, 'NW', 2, 1, 0, false, 100 },
-              [4] = { 1001, 'NW', 1, 1, 15, true, 50 },
+              [4] = { 1001, 'NW', 1, 1, 15, false, 50 },
-            },
-          }
-        else
-          screen:expect {
-            grid = [[
-            three^                         |
-            {n:one            3info}{1:          }|
-            {n:two                 }{1:          }|
-            {s:three          }{1:               }|
-            {1:~                             }|*6
-            {2:-- }{5:match 3 of 3}               |
-          ]],
-          }
-        end
-        -- make sure info has set
-        feed('<C-y>')
-        eq('3info', exec_lua('return vim.v.completed_item.info'))
-
-        -- preview in left
-        feed('<ESC>cc')
-        insert(('test'):rep(5))
-        feed('i<C-x><C-o>')
-        if multigrid then
-          screen:expect {
-            grid = [[
-          ## grid 1
-            [2:------------------------------]|*10
-            [3:------------------------------]|
-          ## grid 2
-            itesttesttesttesttesone^t      |
-            {1:~                             }|*9
-          ## grid 3
-            {2:-- }{5:match 1 of 3}               |
-          ## grid 5
-            {s: one      }|
-            {n: two      }|
-            {n: three    }|
-          ## grid 6
-            {n:1info}|
-            {n:     }|
-          ]],
-            float_pos = {
-              [5] = { -1, 'NW', 2, 1, 19, false, 100 },
-              [6] = { 1002, 'NW', 1, 1, 1, true, 50 },
             },
             win_viewport = {
               [2] = {
@@ -1730,7 +1701,66 @@ describe('builtin popupmenu', function()
                 topline = 0,
                 botline = 2,
                 curline = 0,
-                curcol = 23,
+                curcol = 1,
+                linecount = 1,
+                sum_scroll_delta = 0,
+              },
+              [4] = {
+                win = 1001,
+                topline = 0,
+                botline = 2,
+                curline = 0,
+                curcol = 0,
+                linecount = 1,
+                sum_scroll_delta = 0,
+              },
+            },
+          }
+        else
+          screen:expect {
+            grid = [[
+            o^                                       |
+            {n:one            1info}{1:                    }|
+            {1:~              }{n:     }{1:                    }|
+            {1:~                                       }|*7
+            {2:-- }{8:Back at original}                     |
+          ]],
+          }
+        end
+
+        -- test nvim_complete_set_info
+        feed('<ESC>cc<C-X><C-O><C-N><C-N>')
+        vim.uv.sleep(10)
+        if multigrid then
+          screen:expect {
+            grid = [[
+          ## grid 1
+            [2:----------------------------------------]|*10
+            [3:----------------------------------------]|
+          ## grid 2
+            looooooooooooooong^                      |
+            {1:~                                       }|*9
+          ## grid 3
+            {2:-- }{5:match 3 of 3}                         |
+          ## grid 5
+            {n:one                }|
+            {n:two                }|
+            {s:looooooooooooooong }|
+          ## grid 6
+            {n:3info}|
+            {n:     }|
+          ]],
+            float_pos = {
+              [5] = { -1, 'NW', 2, 1, 0, false, 100 },
+              [6] = { 1002, 'NW', 1, 1, 19, false, 50 },
+            },
+            win_viewport = {
+              [2] = {
+                win = 1000,
+                topline = 0,
+                botline = 2,
+                curline = 0,
+                curcol = 18,
                 linecount = 1,
                 sum_scroll_delta = 0,
               },
@@ -1748,10 +1778,71 @@ describe('builtin popupmenu', function()
         else
           screen:expect {
             grid = [[
+            looooooooooooooong^                      |
+            {n:one                3info}{1:                }|
+            {n:two                     }{1:                }|
+            {s:looooooooooooooong }{1:                     }|
+            {1:~                                       }|*6
+            {2:-- }{5:match 3 of 3}                         |
+          ]],
+          }
+        end
+
+        -- preview in left
+        feed('<ESC>cc')
+        insert(('test'):rep(5))
+        feed('i<C-x><C-o>')
+        if multigrid then
+          screen:expect {
+            grid = [[
+          ## grid 1
+            [2:----------------------------------------]|*10
+            [3:----------------------------------------]|
+          ## grid 2
             itesttesttesttesttesone^t                |
-            {1:~}{n:1info}{1:             }{s: one      }{1: }|
+            {1:~                                       }|*9
-            {1:~}{n:     }{1:             }{n: two      }{1: }|
+          ## grid 3
-            {1:~                  }{n: three    }{1: }|
+            {2:-- }{5:match 1 of 3}                         |
+          ## grid 5
+            {s: one                }|
+            {n: two                }|
+            {n: looooooooooooooong }|
+          ## grid 7
+            {n:1info}|
+            {n:     }|
+          ]],
+            float_pos = {
+              [7] = { 1003, 'NW', 1, 1, 14, false, 50 },
+              [5] = { -1, 'NW', 2, 1, 19, false, 100 },
+            },
+            win_viewport = {
+              [2] = {
+                win = 1000,
+                topline = 0,
+                botline = 2,
+                curline = 0,
+                curcol = 23,
+                linecount = 1,
+                sum_scroll_delta = 0,
+              },
+              [7] = {
+                win = 1003,
+                topline = 0,
+                botline = 2,
+                curline = 0,
+                curcol = 0,
+                linecount = 1,
+                sum_scroll_delta = 0,
+              },
+            },
+          }
+        else
+          screen:expect {
+            grid = [[
+            itesttesttesttesttesone^t                |
+            {1:~             }{n:1info}{s: one                }{1: }|
+            {1:~             }{n:      two                }{1: }|
+            {1:~                  }{n: looooooooooooooong }{1: }|
             {1:~                                       }|*6
             {2:-- }{5:match 1 of 3}                         |
           ]],