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

[Jim Porter]

Thanks for taking a look at the patch. Here's a new version with the 
requested changes.

On 6/5/2022 2:13 AM, Eli Zaretskii wrote:
> I'd prefer to avoid the renaming of the argument, as there's nothing
> wrong with PIXELWISE being not a boolean value.  We have this
> elsewhere; e.g., see line-number-display-width.

Ok, I reverted the name to PIXELWISE (though the enum is still named 
`body_unit').

> > +If @var{unit} is @code{remap} and the default face is remapped
> > +(@pxref{Face Remapping}), use the remapped face to determine the
> > +character height.  For any other value, return the height in pixels.
>                        ^^^^^^^^^^^^^^^^^^^
> "For any other non-@code{nil} value" avoids potential confusion here.

Fixed (in both places).

> > +static enum body_unit
> > +window_body_unit_from_symbol(Lisp_Object unit)
> > +{
> > +  return
> > +    (unit == intern("remap") ? BODY_IN_REMAPPED_CHARS
> > +     : NILP (unit) ? BODY_IN_CANONICAL_CHARS
> > +     : BODY_IN_PIXELS);
>
> Our style in C is to leave one space between the name of a symbol and
> the following left parenthesis.

Fixed.

> I think we should optimize the frequent case where
> Vface_remapping_alist is nil, in which case BODY_IN_REMAPPED_CHARS is
> the same as BODY_IN_CANONICAL_CHARS.  lookup_named_face is not a
> trivial function, so it is best to avoid calling it whenever possible.

Good point. How does the updated implementation look?

> > +The optional argument UNIT defines the units to use for the width.  If
> > +nil, return the largest integer smaller than WINDOW's pixel width
> > +divided by the character width of WINDOW's frame.
>
> I prefer saying "in units of ..." rather than 'divided by ...".  The
> latter is slightly harder to grasp, IME.

Fixed.

> >                                                      This means that if
> > +a column at the right of the text area is only partially visible, that
> > +column is not counted.
>
> I think this sentence doesn't have to be in the doc string; it's
> enough to have this explanation in the manual.  (Please make the same
> change in other similar places.)

Ok, I removed that bit.

0001-Account-for-remapped-faces-in-COLUMNS-and-LINES-in-E.patch
Description: Text document