bug#55696: 28.1; eshell fails to respect text-scale-increase

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 343 bytes --]

On 6/8/2022 1:07 AM, martin rudalics wrote:
>  > Fixed (and in the other function, too).
> 
> Looks good to me.  In window.h writing
> 
> enum window_body_unit
>    {
>      WINDOW_BODY_IN_CANONICAL_CHARS,
>      WINDOW_BODY_IN_PIXELS,
>      WINDOW_BODY_IN_REMAPPED_CHARS
>    };
> 
> might prevent future ambiguities.

Ok, done.

[-- Attachment #2: 0001-Account-for-remapped-faces-in-COLUMNS-and-LINES-in-E.patch --]
[-- Type: text/plain, Size: 20280 bytes --]

From 2356f59164607b17c8285a8ec7429a39231febab Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 2 Jun 2022 21:12:04 -0700
Subject: [PATCH] Account for remapped faces in $COLUMNS and $LINES in Eshell

* src/window.h (window_body_unit): New enum...
(window_body_width): ... use it.

* src/window.c (window_body_unit_from_symbol): New function.
(window_body_height, window_body_width): Make PIXELWISE a
'window_body_unit'.
(window-body-height, window-body-width): Accept 'remap' for PIXELWISE.
(window-lines-pixel-dimensions, window_change_record_windows)
(run_window_change_functions, resize_frame_windows, grow_mini_window)
(shrink_mini_window, scroll-left, scroll-right): Update calls to
'window_body_height' and 'window_body_width'.

* src/indent.c (compute_motion): Update calls to 'window_body_width'.

* lisp/eshell/em-ls.el (eshell-ls-find-column-widths)
(eshell-ls-find-column-lengths): Use 'window-body-width'.

* lisp/eshell/esh-var.el (eshell-variable-aliases-list): Use
'window-body-width' and 'window-body-height'.

* test/lisp/eshell/esh-var-tests.el (esh-var-test/window-height)
(esh-var-test/window-width): Rename to...
(esh-var-test/lines-var, esh-var-test/columns-var): ... and update
expected value.

* doc/lispref/windows.texi (Window Sizes): Document new behavior of
PIXELWISE argument for 'window-body-width' and 'window-body-height'.

* etc/NEWS: Announce this change (bug#55696).
---
 doc/lispref/windows.texi          |  38 +++++---
 etc/NEWS                          |   6 ++
 lisp/eshell/em-ls.el              |   4 +-
 lisp/eshell/esh-var.el            |   4 +-
 src/indent.c                      |   4 +-
 src/window.c                      | 150 +++++++++++++++++++++---------
 src/window.h                      |   8 +-
 test/lisp/eshell/esh-var-tests.el |  16 ++--
 8 files changed, 156 insertions(+), 74 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 0d285b2ad4..704ed30366 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -829,14 +829,18 @@ Window Sizes
 @var{window}.  If @var{window} is omitted or @code{nil}, it defaults to
 the selected window; otherwise it must be a live window.
 
-If the optional argument @var{pixelwise} is non-@code{nil}, this
-function returns the body height of @var{window} counted in pixels.
+The optional argument @var{pixelwise} defines the units to use for the
+height.  If @code{nil}, return the body height of @var{window} in
+characters, rounded down to the nearest integer, if necessary.  This
+means that if a line at the bottom of the text area is only partially
+visible, that line is not counted.  It also means that the height of a
+window's body can never exceed its total height as returned by
+@code{window-total-height}.
 
-If @var{pixelwise} is @code{nil}, the return value is rounded down to
-the nearest integer, if necessary.  This means that if a line at the
-bottom of the text area is only partially visible, that line is not
-counted.  It also means that the height of a window's body can never
-exceed its total height as returned by @code{window-total-height}.
+If @var{pixelwise} is @code{remap} and the default face is remapped
+(@pxref{Face Remapping}), use the remapped face to determine the
+character height.  For any other non-@code{nil} value, return the
+height in pixels.
 @end defun
 
 @cindex window body width
@@ -857,14 +861,18 @@ Window Sizes
 @var{window}.  If @var{window} is omitted or @code{nil}, it defaults to
 the selected window; otherwise it must be a live window.
 
-If the optional argument @var{pixelwise} is non-@code{nil}, this
-function returns the body width of @var{window} in units of pixels.
-
-If @var{pixelwise} is @code{nil}, the return value is rounded down to
-the nearest integer, if necessary.  This means that if a column on the
-right of the text area is only partially visible, that column is not
-counted.  It also means that the width of a window's body can never
-exceed its total width as returned by @code{window-total-width}.
+The optional argument @var{pixelwise} defines the units to use for the
+width.  If @code{nil}, return the body width of @var{window} in
+characters, rounded down to the nearest integer, if necessary.  This
+means that if a column on the right of the text area is only partially
+visible, that column is not counted.  It also means that the width of
+a window's body can never exceed its total width as returned by
+@code{window-total-width}.
+
+If @var{pixelwise} is @code{remap} and the default face is remapped
+(@pxref{Face Remapping}), use the remapped face to determine the
+character width.  For any other non-@code{nil} value, return the width
+in pixels.
 @end defun
 
 @cindex window body size
diff --git a/etc/NEWS b/etc/NEWS
index 3870e937df..79f99dd822 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2398,6 +2398,12 @@ dimensions.
 Specifying a cons as the FROM argument allows to start measuring text
 from a specified amount of pixels above or below a position.
 
++++
+** 'window-body-width' and 'window-body-height' can use remapped faces.
+Specifying 'remap' as the PIXELWISE argument now checks if the default
+face was remapped, and if so, uses the remapped face to determine the
+character width/height.
+
 +++
 ** 'set-window-vscroll' now accepts a new argument PRESERVE-VSCROLL-P.
 This means the vscroll will not be reset when set on a window that is
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 874591d250..bebb0d81b5 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -800,7 +800,7 @@ eshell-ls-find-column-widths
              (+ 2 (length (car file))))
 	   files))
 	 ;; must account for the added space...
-	 (max-width (+ (window-width) 2))
+	 (max-width (+ (window-body-width nil 'remap) 2))
 	 (best-width 0)
 	 col-widths)
 
@@ -845,7 +845,7 @@ eshell-ls-find-column-lengths
            (lambda (file)
              (+ 2 (length (car file))))
 	   files))
-	 (max-width (+ (window-width) 2))
+	 (max-width (+ (window-body-width nil 'remap) 2))
 	 col-widths
 	 colw)
 
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 186f6358bc..a54bee1a61 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -149,8 +149,8 @@ eshell-variable-name-regexp
 
 (defcustom eshell-variable-aliases-list
   `(;; for eshell.el
-    ("COLUMNS" ,(lambda (_indices) (window-width)) t)
-    ("LINES" ,(lambda (_indices) (window-height)) t)
+    ("COLUMNS" ,(lambda (_indices) (window-body-width nil 'remap)) t)
+    ("LINES" ,(lambda (_indices) (window-body-height nil 'remap)) t)
 
     ;; for eshell-cmd.el
     ("_" ,(lambda (indices)
diff --git a/src/indent.c b/src/indent.c
index acbb9dc972..51f6f414de 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -1204,7 +1204,7 @@ compute_motion (ptrdiff_t from, ptrdiff_t frombyte, EMACS_INT fromvpos,
   /* Negative width means use all available text columns.  */
   if (width < 0)
     {
-      width = window_body_width (win, 0);
+      width = window_body_width (win, WINDOW_BODY_IN_CANONICAL_CHARS);
       /* We must make room for continuation marks if we don't have fringes.  */
 #ifdef HAVE_WINDOW_SYSTEM
       if (!FRAME_WINDOW_P (XFRAME (win->frame)))
@@ -1814,7 +1814,7 @@ DEFUN ("compute-motion", Fcompute_motion, Scompute_motion, 7, 7, 0,
 			 ? window_internal_height (w)
 			 : XFIXNUM (XCDR (topos))),
 			(NILP (topos)
-			 ? (window_body_width (w, 0)
+			 ? (window_body_width (w, WINDOW_BODY_IN_CANONICAL_CHARS)
 			    - (
 #ifdef HAVE_WINDOW_SYSTEM
 			       FRAME_WINDOW_P (XFRAME (w->frame)) ? 0 :
diff --git a/src/window.c b/src/window.c
index eba1390fed..e5666ce38e 100644
--- a/src/window.c
+++ b/src/window.c
@@ -1014,11 +1014,20 @@ DEFUN ("window-top-line", Fwindow_top_line, Swindow_top_line, 0, 1, 0,
   return make_fixnum (decode_valid_window (window)->top_line);
 }
 
+static enum window_body_unit
+window_body_unit_from_symbol (Lisp_Object unit)
+{
+  return
+    (unit == Qremap ? WINDOW_BODY_IN_REMAPPED_CHARS
+     : NILP (unit) ? WINDOW_BODY_IN_CANONICAL_CHARS
+     : WINDOW_BODY_IN_PIXELS);
+}
+
 /* Return the number of lines/pixels of W's body.  Don't count any mode
    or header line or horizontal divider of W.  Rounds down to nearest
    integer when not working pixelwise. */
 static int
-window_body_height (struct window *w, bool pixelwise)
+window_body_height (struct window *w, enum window_body_unit pixelwise)
 {
   int height = (w->pixel_height
 		- WINDOW_TAB_LINE_HEIGHT (w)
@@ -1029,11 +1038,27 @@ window_body_height (struct window *w, bool pixelwise)
 		- WINDOW_MODE_LINE_HEIGHT (w)
 		- WINDOW_BOTTOM_DIVIDER_WIDTH (w));
 
+  int denom = 1;
+  if (pixelwise == WINDOW_BODY_IN_REMAPPED_CHARS)
+    {
+      if (!NILP (Vface_remapping_alist))
+	{
+	  struct frame *f = XFRAME (WINDOW_FRAME (w));
+	  int face_id = lookup_named_face (NULL, f, Qdefault, true);
+	  struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);
+	  if (face && face->font && face->font->height)
+	    denom = face->font->height;
+	}
+      /* For performance, use canonical chars if no face remapping.  */
+      else
+	pixelwise = WINDOW_BODY_IN_CANONICAL_CHARS;
+    }
+
+  if (pixelwise == WINDOW_BODY_IN_CANONICAL_CHARS)
+    denom = FRAME_LINE_HEIGHT (WINDOW_XFRAME (w));
+
   /* Don't return a negative value.  */
-  return max (pixelwise
-	      ? height
-	      : height / FRAME_LINE_HEIGHT (WINDOW_XFRAME (w)),
-	      0);
+  return max (height / denom, 0);
 }
 
 /* Return the number of columns/pixels of W's body.  Don't count columns
@@ -1042,7 +1067,7 @@ window_body_height (struct window *w, bool pixelwise)
    fringes either.  Round down to nearest integer when not working
    pixelwise.  */
 int
-window_body_width (struct window *w, bool pixelwise)
+window_body_width (struct window *w, enum window_body_unit pixelwise)
 {
   struct frame *f = XFRAME (WINDOW_FRAME (w));
 
@@ -1059,24 +1084,46 @@ window_body_width (struct window *w, bool pixelwise)
 		   ? WINDOW_FRINGES_WIDTH (w)
 		   : 0));
 
+  int denom = 1;
+  if (pixelwise == WINDOW_BODY_IN_REMAPPED_CHARS)
+    {
+      if (!NILP (Vface_remapping_alist))
+	{
+	  int face_id = lookup_named_face (NULL, f, Qdefault, true);
+	  struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);
+	  if (face && face->font)
+	    {
+	      if (face->font->average_width)
+		denom = face->font->average_width;
+	      else if (face->font->space_width)
+		denom = face->font->space_width;
+	    }
+	}
+      /* For performance, use canonical chars if no face remapping.  */
+      else
+	pixelwise = WINDOW_BODY_IN_CANONICAL_CHARS;
+    }
+
+  if (pixelwise == WINDOW_BODY_IN_CANONICAL_CHARS)
+    denom = FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w));
+
   /* Don't return a negative value.  */
-  return max (pixelwise
-	      ? width
-	      : width / FRAME_COLUMN_WIDTH (WINDOW_XFRAME (w)),
-	      0);
+  return max (width / denom, 0);
 }
 
 DEFUN ("window-body-width", Fwindow_body_width, Swindow_body_width, 0, 2, 0,
        doc: /* Return the width of WINDOW's text area.
-WINDOW must be a live window and defaults to the selected one.  Optional
-argument PIXELWISE non-nil means return the width in pixels.  The return
-value does not include any vertical dividers, fringes or marginal areas,
-or scroll bars.
+WINDOW must be a live window and defaults to the selected one.  The
+return value does not include any vertical dividers, fringes or
+marginal areas, or scroll bars.
 
-If PIXELWISE is nil, return the largest integer smaller than WINDOW's
-pixel width divided by the character width of WINDOW's frame.  This
-means that if a column at the right of the text area is only partially
-visible, that column is not counted.
+The optional argument PIXELWISE defines the units to use for the
+width.  If nil, return the largest integer smaller than WINDOW's pixel
+width in units of the character width of WINDOW's frame.  If PIXELWISE
+is `remap' and the default face is remapped (see
+`face-remapping-alist'), use the remapped face to determine the
+character width.  For any other non-nil value, return the width in
+pixels.
 
 Note that the returned value includes the column reserved for the
 continuation glyph.
@@ -1084,25 +1131,29 @@ DEFUN ("window-body-width", Fwindow_body_width, Swindow_body_width, 0, 2, 0,
 Also see `window-max-chars-per-line'.  */)
   (Lisp_Object window, Lisp_Object pixelwise)
 {
-  return make_fixnum (window_body_width (decode_live_window (window),
-					 !NILP (pixelwise)));
+  return (make_fixnum
+	  (window_body_width (decode_live_window (window),
+			      window_body_unit_from_symbol (pixelwise))));
 }
 
 DEFUN ("window-body-height", Fwindow_body_height, Swindow_body_height, 0, 2, 0,
        doc: /* Return the height of WINDOW's text area.
-WINDOW must be a live window and defaults to the selected one.  Optional
-argument PIXELWISE non-nil means return the height of WINDOW's text area
-in pixels.  The return value does not include the mode line or header
-line or any horizontal divider.
-
-If PIXELWISE is nil, return the largest integer smaller than WINDOW's
-pixel height divided by the character height of WINDOW's frame.  This
-means that if a line at the bottom of the text area is only partially
-visible, that line is not counted.  */)
+WINDOW must be a live window and defaults to the selected one.  The
+return value does not include the mode line or header line or any
+horizontal divider.
+
+The optional argument PIXELWISE defines the units to use for the
+height.  If nil, return the largest integer smaller than WINDOW's
+pixel height in units of the character height of WINDOW's frame.  If
+PIXELWISE is `remap' and the default face is remapped (see
+`face-remapping-alist'), use the remapped face to determine the
+character height.  For any other non-nil value, return the height in
+pixels.  */)
   (Lisp_Object window, Lisp_Object pixelwise)
 {
-  return make_fixnum (window_body_height (decode_live_window (window),
-					  !NILP (pixelwise)));
+  return (make_fixnum
+	  (window_body_height (decode_live_window (window),
+			       window_body_unit_from_symbol (pixelwise))));
 }
 
 DEFUN ("window-old-body-pixel-width",
@@ -2124,7 +2175,8 @@ DEFUN ("window-lines-pixel-dimensions", Fwindow_lines_pixel_dimensions, Swindow_
   struct glyph_row *row, *end_row;
   int max_y = NILP (body) ? WINDOW_PIXEL_HEIGHT (w) : window_text_bottom_y (w);
   Lisp_Object rows = Qnil;
-  int window_width = NILP (body) ? w->pixel_width : window_body_width (w, true);
+  int window_width = NILP (body)
+    ? w->pixel_width : window_body_width (w, WINDOW_BODY_IN_PIXELS);
   int tab_line_height = WINDOW_TAB_LINE_HEIGHT (w);
   int header_line_height = WINDOW_HEADER_LINE_HEIGHT (w);
   int subtract = NILP (body) ? 0 : (tab_line_height + header_line_height);
@@ -3657,8 +3709,10 @@ window_change_record_windows (Lisp_Object window, int stamp, ptrdiff_t number)
 	  wset_old_buffer (w, w->contents);
 	  w->old_pixel_width = w->pixel_width;
 	  w->old_pixel_height = w->pixel_height;
-	  w->old_body_pixel_width = window_body_width (w, true);
-	  w->old_body_pixel_height = window_body_height (w, true);
+	  w->old_body_pixel_width
+	    = window_body_width (w, WINDOW_BODY_IN_PIXELS);
+	  w->old_body_pixel_height
+	    = window_body_height (w, WINDOW_BODY_IN_PIXELS);
 	}
 
       w = NILP (w->next) ? 0 : XWINDOW (w->next);
@@ -3903,8 +3957,10 @@ run_window_change_functions (void)
 	     && (window_buffer_change
 		 || w->pixel_width != w->old_pixel_width
 		 || w->pixel_height != w->old_pixel_height
-		 || window_body_width (w, true) != w->old_body_pixel_width
-		 || window_body_height (w, true) != w->old_body_pixel_height));
+		 || (window_body_width (w, WINDOW_BODY_IN_PIXELS)
+		     != w->old_body_pixel_width)
+		 || (window_body_height (w, WINDOW_BODY_IN_PIXELS)
+		     != w->old_body_pixel_height)));
 
 	  /* The following two are needed when running the default
 	     values for this frame below.  */
@@ -4768,7 +4824,8 @@ resize_frame_windows (struct frame *f, int size, bool horflag)
   Lisp_Object mini = f->minibuffer_window;
   struct window *m = WINDOWP (mini) ? XWINDOW (mini) : NULL;
   int mini_height = ((FRAME_HAS_MINIBUF_P (f) && !FRAME_MINIBUF_ONLY_P (f))
-		     ? unit + m->pixel_height - window_body_height (m, true)
+		     ? (unit + m->pixel_height
+			- window_body_height (m, WINDOW_BODY_IN_PIXELS))
 		     : 0);
 
   new_pixel_size = max (horflag ? size : size - mini_height, unit);
@@ -5255,7 +5312,7 @@ resize_mini_window_apply (struct window *w, int delta)
 grow_mini_window (struct window *w, int delta)
 {
   struct frame *f = XFRAME (w->frame);
-  int old_height = window_body_height (w, true);
+  int old_height = window_body_height (w, WINDOW_BODY_IN_PIXELS);
   int min_height = FRAME_LINE_HEIGHT (f);
 
   eassert (MINI_WINDOW_P (w));
@@ -5289,7 +5346,8 @@ grow_mini_window (struct window *w, int delta)
 shrink_mini_window (struct window *w)
 {
   struct frame *f = XFRAME (w->frame);
-  int delta = window_body_height (w, true) - FRAME_LINE_HEIGHT (f);
+  int delta = (window_body_height (w, WINDOW_BODY_IN_PIXELS)
+	       - FRAME_LINE_HEIGHT (f));
 
   eassert (MINI_WINDOW_P (w));
 
@@ -6356,9 +6414,10 @@ DEFUN ("scroll-left", Fscroll_left, Sscroll_left, 0, 2, "^P\np",
   (register Lisp_Object arg, Lisp_Object set_minimum)
 {
   struct window *w = XWINDOW (selected_window);
-  EMACS_INT requested_arg = (NILP (arg)
-			     ? window_body_width (w, 0) - 2
-			     : XFIXNUM (Fprefix_numeric_value (arg)));
+  EMACS_INT requested_arg =
+    (NILP (arg)
+     ? window_body_width (w, WINDOW_BODY_IN_CANONICAL_CHARS) - 2
+     : XFIXNUM (Fprefix_numeric_value (arg)));
   Lisp_Object result = set_window_hscroll (w, w->hscroll + requested_arg);
 
   if (!NILP (set_minimum))
@@ -6381,9 +6440,10 @@ DEFUN ("scroll-right", Fscroll_right, Sscroll_right, 0, 2, "^P\np",
   (register Lisp_Object arg, Lisp_Object set_minimum)
 {
   struct window *w = XWINDOW (selected_window);
-  EMACS_INT requested_arg = (NILP (arg)
-			     ? window_body_width (w, 0) - 2
-			     : XFIXNUM (Fprefix_numeric_value (arg)));
+  EMACS_INT requested_arg =
+    (NILP (arg)
+     ? window_body_width (w, WINDOW_BODY_IN_CANONICAL_CHARS) - 2
+     : XFIXNUM (Fprefix_numeric_value (arg)));
   Lisp_Object result = set_window_hscroll (w, w->hscroll - requested_arg);
 
   if (!NILP (set_minimum))
diff --git a/src/window.h b/src/window.h
index 7f7de58846..298a80a536 100644
--- a/src/window.h
+++ b/src/window.h
@@ -1186,7 +1186,13 @@ #define CHECK_LIVE_WINDOW(WINDOW)				\
 extern bool window_wants_header_line (struct window *);
 extern bool window_wants_tab_line (struct window *);
 extern int window_internal_height (struct window *);
-extern int window_body_width (struct window *w, bool);
+enum window_body_unit
+  {
+    WINDOW_BODY_IN_CANONICAL_CHARS,
+    WINDOW_BODY_IN_PIXELS,
+    WINDOW_BODY_IN_REMAPPED_CHARS
+  };
+extern int window_body_width (struct window *w, enum window_body_unit);
 enum margin_unit { MARGIN_IN_LINES, MARGIN_IN_PIXELS };
 extern int window_scroll_margin (struct window *, enum margin_unit);
 extern void temp_output_buffer_show (Lisp_Object);
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 4e2a18861e..7b8acfcc7d 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -469,13 +469,15 @@ esh-var-test/quoted-interp-convert-cmd-split-indices
 \f
 ;; Built-in variables
 
-(ert-deftest esh-var-test/window-height ()
-  "$LINES should equal (window-height)"
-  (should (eshell-test-command-result "= $LINES (window-height)")))
-
-(ert-deftest esh-var-test/window-width ()
-  "$COLUMNS should equal (window-width)"
-  (should (eshell-test-command-result "= $COLUMNS (window-width)")))
+(ert-deftest esh-var-test/lines-var ()
+  "$LINES should equal (window-body-height nil 'remap)"
+  (should (equal (eshell-test-command-result "echo $LINES")
+                 (window-body-height nil 'remap))))
+
+(ert-deftest esh-var-test/columns-var ()
+  "$COLUMNS should equal (window-body-width nil 'remap)"
+  (should (equal (eshell-test-command-result "echo $COLUMNS")
+                 (window-body-width nil 'remap))))
 
 (ert-deftest esh-var-test/last-result-var ()
   "Test using the \"last result\" ($$) variable"
-- 
2.25.1