emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Patch to vertically center line content when using line-spacing vari


From: Eli Zaretskii
Subject: Re: Patch to vertically center line content when using line-spacing variable
Date: Sat, 07 Sep 2019 12:50:04 +0300

> From: Jesse Medeiros <address@hidden>
> Date: Sat, 31 Aug 2019 21:01:44 -0300
> Cc: address@hidden
> 
> > The preferred format for patches is to have a commit message in the
> > form of a ChangeLog entry, and to send it as an attached file produced
> > with e.g. "git format-patch -1".
> 
> Oh, sorry I missed that. I'm sending the patch in the attachment.
> Thanks for the tips.

Thanks, I have a few minor comments:

> @@ -956,6 +961,7 @@ reset_buffer (register struct buffer *b)
>      (b, BVAR (&buffer_defaults, enable_multibyte_characters));
>    bset_cursor_type (b, BVAR (&buffer_defaults, cursor_type));
>    bset_extra_line_spacing (b, BVAR (&buffer_defaults, extra_line_spacing));
> +  bset_line_spacing_vertical_center (b, BVAR (&buffer_defaults, 
> line_spacing_vertical_center));

Please break this long line into two.

> +  DEFVAR_PER_BUFFER ("line-spacing-vertical-center",
> +                  &BVAR (current_buffer, line_spacing_vertical_center), Qnil,
> +                     doc: /* Non-nil will center the line content vertically
                                        ^^^^
"means" instead of "will" follows our style closer.

> +when using `line-spacing' variable.  */);
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -29294,7 +29294,13 @@ gui_produce_glyphs (struct it *it)
>  
>    if (extra_line_spacing > 0)
>      {
> -      it->descent += extra_line_spacing;
> +      if (! BVAR (XBUFFER (it->w->contents), line_spacing_vertical_center)) {
> +        it->descent += extra_line_spacing;
> +      } else {
> +        int spacing = extra_line_spacing / 2;
> +        it->ascent += spacing;
> +        it->descent += spacing;
> +      }

Please use our style of putting braces around blocks:

   if (something)
     {
       do_this;
       do_that;
     }
   else
     {
       do_something_else;
     }

Finally, this change warrants an entry in NEWS, please provide one.
I think we should also mention this variable in the ELisp manual,
where we describe the line-spacing feature.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]