Merge #8913 'popupmenu placement'

close #8913

5 files changed, 350 insertions, 178 deletions

diff --git a/src/nvim/popupmnu.c b/src/nvim/popupmnu.c
index 60c7502fa4..4c51f5de5e 100644
--- a/src/nvim/popupmnu.c
+++ b/src/nvim/popupmnu.c
@@ -81,225 +81,224 @@ void pum_display(pumitem_T *array, int size, int selected, bool array_changed)
     pum_external = ui_is_external(kUIPopupmenu);
   }
 
-redo:
-  // Mark the pum as visible already here,
-  // to avoid that must_redraw is set when 'cursorcolumn' is on.
-  pum_is_visible = true;
-  validate_cursor_col();
-  above_row = 0;
-  below_row = cmdline_row;
-
-  // anchor position: the start of the completed word
-  row = curwin->w_wrow + curwin->w_winrow;
-  if (curwin->w_p_rl) {
-    col = curwin->w_wincol + curwin->w_width - curwin->w_wcol - 1;
-  } else {
-    col = curwin->w_wincol + curwin->w_wcol;
-  }
-
-  if (pum_external) {
-    if (array_changed) {
-      Array arr = ARRAY_DICT_INIT;
-      for (i = 0; i < size; i++) {
-        Array item = ARRAY_DICT_INIT;
-        ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_text)));
-        ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_kind)));
-        ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_extra)));
-        ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_info)));
-        ADD(arr, ARRAY_OBJ(item));
-      }
-      ui_call_popupmenu_show(arr, selected, row, col);
+  do {
+    // Mark the pum as visible already here,
+    // to avoid that must_redraw is set when 'cursorcolumn' is on.
+    pum_is_visible = true;
+    validate_cursor_col();
+    above_row = 0;
+    below_row = cmdline_row;
+
+    // anchor position: the start of the completed word
+    row = curwin->w_wrow + curwin->w_winrow;
+    if (curwin->w_p_rl) {
+      col = curwin->w_wincol + curwin->w_width - curwin->w_wcol - 1;
     } else {
-      ui_call_popupmenu_select(selected);
+      col = curwin->w_wincol + curwin->w_wcol;
     }
-    return;
-  }
-
-  def_width = PUM_DEF_WIDTH;
-  max_width = 0;
-  kind_width = 0;
-  extra_width = 0;
-
-  win_T *pvwin = NULL;
-  FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
-    if (wp->w_p_pvw) {
-      pvwin = wp;
-      break;
-    }
-  }
 
-  if (pvwin != NULL) {
-    if (pvwin->w_wrow < curwin->w_wrow) {
-      above_row = pvwin->w_wrow + pvwin->w_height;
-    } else if (pvwin->w_wrow > pvwin->w_wrow + curwin->w_height) {
-      below_row = pvwin->w_wrow;
+    if (pum_external) {
+      if (array_changed) {
+        Array arr = ARRAY_DICT_INIT;
+        for (i = 0; i < size; i++) {
+          Array item = ARRAY_DICT_INIT;
+          ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_text)));
+          ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_kind)));
+          ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_extra)));
+          ADD(item, STRING_OBJ(cstr_to_string((char *)array[i].pum_info)));
+          ADD(arr, ARRAY_OBJ(item));
+        }
+        ui_call_popupmenu_show(arr, selected, row, col);
+      } else {
+        ui_call_popupmenu_select(selected);
+      }
+      return;
     }
-  }
-
-  // Figure out the size and position of the pum.
-  if (size < PUM_DEF_HEIGHT) {
-    pum_height = size;
-  } else {
-    pum_height = PUM_DEF_HEIGHT;
-  }
 
-  if ((p_ph > 0) && (pum_height > p_ph)) {
-    pum_height = (int)p_ph;
-  }
+    def_width = PUM_DEF_WIDTH;
+    max_width = 0;
+    kind_width = 0;
+    extra_width = 0;
 
-  // Put the pum below "row" if possible.  If there are few lines decide on
-  // where there is more room.
-  if (row + 2 >= below_row - pum_height
-      && row - above_row > (below_row - above_row) / 2) {
-    // pum above "row"
+    win_T *pvwin = NULL;
+    FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
+      if (wp->w_p_pvw) {
+        pvwin = wp;
+        break;
+      }
+    }
 
-    // Leave two lines of context if possible
-    if (curwin->w_wrow - curwin->w_cline_row >= 2) {
-      context_lines = 2;
-    } else {
-      context_lines = curwin->w_wrow - curwin->w_cline_row;
+    if (pvwin != NULL) {
+      if (pvwin->w_winrow < curwin->w_winrow) {
+        above_row = pvwin->w_winrow + pvwin->w_height;
+      } else if (pvwin->w_winrow > curwin->w_winrow + curwin->w_height) {
+        below_row = pvwin->w_winrow;
+      }
     }
 
-    if (row >= size + context_lines) {
-      pum_row = row - size - context_lines;
+    // Figure out the size and position of the pum.
+    if (size < PUM_DEF_HEIGHT) {
       pum_height = size;
     } else {
-      pum_row = 0;
-      pum_height = row - context_lines;
+      pum_height = PUM_DEF_HEIGHT;
     }
 
     if ((p_ph > 0) && (pum_height > p_ph)) {
-      pum_row += pum_height - (int)p_ph;
       pum_height = (int)p_ph;
     }
-  } else {
-    // pum below "row"
 
-    // Leave two lines of context if possible
-    if (curwin->w_cline_row + curwin->w_cline_height - curwin->w_wrow >= 3) {
-      context_lines = 3;
-    } else {
-      context_lines = curwin->w_cline_row
-                      + curwin->w_cline_height - curwin->w_wrow;
-    }
+    // Put the pum below "row" if possible.  If there are few lines decide on
+    // where there is more room.
+    if (row + 2 >= below_row - pum_height
+        && row - above_row > (below_row - above_row) / 2) {
+      // pum above "row"
 
-    pum_row = row + context_lines;
-    if (size > below_row - pum_row) {
-      pum_height = below_row - pum_row;
+      // Leave two lines of context if possible
+      if (curwin->w_wrow - curwin->w_cline_row >= 2) {
+        context_lines = 2;
+      } else {
+        context_lines = curwin->w_wrow - curwin->w_cline_row;
+      }
+
+      if (row >= size + context_lines) {
+        pum_row = row - size - context_lines;
+        pum_height = size;
+      } else {
+        pum_row = 0;
+        pum_height = row - context_lines;
+      }
+
+      if ((p_ph > 0) && (pum_height > p_ph)) {
+        pum_row += pum_height - (int)p_ph;
+        pum_height = (int)p_ph;
+      }
     } else {
-      pum_height = size;
-    }
+      // pum below "row"
 
-    if ((p_ph > 0) && (pum_height > p_ph)) {
-      pum_height = (int)p_ph;
-    }
-  }
+      // Leave two lines of context if possible
+      if (curwin->w_cline_row + curwin->w_cline_height - curwin->w_wrow >= 3) {
+        context_lines = 3;
+      } else {
+        context_lines = curwin->w_cline_row
+          + curwin->w_cline_height - curwin->w_wrow;
+      }
 
-  // don't display when we only have room for one line
-  if ((pum_height < 1) || ((pum_height == 1) && (size > 1))) {
-    return;
-  }
+      pum_row = row + context_lines;
+      if (size > below_row - pum_row) {
+        pum_height = below_row - pum_row;
+      } else {
+        pum_height = size;
+      }
 
-  // If there is a preview window at the above avoid drawing over it.
-  if (pvwin != NULL && pum_row < above_row && pum_height > above_row) {
-    pum_row += above_row;
-    pum_height -= above_row;
-  }
+      if ((p_ph > 0) && (pum_height > p_ph)) {
+        pum_height = (int)p_ph;
+      }
+    }
 
-  // Compute the width of the widest match and the widest extra.
-  for (i = 0; i < size; ++i) {
-    w = vim_strsize(array[i].pum_text);
+    // don't display when we only have room for one line
+    if ((pum_height < 1) || ((pum_height == 1) && (size > 1))) {
+      return;
+    }
 
-    if (max_width < w) {
-      max_width = w;
+    // If there is a preview window above, avoid drawing over it.
+    if (pvwin != NULL && pum_row < above_row && pum_height > above_row) {
+      pum_row += above_row;
+      pum_height -= above_row;
     }
 
-    if (array[i].pum_kind != NULL) {
-      w = vim_strsize(array[i].pum_kind) + 1;
+    // Compute the width of the widest match and the widest extra.
+    for (i = 0; i < size; i++) {
+      w = vim_strsize(array[i].pum_text);
 
-      if (kind_width < w) {
-        kind_width = w;
+      if (max_width < w) {
+        max_width = w;
       }
-    }
 
-    if (array[i].pum_extra != NULL) {
-      w = vim_strsize(array[i].pum_extra) + 1;
+      if (array[i].pum_kind != NULL) {
+        w = vim_strsize(array[i].pum_kind) + 1;
 
-      if (extra_width < w) {
-        extra_width = w;
+        if (kind_width < w) {
+          kind_width = w;
+        }
       }
-    }
-  }
-  pum_base_width = max_width;
-  pum_kind_width = kind_width;
 
-  // if there are more items than room we need a scrollbar
-  if (pum_height < size) {
-    pum_scrollbar = 1;
-    max_width++;
-  } else {
-    pum_scrollbar = 0;
-  }
+      if (array[i].pum_extra != NULL) {
+        w = vim_strsize(array[i].pum_extra) + 1;
 
-  if (def_width < max_width) {
-    def_width = max_width;
-  }
+        if (extra_width < w) {
+          extra_width = w;
+        }
+      }
+    }
+    pum_base_width = max_width;
+    pum_kind_width = kind_width;
 
-  if ((((col < Columns - PUM_DEF_WIDTH) || (col < Columns - max_width))
-       && !curwin->w_p_rl)
-      || (curwin->w_p_rl && ((col > PUM_DEF_WIDTH) || (col > max_width)))) {
-    // align pum column with "col"
-    pum_col = col;
-    if (curwin->w_p_rl) {
-      pum_width = pum_col - pum_scrollbar + 1;
+    // if there are more items than room we need a scrollbar
+    if (pum_height < size) {
+      pum_scrollbar = 1;
+      max_width++;
     } else {
-      assert(Columns - pum_col - pum_scrollbar >= INT_MIN
-             && Columns - pum_col - pum_scrollbar <= INT_MAX);
-      pum_width = (int)(Columns - pum_col - pum_scrollbar);
+      pum_scrollbar = 0;
     }
 
-    if ((pum_width > max_width + kind_width + extra_width + 1)
-        && (pum_width > PUM_DEF_WIDTH)) {
-      pum_width = max_width + kind_width + extra_width + 1;
+    if (def_width < max_width) {
+      def_width = max_width;
+    }
 
-      if (pum_width < PUM_DEF_WIDTH) {
-        pum_width = PUM_DEF_WIDTH;
+    if ((((col < Columns - PUM_DEF_WIDTH) || (col < Columns - max_width))
+         && !curwin->w_p_rl)
+        || (curwin->w_p_rl && ((col > PUM_DEF_WIDTH) || (col > max_width)))) {
+      // align pum column with "col"
+      pum_col = col;
+      if (curwin->w_p_rl) {
+        pum_width = pum_col - pum_scrollbar + 1;
+      } else {
+        assert(Columns - pum_col - pum_scrollbar >= INT_MIN
+               && Columns - pum_col - pum_scrollbar <= INT_MAX);
+        pum_width = (int)(Columns - pum_col - pum_scrollbar);
+      }
+
+      if ((pum_width > max_width + kind_width + extra_width + 1)
+          && (pum_width > PUM_DEF_WIDTH)) {
+        pum_width = max_width + kind_width + extra_width + 1;
+
+        if (pum_width < PUM_DEF_WIDTH) {
+          pum_width = PUM_DEF_WIDTH;
+        }
+      }
+    } else if (Columns < def_width) {
+      // not enough room, will use what we have
+      if (curwin->w_p_rl) {
+        assert(Columns - 1 >= INT_MIN);
+        pum_col = (int)(Columns - 1);
+      } else {
+        pum_col = 0;
       }
-    }
-  } else if (Columns < def_width) {
-    // not enough room, will use what we have
-    if (curwin->w_p_rl) {
       assert(Columns - 1 >= INT_MIN);
-      pum_col = (int)(Columns - 1);
+      pum_width = (int)(Columns - 1);
     } else {
-      pum_col = 0;
-    }
-    assert(Columns - 1 >= INT_MIN);
-    pum_width = (int)(Columns - 1);
-  } else {
-    if (max_width > PUM_DEF_WIDTH) {
-      // truncate
-      max_width = PUM_DEF_WIDTH;
-    }
+      if (max_width > PUM_DEF_WIDTH) {
+        // truncate
+        max_width = PUM_DEF_WIDTH;
+      }
 
-    if (curwin->w_p_rl) {
-      pum_col = max_width - 1;
-    } else {
-      assert(Columns - max_width >= INT_MIN && Columns - max_width <= INT_MAX);
-      pum_col = (int)(Columns - max_width);
+      if (curwin->w_p_rl) {
+        pum_col = max_width - 1;
+      } else {
+        assert(Columns - max_width >= INT_MIN
+               && Columns - max_width <= INT_MAX);
+        pum_col = (int)(Columns - max_width);
+      }
+      pum_width = max_width - pum_scrollbar;
     }
-    pum_width = max_width - pum_scrollbar;
-  }
 
-  pum_array = array;
-  pum_size = size;
+    pum_array = array;
+    pum_size = size;
 
-  // Set selected item and redraw.  If the window size changed need to redo
-  // the positioning.  Limit this to two times, when there is not much
-  // room the window size will keep changing.
-  if (pum_set_selected(selected, redo_count) && (++redo_count <= 2)) {
-    goto redo;
-  }
+    // Set selected item and redraw.  If the window size changed need to redo
+    // the positioning.  Limit this to two times, when there is not much
+    // room the window size will keep changing.
+  } while (pum_set_selected(selected, redo_count) && (++redo_count <= 2));
 }
 
 /// Redraw the popup menu, using "pum_first" and "pum_selected".
diff --git a/src/nvim/screen.c b/src/nvim/screen.c
index b38dff0d6c..08325184e4 100644
--- a/src/nvim/screen.c
+++ b/src/nvim/screen.c
@@ -7090,11 +7090,12 @@ void screen_resize(int width, int height)
         update_topline();
         if (pum_drawn()) {
           redraw_later(NOT_VALID);
-          ins_compl_show_pum();           /* This includes the redraw. */
-        } else
-          update_screen(NOT_VALID);
-        if (redrawing())
+          ins_compl_show_pum();
+        }
+        update_screen(NOT_VALID);
+        if (redrawing()) {
           setcursor();
+        }
       }
     }
   }
diff --git a/src/nvim/testdir/shared.vim b/src/nvim/testdir/shared.vim
index 4925b04a82..b2fe9413e8 100644
--- a/src/nvim/testdir/shared.vim
+++ b/src/nvim/testdir/shared.vim
@@ -1,5 +1,10 @@
 " Functions shared by several tests.
 
+" Only load this script once.
+if exists('*WaitFor')
+  finish
+endif
+
 " {Nvim}
 " Filepath captured from output may be truncated, like this:
 "   /home/va...estdir/Xtest-tmpdir/nvimxbXN4i/10
diff --git a/src/nvim/testdir/test_popup.vim b/src/nvim/testdir/test_popup.vim
index 2191e3144f..6fd58a1483 100644
--- a/src/nvim/testdir/test_popup.vim
+++ b/src/nvim/testdir/test_popup.vim
@@ -1,5 +1,7 @@
 " Test for completion menu
 
+source shared.vim
+
 let g:months = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']
 let g:setting = ''
 
@@ -665,4 +667,30 @@ func Test_complete_CTRLN_startofbuffer()
   bwipe!
 endfunc
 
+func Test_popup_and_window_resize()
+  if !has('terminal') || has('gui_running')
+    return
+  endif
+  let h = winheight(0)
+  if h < 15
+    return
+  endif
+  let g:buf = term_start([$NVIM_PRG, '--clean', '-c', 'set noswapfile'], {'term_rows': h / 3})
+  call term_sendkeys(g:buf, (h / 3 - 1)."o\<esc>G")
+  call term_sendkeys(g:buf, "i\<c-x>")
+  call term_wait(g:buf, 100)
+  call term_sendkeys(g:buf, "\<c-v>")
+  call term_wait(g:buf, 100)
+  call assert_match('^!\s*$', term_getline(g:buf, 1))
+  exe 'resize +' . (h - 1)
+  call term_wait(g:buf, 100)
+  redraw!
+  call WaitFor('"" == term_getline(g:buf, 1)')
+  call assert_equal('', term_getline(g:buf, 1))
+  sleep 100m
+  call WaitFor('"^!" =~ term_getline(g:buf, term_getcursor(g:buf)[0] + 1)')
+  call assert_match('^!\s*$', term_getline(g:buf, term_getcursor(g:buf)[0] + 1))
+  bwipe!
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab
diff --git a/test/functional/ui/popupmenu_spec.lua b/test/functional/ui/popupmenu_spec.lua
index 8c583c90fe..2689e004c0 100644
--- a/test/functional/ui/popupmenu_spec.lua
+++ b/test/functional/ui/popupmenu_spec.lua
@@ -12,6 +12,9 @@ describe('ui/ext_popupmenu', function()
     screen:set_default_attr_ids({
       [1] = {bold=true, foreground=Screen.colors.Blue},
       [2] = {bold = true},
+      [3] = {reverse = true},
+      [4] = {bold = true, reverse = true},
+      [5] = {bold = true, foreground = Screen.colors.SeaGreen}
     })
   end)
 
@@ -89,3 +92,139 @@ describe('ui/ext_popupmenu', function()
     ]]}
   end)
 end)
+
+describe('popup placement', function()
+  local screen
+  before_each(function()
+    clear()
+    screen = Screen.new(32, 20)
+    screen:attach()
+    screen:set_default_attr_ids({
+      -- popup selected item / scrollbar track
+      ['s'] = {background = Screen.colors.WebGray},
+      -- popup non-selected item
+      ['n'] = {background = Screen.colors.LightMagenta},
+      -- popup scrollbar knob
+      ['c'] = {background = Screen.colors.Grey0},
+      [1] = {bold = true, foreground = Screen.colors.Blue},
+      [2] = {bold = true},
+      [3] = {reverse = true},
+      [4] = {bold = true, reverse = true},
+      [5] = {bold = true, foreground = Screen.colors.SeaGreen}
+    })
+  end)
+
+  it('works with preview-window above', function()
+    feed(':ped<CR><c-w>4+')
+    feed('iaa bb cc dd ee ff gg hh ii jj<cr>')
+    feed('<c-x><c-n>')
+    screen:expect([[
+      aa bb cc dd ee ff gg hh ii jj   |
+      aa                              |
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {3:[No Name] [Preview][+]          }|
+      aa bb cc dd ee ff gg hh ii jj   |
+      aa^                              |
+      {s:aa             }{c: }{1:                }|
+      {n:bb             }{c: }{1:                }|
+      {n:cc             }{c: }{1:                }|
+      {n:dd             }{c: }{1:                }|
+      {n:ee             }{c: }{1:                }|
+      {n:ff             }{c: }{1:                }|
+      {n:gg             }{s: }{1:                }|
+      {n:hh             }{s: }{4:                }|
+      {2:-- }{5:match 1 of 10}                |
+    ]])
+  end)
+
+  it('works with preview-window below', function()
+    feed(':ped<CR><c-w>4+<c-w>r')
+    feed('iaa bb cc dd ee ff gg hh ii jj<cr>')
+    feed('<c-x><c-n>')
+    screen:expect([[
+      aa bb cc dd ee ff gg hh ii jj   |
+      aa^                              |
+      {s:aa             }{c: }{1:                }|
+      {n:bb             }{c: }{1:                }|
+      {n:cc             }{c: }{1:                }|
+      {n:dd             }{c: }{1:                }|
+      {n:ee             }{c: }{1:                }|
+      {n:ff             }{c: }{1:                }|
+      {n:gg             }{s: }{1:                }|
+      {n:hh             }{s: }{4:                }|
+      aa bb cc dd ee ff gg hh ii jj   |
+      aa                              |
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {1:~                               }|
+      {3:[No Name] [Preview][+]          }|
+      {2:-- }{5:match 1 of 10}                |
+      ]])
+  end)
+
+  it('works with preview-window above and inverted', function()
+    feed(':ped<CR><c-w>4+')
+    feed('iaa<cr>bb<cr>cc<cr>dd<cr>ee<cr>')
+    feed('ff<cr>gg<cr>hh<cr>ii<cr>jj<cr>')
+    feed('<c-x><c-n>')
+    screen:expect([[
+      aa                              |
+      bb                              |
+      cc                              |
+      dd                              |
+      ee                              |
+      ff                              |
+      gg                              |
+      hh                              |
+      {3:[No Name] [Preview][+]          }|
+      cc                              |
+      dd                              |
+      ee                              |
+      ff                              |
+      gg                              |
+      hh                              |
+      {s:aa             }{c: }                |
+      {n:bb             }{s: }                |
+      aa^                              |
+      {4:[No Name] [+]                   }|
+      {2:-- }{5:match 1 of 10}                |
+    ]])
+  end)
+
+  it('works with preview-window below and inverted', function()
+    feed(':ped<CR><c-w>4+<c-w>r')
+    feed('iaa<cr>bb<cr>cc<cr>dd<cr>ee<cr>')
+    feed('ff<cr>gg<cr>hh<cr>ii<cr>jj<cr>')
+    feed('<c-x><c-n>')
+    screen:expect([[
+      {s:aa             }{c: }                |
+      {n:bb             }{c: }                |
+      {n:cc             }{c: }                |
+      {n:dd             }{c: }                |
+      {n:ee             }{c: }                |
+      {n:ff             }{c: }                |
+      {n:gg             }{s: }                |
+      {n:hh             }{s: }                |
+      aa^                              |
+      {4:[No Name] [+]                   }|
+      aa                              |
+      bb                              |
+      cc                              |
+      dd                              |
+      ee                              |
+      ff                              |
+      gg                              |
+      hh                              |
+      {3:[No Name] [Preview][+]          }|
+      {2:-- }{5:match 1 of 10}                |
+    ]])
+  end)
+end)