api/ui: simplify popup menu position get/set logic; fix test

src/nvim/api/ui.c

@@ -349,15 +349,15 @@ void nvim_ui_pum_set_height(uint64_t channel_id, Integer height, Error *err)
   ui->pum_nlines = (int)height;
 }
 
-/// Tells Nvim the geometry of the popumenu, to align floating
+/// Tells Nvim the geometry of the popumenu, to align floating windows with an
-/// windows with an external popup menu. Note that this method
+/// external popup menu.
-/// is not to be confused with |nvim_ui_pum_set_height()|, which
+///
-/// sets the number of visible items in the popup menu, while
+/// Note that this method is not to be confused with |nvim_ui_pum_set_height()|,
-/// this function sets the bounding box of the popup menu,
+/// which sets the number of visible items in the popup menu, while this
-/// including visual decorations such as boarders and sliders.
+/// function sets the bounding box of the popup menu, including visual
-/// Floats need not use the same font size, nor be anchored to
+/// decorations such as boarders and sliders. Floats need not use the same font
-/// exact grid corners, so one can set floating-point numbers
+/// size, nor be anchored to exact grid corners, so one can set floating-point
-/// to the popup menu geometry.
+/// numbers to the popup menu geometry.
 ///
 /// @param channel_id
 /// @param width   Popupmenu width.

src/nvim/popupmnu.c

@@ -902,18 +902,6 @@ int pum_get_height(void)
   return pum_height;
 }
 
-/// Gets the internal pum geometry.
-///
-/// @return the internal pum geometry. Ignores UI external pum geometry.
-/// Only valid when pum_visible() returns TRUE!
-void pum_get_internal_pos(int *pwidth, int *pheight, int *prow, int *pcol)
-{
-  *pwidth = pum_width;
-  *pheight = pum_height;
-  *prow = pum_row;
-  *pcol = pum_col;
-}
-
 /// Add size information about the pum to "dict".
 void pum_set_event_info(dict_T *dict)
 {
@@ -921,7 +909,12 @@ void pum_set_event_info(dict_T *dict)
     return;
   }
   double w, h, r, c;
-  ui_pum_get_pos(&w, &h, &r, &c);
+  if (!ui_pum_get_pos(&w, &h, &r, &c)) {
+    w = (double)pum_width;
+    h = (double)pum_height;
+    r = (double)pum_row;
+    c = (double)pum_col;
+  }
   tv_dict_add_float(dict, S_LEN("height"), h);
   tv_dict_add_float(dict, S_LEN("width"), w);
   tv_dict_add_float(dict, S_LEN("row"), r);

src/nvim/ui.c

@@ -235,34 +235,19 @@ int ui_pum_get_height(void)
   return pum_height;
 }
 
-void ui_pum_get_pos(double *pwidth, double *pheight, double *prow, double *pcol)
+bool ui_pum_get_pos(double *pwidth, double *pheight, double *prow, double *pcol)
 {
-  double w = 0.0, h = 0.0, r = 0.0, c = 0.0;
-  bool found = false;
   for (size_t i = 1; i < ui_count; i++) {
     if (!uis[i]->pum_pos) {
       continue;
     }
-    w = uis[i]->pum_width;
+    *pwidth = uis[i]->pum_width;
-    h = uis[i]->pum_height;
+    *pheight = uis[i]->pum_height;
-    r = uis[i]->pum_row;
+    *prow = uis[i]->pum_row;
-    c = uis[i]->pum_col;
+    *pcol = uis[i]->pum_col;
-    found = true;
+    return true;
-    break;
-  }
-  if (found) {
-    *pwidth = w;
-    *pheight = h;
-    *prow = r;
-    *pcol = c;
-  } else {
-    int iw, ih, ir, ic;
-    pum_get_internal_pos(&iw, &ih, &ir, &ic);
-    *pwidth = (double)iw;
-    *pheight = (double)ih;
-    *prow = (double)ir;
-    *pcol = (double)ic;
   }
+  return false;
 }
 
 static void ui_refresh_event(void **argv)

test/functional/ui/popupmenu_spec.lua

@@ -8,7 +8,7 @@ local command = helpers.command
 local funcs = helpers.funcs
 local get_pathsep = helpers.get_pathsep
 local eq = helpers.eq
-local matches = helpers.matches
+local pcall_err = helpers.pcall_err
 
 describe('ui/ext_popupmenu', function()
   local screen
@@ -421,22 +421,16 @@ describe('ui/ext_popupmenu', function()
     end)
 
     it('an error occurs if set 0 or less', function()
-      local ok, err, _
+      meths.ui_pum_set_height(1)
-      ok, _ = pcall(meths.ui_pum_set_height, 1)
+      eq('Expected pum height > 0',
-      eq(true, ok)
+         pcall_err(meths.ui_pum_set_height, 0))
-      ok, err = pcall(meths.ui_pum_set_height, 0)
-      eq(false, ok)
-      matches('.*: Expected pum height > 0', err)
     end)
 
     it('an error occurs when ext_popupmenu is false', function()
-      local ok, err, _
+      meths.ui_pum_set_height(1)
-      ok, _ = pcall(meths.ui_pum_set_height, 1)
-      eq(true, ok)
       screen:set_option('ext_popupmenu', false)
-      ok, err = pcall(meths.ui_pum_set_height, 1)
+      eq('It must support the ext_popupmenu option',
-      eq(false, ok)
+         pcall_err(meths.ui_pum_set_height, 1))
-      matches('.*: It must support the ext_popupmenu option', err)
     end)
   end)
 
@@ -482,35 +476,24 @@ describe('ui/ext_popupmenu', function()
     end)
 
     it('no error occurs if row or col set less than 0', function()
-      local ok, err, _
+      meths.ui_pum_set_bounds(1.0, 1.0, 0.0, 1.5)
-      ok, _ = pcall(meths.ui_pum_set_bounds, 1.0, 1.0, 0.0, 1.5)
+      meths.ui_pum_set_bounds(1.0, 1.0, -1.0, 0.0)
-      eq(true, ok)
+      meths.ui_pum_set_bounds(1.0, 1.0, 0.0, -1.0)
-      ok, _ = pcall(meths.ui_pum_set_bounds, 1.0, 1.0, -1.0, 0.0)
-      eq(true, ok)
-      ok, _ = pcall(meths.ui_pum_set_bounds, 1.0, 1.0, 0.0, -1.0)
-      eq(true, ok)
     end)
 
     it('an error occurs if width or height set 0 or less', function()
-      local ok, err, _
+      meths.ui_pum_set_bounds(1.0, 1.0, 0.0, 1.5)
-      ok, _ = pcall(meths.ui_pum_set_bounds, 1.0, 1.0, 0.0, 1.5)
+      eq('Expected width > 0',
-      eq(true, ok)
+         pcall_err(meths.ui_pum_set_bounds, 0.0, 1.0, 1.0, 0.0))
-      ok, err = pcall(meths.ui_pum_set_bounds, 0.0, 1.0, 1.0, 0.0)
+      eq('Expected height > 0',
-      eq(false, ok)
+         pcall_err(meths.ui_pum_set_bounds, 1.0, -1.0, 1.0, 0.0))
-      matches('.*: Expected pumpos width > 0', err)
-      ok, err = pcall(meths.ui_pum_set_bounds, 1.0, 0.0, 1.0, 0.0)
-      eq(false, ok)
-      matches('.*: Expected pumpos height > 0', err)
     end)
 
     it('an error occurs when ext_popupmenu is false', function()
-      local ok, err, _
+      meths.ui_pum_set_bounds(1.0, 1.0, 0.0, 1.5)
-      ok, _ = pcall(meths.ui_pum_set_bounds, 1.0, 1.0, 0.0, 1.5)
-      eq(true, ok)
       screen:set_option('ext_popupmenu', false)
-      ok, err = pcall(meths.ui_pum_set_bounds, 1.0, 1.0, 0.0, 1.5)
+      eq('UI must support the ext_popupmenu option',
-      eq(false, ok)
+         pcall_err(meths.ui_pum_set_bounds, 1.0, 1.0, 0.0, 1.5))
-      matches('.*: UI must support the ext_popupmenu option', err)
     end)
   end)