Subject: Re: bug#48307: Feature request: provide default keybindings to change the font size in all windows

From: Eli Zaretskii <[email-masked]>
Date: Tue, 25 May 2021 15:31:39 +0300

> Date: Tue, 25 May 2021 07:58:42 +0000
> From: Gregory Heytings <[email-masked]>
> Cc: [email-masked]
> 
> Updated patch attached.

Thanks.

> The main added feature is that face resizing is now bounded (between
> 10 and 500).

Hmm, the GCS frowns on arbitrary limits, so perhaps we should have a
defcustom to control the limits?

> +@kindex C-M-wheel-up
> +  Similarly, to change the height of the default face globally, type
> +@kbd{C-x C-M-+}, @kbd{C-x C-M-=}, @kbd{C-x C-M--} or @kbd{C-x C-M-0},
> +or scroll the mouse wheel with both the @kbd{Ctrl} and @kbd{Meta}
> +modifiers pressed.  To enable frame resizing when the height of the
> +default face is changed globally, customize the variable
> +@code{global-text-scale-adjust-resizes-frames} (@pxref{Easy
> +Customization}).

The last part should explain why frame resizing is related to font
size changes, it isn't obvious.

Also, I wonder whether we should say "height of the default face's
font", because this is what this actually does.  I'm not sure every
user will understand the notion of the height of a face.

And finally, "height" or "size"?

> +(defun global-text-scale-adjust (increment)
> +  "Globally adjust the height of the default face by INCREMENT.
> +
> +INCREMENT may be passed as a numeric prefix argument.

The last sentence should start with "Interactively, ".  Also, the doc
string should tell what are the units of INCREMENT.

> +The adjustment made depends on the final component of the key binding
> +used to invoke the command, with all modifiers removed:

I wonder if we could rephrase this to be less awkward and more clear.

> +After adjusting, further adjust the default face height as long as the
> +key, with all modifiers removed, is one of the above characters.

And this.

> +Buffer-local face adjustements remain in effect when global face
> +adjustments are made.

I don't think I understand what this means, so it would be good to
clarify that.

> +The variable `global-text-scale-adjust-resizes-frames' controls whether
> +the frames are resized when the default face is adjusted.

Suggest to say "... resized to keep the same number of lines and
characters per line ...".

> +           (inc
> +            (pcase key
> +              (?- (* (- increment) 5))
> +              (?0 (- global-text-scale-adjust--default-height cur))
> +              (_ (* increment 5))))

Do we really need pcase here?

> +(defun mouse-wheel-global-text-scale (event)
> +  "Increase or decrease the global height of the default face according to the EVENT."

This sentence is too long to fit on a single line.  Suggest to make it
shorter:

  "Increase or decrease the height of the default face according to EVENT."