patch 9.1.0863: getcellpixels() can be further improved

Problem:  getcellpixels() can be further improved
Solution: improve it further, add more tests
          (mikoto2000)

closes: #16047

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

runtime/doc/builtin.txt

@@ -1,4 +1,4 @@
-*builtin.txt*	For Vim version 9.1.  Last change: 2024 Nov 11
+*builtin.txt*	For Vim version 9.1.  Last change: 2024 Nov 14
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -3790,10 +3790,10 @@ getbufvar({buf}, {varname} [, {def}])				*getbufvar()*
 getcellpixels()						*getcellpixels()*
 		Returns a |List| of terminal cell pixel size.
 		List format is [xpixels, ypixels].
-		Only works on Unix. For gVim and on other systems,
+		Only works on (terminal) Unix.  For gVim, on other systems and
-		returns [-1, -1].
+		on failure returns [].
 
-		Return type: list<Number>
+		Return type: list<any>
 
 
 getcellwidths()						*getcellwidths()*

src/os_unix.c

@@ -4358,6 +4358,17 @@ f_getcellpixels(typval_T *argvars UNUSED, typval_T *rettv)
     if (rettv_list_alloc(rettv) == FAIL)
         return;
 
+    // failed get pixel size.
+    if (cs.cs_xpixel == -1)
+        return;
+
+#if defined(FEAT_GUI)
+    // gui return [].
+    if (gui.in_use)
+        return;
+#endif
+
+    // success pixel size and no gui.
     list_append_number(rettv->vval.v_list, (varnumber_T)cs.cs_xpixel);
     list_append_number(rettv->vval.v_list, (varnumber_T)cs.cs_ypixel);
 }
@@ -4365,48 +4376,38 @@ f_getcellpixels(typval_T *argvars UNUSED, typval_T *rettv)
 
 /*
  * Try to get the current terminal cell size.
- * If faile get cell size, fallback 5x10 pixel.
+ * On failure, returns -1x-1
  */
     void
 mch_calc_cell_size(struct cellsize *cs_out)
 {
-#if defined(FEAT_GUI)
+   // get current tty size.
-    if (!gui.in_use)
+   struct winsize ws;
-    {
+   int fd = 1;
+   int retval = -1;
+   retval = ioctl(fd, TIOCGWINSZ, &ws);
+
+#ifdef FEAT_EVAL
+   ch_log(NULL, "ioctl(TIOCGWINSZ) %s", retval == 0 ? "success" : "failed");
 #endif
-        // get current tty size.
-        struct winsize ws;
-        int fd = 1;
-        int retval = -1;
-        retval = ioctl(fd, TIOCGWINSZ, &ws);
-#  ifdef FEAT_EVAL
-        ch_log(NULL, "ioctl(TIOCGWINSZ) %s", retval == 0 ? "success" : "failed");
-#  endif
-        if (retval == -1)
-        {
-            cs_out->cs_xpixel = -1;
-            cs_out->cs_ypixel = -1;
-            return;
-        }
 
-        // calculate parent tty's pixel per cell.
+   if (retval == -1)
-        int x_cell_size = ws.ws_xpixel / ws.ws_col;
+   {
-        int y_cell_size = ws.ws_ypixel / ws.ws_row;
+       cs_out->cs_xpixel = -1;
+       cs_out->cs_ypixel = -1;
+       return;
+   }
 
-        // calculate current tty's pixel
+   // calculate parent tty's pixel per cell.
-        cs_out->cs_xpixel = x_cell_size;
+   int x_cell_size = ws.ws_xpixel / ws.ws_col;
-        cs_out->cs_ypixel = y_cell_size;
+   int y_cell_size = ws.ws_ypixel / ws.ws_row;
 
-#  ifdef FEAT_EVAL
+   // calculate current tty's pixel
-        ch_log(NULL, "Got cell pixel size with TIOCGWINSZ: %d x %d", x_cell_size, y_cell_size);
+   cs_out->cs_xpixel = x_cell_size;
-#  endif
+   cs_out->cs_ypixel = y_cell_size;
-#if defined(FEAT_GUI)
+
-    }
+#ifdef FEAT_EVAL
-    else
+   ch_log(NULL, "Got cell pixel size with TIOCGWINSZ: %d x %d", x_cell_size, y_cell_size);
-    {
-        cs_out->cs_xpixel = -1;
-        cs_out->cs_ypixel = -1;
-    }
 #endif
 }
 
@@ -4433,8 +4434,18 @@ mch_report_winsize(int fd, int rows, int cols)
     // calcurate and set tty pixel size
     struct cellsize cs;
     mch_calc_cell_size(&cs);
-    ws.ws_xpixel = cols * cs.cs_xpixel;
+
-    ws.ws_ypixel = rows * cs.cs_ypixel;
+    if (cs.cs_xpixel == -1)
+    {
+        // failed get pixel size.
+        ws.ws_xpixel = 0;
+        ws.ws_ypixel = 0;
+    }
+    else
+    {
+        ws.ws_xpixel = cols * cs.cs_xpixel;
+        ws.ws_ypixel = rows * cs.cs_ypixel;
+    }
 
     retval = ioctl(tty_fd, TIOCSWINSZ, &ws);
     ch_log(NULL, "ioctl(TIOCSWINSZ) %s", retval == 0 ? "success" : "failed");

src/os_unix.h

@@ -489,8 +489,9 @@ int mch_rename(const char *src, const char *dest);
 // We have three kinds of ACL support.
 #define HAVE_ACL (HAVE_POSIX_ACL || HAVE_SOLARIS_ACL || HAVE_AIX_ACL)
 
+// Defined as signed, to return -1 on error
 struct cellsize {
-    unsigned int cs_xpixel;
+    int cs_xpixel;
-    unsigned int cs_ypixel;
+    int cs_ypixel;
 };
 

src/testdir/test_functions.vim

@@ -4159,4 +4159,35 @@ func Test_slice()
   call assert_equal(0, slice(v:true, 1))
 endfunc
 
+
+" Test for getcellpixels()
+" Pixel size of a cell is terminal-dependent, so in the test, only the list and size 2 are checked.
+func Test_getcellpixels()
+  " Not yet Windows-compatible
+  CheckNotMSWindows
+  CheckRunVimInTerminal
+
+  let buf = RunVimInTerminal('', #{rows: 6})
+
+  " write getcellpixels() result to current buffer.
+  call term_sendkeys(buf, ":redi @\"\<CR>")
+  call term_sendkeys(buf, ":echo getcellpixels()\<CR>")
+  call term_sendkeys(buf, ":redi END\<CR>")
+  call term_sendkeys(buf, "P")
+
+  call WaitForAssert({-> assert_match("\[\d+, \d+\]", term_getline(buf, 3))}, 1000)
+
+  call StopVimInTerminal(buf)
+endfunc
+
+" Test for getcellpixels() on gVim
+func Test_getcellpixels_gui()
+  " Not yet Windows-compatible
+  CheckNotMSWindows
+  if has("gui_running")
+    let cellpixels = getcellpixels()
+    call assert_equal(0, len(cellpixels))
+  endif
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

src/testdir/test_utf8.vim

@@ -273,14 +273,6 @@ func Test_setcellwidths()
   bwipe!
 endfunc
 
-" Pixel size of a cell is terminal-dependent, so in the test, only the list and size 2 are checked.
-func Test_getcellpixels()
-  " Not yet Windows-compatible
-  CheckNotMSWindows
-  let cellpixels = getcellpixels()
-  call assert_equal(2, len(cellpixels))
-endfunc
-
 func Test_getcellwidths()
   call setcellwidths([])
   call assert_equal([], getcellwidths())

src/version.c

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