Patch to vertically center line content when using line-spacing variable

Patch to vertically center line content when using line-spacing variable

[Jesse Medeiros]

When you set the line-spacing variable, Emacs only adds margin to the
bottom of the line, which always annoyed me. So I decided to work on
this and wrote a patch to vertically center the line when setting the
line-spacing variable. I created a new variable called
line-spacing-vertical-center, and the centering will only apply when
this variable is set to non-nil. Could this be merged upstream? Here's
the link for the patch:

https://github.com/sollidsnake/emacs/commit/aff5c59d10001e1161884ef3d0725eda86fa3ea0

This is my first attempt to contribute to Emacs, and I'm not very
familiar neither with C nor with Emacs core, so please let me know if
I'm doing something wrong.


Jesse Medeiros

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

[Stefan Kangas]

Jesse Medeiros <jessenzr@gmail.com>:

> Here's the link for the patch:
>
> https://github.com/sollidsnake/emacs/commit/aff5c59d10001e1161884ef3d0725eda86fa3ea0

Hi Jesse,

Thanks, and welcome to Emacs.

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".  If you could re-send your patch in
that format, it would help others to review your changes.

You can find the details in the CONTRIBUTE file in the source
repository.  One tip is also to have a look at the git log to see how
others have formatted their commit messages.

Best regards,
Sefan Kangas

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

[Jesse Medeiros]

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

> 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.

On Sat, Aug 31, 2019 at 7:36 PM Stefan Kangas <stefan@marxist.se> wrote:
>
> Jesse Medeiros <jessenzr@gmail.com>:
>
> > Here's the link for the patch:
> >
> > https://github.com/sollidsnake/emacs/commit/aff5c59d10001e1161884ef3d0725eda86fa3ea0
>
> Hi Jesse,
>
> Thanks, and welcome to Emacs.
>
> 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".  If you could re-send your patch in
> that format, it would help others to review your changes.
>
> You can find the details in the CONTRIBUTE file in the source
> repository.  One tip is also to have a look at the git log to see how
> others have formatted their commit messages.
>
> Best regards,
> Sefan Kangas

[-- Attachment #2: 0001-Center-lines-vertically-with-line-spacing-vertical-c.patch --]
[-- Type: text/x-patch, Size: 4159 bytes --]

From 60532f6087d0f3cc20197da1790d2f5cec0ecbee Mon Sep 17 00:00:00 2001
From: Jesse Nazario <jessenzr@gmail.com>
Date: Sat, 31 Aug 2019 15:35:34 -0300
Subject: [PATCH] Center lines vertically with line-spacing-vertical-center

When using line-spacing, the new variable line-spacing-vertical-center
can be set to non-nil to center the line content vertically.
---
 src/buffer.c | 13 +++++++++++++
 src/buffer.h |  4 ++++
 src/xdisp.c  |  8 +++++++-
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/buffer.c b/src/buffer.c
index 62a3d66c8b..1b9f96d61b 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -219,6 +219,11 @@ bset_extra_line_spacing (struct buffer *b, Lisp_Object val)
   b->extra_line_spacing_ = val;
 }
 static void
+bset_line_spacing_vertical_center (struct buffer *b, Lisp_Object val)
+{
+  b->line_spacing_vertical_center_ = val;
+}
+static void
 bset_file_format (struct buffer *b, Lisp_Object val)
 {
   b->file_format_ = val;
@@ -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));
 
   b->display_error_modiff = 0;
 }
@@ -5190,6 +5196,7 @@ init_buffer_once (void)
   XSETFASTINT (BVAR (&buffer_local_flags, header_line_format), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_type), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, extra_line_spacing), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, line_spacing_vertical_center), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_in_non_selected_windows), idx); ++idx;
 
   /* buffer_local_flags contains no pointers, so it's safe to treat it
@@ -5259,6 +5266,7 @@ init_buffer_once (void)
   bset_bidi_paragraph_separate_re (&buffer_defaults, Qnil);
   bset_cursor_type (&buffer_defaults, Qt);
   bset_extra_line_spacing (&buffer_defaults, Qnil);
+  bset_line_spacing_vertical_center (&buffer_defaults, Qnil);
   bset_cursor_in_non_selected_windows (&buffer_defaults, Qt);
 
   bset_enable_multibyte_characters (&buffer_defaults, Qt);
@@ -6248,6 +6256,11 @@ from (abs POSITION).  If POSITION is positive, point was at the front
 If value is a floating point number, it specifies the spacing relative
 to the default frame line height.  A value of nil means add no extra space.  */);
 
+  DEFVAR_PER_BUFFER ("line-spacing-vertical-center",
+		     &BVAR (current_buffer, line_spacing_vertical_center), Qnil,
+                     doc: /* Non-nil will center the line content vertically
+when using `line-spacing' variable.  */);
+
   DEFVAR_PER_BUFFER ("cursor-in-non-selected-windows",
 		     &BVAR (current_buffer, cursor_in_non_selected_windows), Qnil,
 		     doc: /* Non-nil means show a cursor in non-selected windows.
diff --git a/src/buffer.h b/src/buffer.h
index 2080a6f40b..48cf06d06f 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -740,6 +740,10 @@ #define BVAR(buf, field) ((buf)->field ## _)
      in the display of this buffer.  */
   Lisp_Object extra_line_spacing_;
 
+  /* When non-nil will center the line content vertically. To be used
+     along with `line-spacing'.  */
+  Lisp_Object line_spacing_vertical_center_;
+
   /* Cursor type to display in non-selected windows.
      t means to use hollow box cursor.
      See `cursor-type' for other values.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index 75bc536cb9..0ddabe6e09 100644
--- 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;
+      }
       if (extra_line_spacing > it->max_extra_line_spacing)
 	it->max_extra_line_spacing = extra_line_spacing;
     }
-- 
2.23.0

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

[Jesse Medeiros]

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

I forgot to mention, it's easier to notice the difference when you have the
hl-line-mode enabled.

[-- Attachment #2: Type: text/html, Size: 321 bytes --]

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

[Eli Zaretskii]

> From: Jesse Medeiros <jessenzr@gmail.com>
> Date: Sat, 31 Aug 2019 21:01:44 -0300
> Cc: emacs-devel@gnu.org
> 
> > 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.

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

[Jesse Medeiros]

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

I made these changes and wrote entries in the NEWS and in the manual.
I wasn't sure where I should place it in the NEWS, so I put it at the
end of the 'Changes in Emacs 27.1' section. If there still something
amiss just let me know.

[-- Attachment #2: 0001-Center-lines-vertically-with-line-spacing-vertical-c.patch --]
[-- Type: text/x-patch, Size: 5444 bytes --]

From 873c4f2f778478ef22b91e5000cba72970602d26 Mon Sep 17 00:00:00 2001
From: Jesse Nazario <jessenzr@gmail.com>
Date: Sun, 8 Sep 2019 20:06:15 -0300
Subject: [PATCH] Center lines vertically with line-spacing-vertical-center

When using line-spacing, the new variable line-spacing-vertical-center
can be set to non-nil to center the line content vertically.
---
 doc/lispref/frames.texi |  5 +++++
 etc/NEWS                |  4 ++++
 src/buffer.c            | 14 ++++++++++++++
 src/buffer.h            |  4 ++++
 src/xdisp.c             |  9 ++++++++-
 5 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index 618ea16fcf..75a6725f03 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -1857,6 +1857,11 @@ Layout Parameters
 Additional space to leave below each text line, in pixels (a positive
 integer).  @xref{Line Height}, for more information.
 
+@vindex line-spacing-vertical-center@r{, a frame parameter}
+@item line-spacing-vertical-center
+If non-nil, centers the line content vertically when using
+using the @code{line-spacing} variable.
+
 @vindex no-special-glyphs@r{, a frame parameter}
 @item no-special-glyphs
 If this is non-@code{nil}, it suppresses the display of any truncation
diff --git a/etc/NEWS b/etc/NEWS
index 87666740df..aa61c23252 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -443,6 +443,10 @@ RGB triplets with a single hexadecimal digit per component.
 ---
 ** The toolbar now shows the equivalent key binding in its tooltips.
 
+** New variable line-spacing-vertical-center.
+This variable, if non-nil, centers the line content vertically when
+using the 'line-spacing' variable.
+
 \f
 * Editing Changes in Emacs 27.1
 
diff --git a/src/buffer.c b/src/buffer.c
index 77e8b6bb77..854b3924c0 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -219,6 +219,11 @@ bset_extra_line_spacing (struct buffer *b, Lisp_Object val)
   b->extra_line_spacing_ = val;
 }
 static void
+bset_line_spacing_vertical_center (struct buffer *b, Lisp_Object val)
+{
+  b->line_spacing_vertical_center_ = val;
+}
+static void
 bset_file_format (struct buffer *b, Lisp_Object val)
 {
   b->file_format_ = val;
@@ -962,6 +967,8 @@ 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));
 
   b->display_error_modiff = 0;
 }
@@ -5196,6 +5203,7 @@ init_buffer_once (void)
   XSETFASTINT (BVAR (&buffer_local_flags, header_line_format), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_type), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, extra_line_spacing), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, line_spacing_vertical_center), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_in_non_selected_windows), idx); ++idx;
 
   /* buffer_local_flags contains no pointers, so it's safe to treat it
@@ -5265,6 +5273,7 @@ init_buffer_once (void)
   bset_bidi_paragraph_separate_re (&buffer_defaults, Qnil);
   bset_cursor_type (&buffer_defaults, Qt);
   bset_extra_line_spacing (&buffer_defaults, Qnil);
+  bset_line_spacing_vertical_center (&buffer_defaults, Qnil);
   bset_cursor_in_non_selected_windows (&buffer_defaults, Qt);
 
   bset_enable_multibyte_characters (&buffer_defaults, Qt);
@@ -6254,6 +6263,11 @@ from (abs POSITION).  If POSITION is positive, point was at the front
 If value is a floating point number, it specifies the spacing relative
 to the default frame line height.  A value of nil means add no extra space.  */);
 
+  DEFVAR_PER_BUFFER ("line-spacing-vertical-center",
+		     &BVAR (current_buffer, line_spacing_vertical_center), Qnil,
+                     doc: /* Non-nil means center the line content vertically
+when using `line-spacing' variable.  */);
+
   DEFVAR_PER_BUFFER ("cursor-in-non-selected-windows",
 		     &BVAR (current_buffer, cursor_in_non_selected_windows), Qnil,
 		     doc: /* Non-nil means show a cursor in non-selected windows.
diff --git a/src/buffer.h b/src/buffer.h
index 82d9350bfc..e4a6b8d4d7 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -549,6 +549,10 @@ #define BVAR(buf, field) ((buf)->field ## _)
      in the display of this buffer.  */
   Lisp_Object extra_line_spacing_;
 
+  /* Non-nil means center the line content vertically. To be used
+     along with `line-spacing'.  */
+  Lisp_Object line_spacing_vertical_center_;
+
   /* Cursor type to display in non-selected windows.
      t means to use hollow box cursor.
      See `cursor-type' for other values.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index 94f969f37c..bdd6d65994 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -29295,7 +29295,14 @@ 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;
+        }
       if (extra_line_spacing > it->max_extra_line_spacing)
 	it->max_extra_line_spacing = extra_line_spacing;
     }
-- 
2.23.0

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

[martin rudalics]

+If non-nil, centers the line content vertically when using
+using the @code{line-spacing} variable.

"using using"

martin

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

[Robert Pluim]

>>>>> On Sun, 8 Sep 2019 20:24:50 -0300, Jesse Medeiros <jessenzr@gmail.com> said:

    Jesse> When using line-spacing, the new variable line-spacing-vertical-center
    Jesse> can be set to non-nil to center the line content vertically.
    Jesse> ---
    Jesse>  doc/lispref/frames.texi |  5 +++++
    Jesse>  etc/NEWS                |  4 ++++
    Jesse>  src/buffer.c            | 14 ++++++++++++++
    Jesse>  src/buffer.h            |  4 ++++
    Jesse>  src/xdisp.c             |  9 ++++++++-
    Jesse>  5 files changed, 35 insertions(+), 1 deletion(-)

Hmm, should this be customizable, like 'line-spacing' is? In any case,
I think the doc string of 'line-spacing' should refer to this new
variable as well.

Also: could this not work as a new type of value for 'line-spacing'
instead? Something like

'(both . 5)

to mean 5 pixels above and below?

    Jesse> diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
    Jesse> index 618ea16fcf..75a6725f03 100644
    Jesse> --- a/doc/lispref/frames.texi
    Jesse> +++ b/doc/lispref/frames.texi
    Jesse> @@ -1857,6 +1857,11 @@ Layout Parameters
    Jesse>  Additional space to leave below each text line, in pixels (a positive
    Jesse>  integer).  @xref{Line Height}, for more information.
 
    Jesse> +@vindex line-spacing-vertical-center@r{, a frame parameter}
    Jesse> +@item line-spacing-vertical-center
    Jesse> +If non-nil, centers the line content vertically when using
    Jesse> +using the @code{line-spacing} variable.
    Jesse> +

I donʼt think you implemented this as a frame parameter, which means the
documentation should go in "@node Line Height" in display.texi, or you
could implement the frame parameter as well. [1]

    Jesse>    if (extra_line_spacing > 0)
    Jesse>      {
    Jesse> -      it->descent += extra_line_spacing;
    Jesse> +      if (! BVAR (XBUFFER (it->w->contents), line_spacing_vertical_center))

I think this currently works because Qnil == 0, for now, but you should
probably use NILP.

Footnotes:
[1] line-spacing can be a frame parameter, per-buffer, or a newline overlay
     property. Implementing all three is probably overkill.

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

[Jesse Medeiros]

> Hmm, should this be customizable, like 'line-spacing' is? In any case,
> I think the doc string of 'line-spacing' should refer to this new
> variable as well.

Yes, it's supposed to be customizable like line-spacing. Personally I
think center vertically should be the default behaviour, but I created
this new variable so it won't change with default to nil the setup of
other people.

> Also: could this not work as a new type of value for 'line-spacing'
> instead? Something like
>
> '(both . 5)
>
> to mean 5 pixels above and below?

I guess it could, but does that make sense? I think one would expect
the line to be centered by default. That's how it works on other
editors.


> I donʼt think you implemented this as a frame parameter, which means the
> documentation should go in "@node Line Height" in display.texi, or you
> could implement the frame parameter as well. [1]

My bad. I searched for currencies of line-spacing at the docs and I
though I had found the right place. I'll change this to display.texi.

>     Jesse>    if (extra_line_spacing > 0)
>     Jesse>      {
>     Jesse> -      it->descent += extra_line_spacing;
>     Jesse> +      if (! BVAR (XBUFFER (it->w->contents), line_spacing_vertical_center))
>
> I think this currently works because Qnil == 0, for now, but you should
> probably use NILP.

Good catch, thanks.


PS: Sorry for the late reply, it's been a busy week.

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

[Robert Pluim]

>>>>> On Thu, 12 Sep 2019 09:08:38 -0300, Jesse Medeiros <jessenzr@gmail.com> said:

    >> Hmm, should this be customizable, like 'line-spacing' is? In any case,
    >> I think the doc string of 'line-spacing' should refer to this new
    >> variable as well.

    Jesse> Yes, it's supposed to be customizable like line-spacing. Personally I
    Jesse> think center vertically should be the default behaviour, but I created
    Jesse> this new variable so it won't change with default to nil the setup of
    Jesse> other people.

In that case you need to add the appropriate code to the list of
c-defined lisp variables in cus-start.el.

I donʼt think you can change the default behaviour.

    >> Also: could this not work as a new type of value for 'line-spacing'
    >> instead? Something like
    >> 
    >> '(both . 5)
    >> 
    >> to mean 5 pixels above and below?

    Jesse> I guess it could, but does that make sense? I think one would expect
    Jesse> the line to be centered by default. That's how it works on other
    Jesse> editors.

Perhaps, but backwards compatibility wins out here, I think. Anyway,
what you have now works.

    Jesse> PS: Sorry for the late reply, it's been a busy week.

Weʼre all just volunteers here.

Robert

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

[Jesse Medeiros]

> In that case you need to add the appropriate code to the list of
> c-defined lisp variables in cus-start.el.
>
> I donʼt think you can change the default behaviour.

I didn't know about cus-start.el, I thought setting it in buffer.c
with Qnil was enough. I will work on theses issues you mentioned and
resend the patch. Thanks!

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

[Robert Pluim]

>>>>> On Thu, 12 Sep 2019 17:49:29 -0300, Jesse Medeiros <jessenzr@gmail.com> said:

    >> In that case you need to add the appropriate code to the list of
    >> c-defined lisp variables in cus-start.el.
    >> 
    >> I donʼt think you can change the default behaviour.

    Jesse> I didn't know about cus-start.el, I thought setting it in buffer.c
    Jesse> with Qnil was enough. I will work on theses issues you mentioned and
    Jesse> resend the patch. Thanks!

(info "(elisp)Writing Emacs Primitives") has more detail than you care
to know about all of this.

Robert

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

[Jesse Medeiros]

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

>     Jesse> diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
>     Jesse> index 618ea16fcf..75a6725f03 100644
>     Jesse> --- a/doc/lispref/frames.texi
>     Jesse> +++ b/doc/lispref/frames.texi
>     Jesse> @@ -1857,6 +1857,11 @@ Layout Parameters
>     Jesse>  Additional space to leave below each text line, in pixels (a positive
>     Jesse>  integer).  @xref{Line Height}, for more information.
>
>     Jesse> +@vindex line-spacing-vertical-center@r{, a frame parameter}
>     Jesse> +@item line-spacing-vertical-center
>     Jesse> +If non-nil, centers the line content vertically when using
>     Jesse> +using the @code{line-spacing} variable.
>     Jesse> +
>
> I donʼt think you implemented this as a frame parameter, which means the
> documentation should go in "@node Line Height" in display.texi, or you
> could implement the frame parameter as well. [1]
>
>     Jesse>    if (extra_line_spacing > 0)
>     Jesse>      {
>     Jesse> -      it->descent += extra_line_spacing;
>     Jesse> +      if (! BVAR (XBUFFER (it->w->contents), line_spacing_vertical_center))
>
> I think this currently works because Qnil == 0, for now, but you should
> probably use NILP.

I've worked the on these issues you raised. Hopefully it's all ok now.

[-- Attachment #2: 0001-Center-lines-vertically-with-line-spacing-vertical-c.patch --]
[-- Type: text/x-patch, Size: 5522 bytes --]

From 9bc5cbb727e5dcac9f774028858e2608d1a23033 Mon Sep 17 00:00:00 2001
From: Jesse Nazario <jessenzr@gmail.com>
Date: Sun, 8 Sep 2019 20:06:15 -0300
Subject: [PATCH] Center lines vertically with line-spacing-vertical-center

When using line-spacing, the new variable line-spacing-vertical-center
can be set to non-nil to center the line content vertically.
---
 doc/lispref/display.texi |  5 +++++
 etc/NEWS                 |  4 ++++
 src/buffer.c             | 14 ++++++++++++++
 src/buffer.h             |  4 ++++
 src/xdisp.c              | 10 +++++++++-
 5 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index fd6820897f..875c50c6ef 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2195,6 +2195,11 @@ Line Height
 number of pixels put below lines.  A floating-point number specifies
 the spacing relative to the frame's default line height.
 
+  The @code{line-spacing} parameter creates the space by putting some
+margin only below the line. If you wish to center the line vertically
+instead, you can set the variable @code{line-spacing-vertical-center}
+as non-nin.
+
 @vindex line-spacing
   You can specify the line spacing for all lines in a buffer via the
 buffer-local @code{line-spacing} variable.  An integer specifies
diff --git a/etc/NEWS b/etc/NEWS
index cb8b6fcac1..3209f2535f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -445,6 +445,10 @@ RGB triplets with a single hexadecimal digit per component.
 ---
 ** The toolbar now shows the equivalent key binding in its tooltips.
 
+** New variable line-spacing-vertical-center.
+This variable, if non-nil, centers the line content vertically when
+using the 'line-spacing' variable.
+
 \f
 * Editing Changes in Emacs 27.1
 
diff --git a/src/buffer.c b/src/buffer.c
index 77e8b6bb77..854b3924c0 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -219,6 +219,11 @@ bset_extra_line_spacing (struct buffer *b, Lisp_Object val)
   b->extra_line_spacing_ = val;
 }
 static void
+bset_line_spacing_vertical_center (struct buffer *b, Lisp_Object val)
+{
+  b->line_spacing_vertical_center_ = val;
+}
+static void
 bset_file_format (struct buffer *b, Lisp_Object val)
 {
   b->file_format_ = val;
@@ -962,6 +967,8 @@ 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));
 
   b->display_error_modiff = 0;
 }
@@ -5196,6 +5203,7 @@ init_buffer_once (void)
   XSETFASTINT (BVAR (&buffer_local_flags, header_line_format), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_type), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, extra_line_spacing), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, line_spacing_vertical_center), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_in_non_selected_windows), idx); ++idx;
 
   /* buffer_local_flags contains no pointers, so it's safe to treat it
@@ -5265,6 +5273,7 @@ init_buffer_once (void)
   bset_bidi_paragraph_separate_re (&buffer_defaults, Qnil);
   bset_cursor_type (&buffer_defaults, Qt);
   bset_extra_line_spacing (&buffer_defaults, Qnil);
+  bset_line_spacing_vertical_center (&buffer_defaults, Qnil);
   bset_cursor_in_non_selected_windows (&buffer_defaults, Qt);
 
   bset_enable_multibyte_characters (&buffer_defaults, Qt);
@@ -6254,6 +6263,11 @@ from (abs POSITION).  If POSITION is positive, point was at the front
 If value is a floating point number, it specifies the spacing relative
 to the default frame line height.  A value of nil means add no extra space.  */);
 
+  DEFVAR_PER_BUFFER ("line-spacing-vertical-center",
+		     &BVAR (current_buffer, line_spacing_vertical_center), Qnil,
+                     doc: /* Non-nil means center the line content vertically
+when using `line-spacing' variable.  */);
+
   DEFVAR_PER_BUFFER ("cursor-in-non-selected-windows",
 		     &BVAR (current_buffer, cursor_in_non_selected_windows), Qnil,
 		     doc: /* Non-nil means show a cursor in non-selected windows.
diff --git a/src/buffer.h b/src/buffer.h
index 280d4e9098..d7cb27305e 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -549,6 +549,10 @@ #define BVAR(buf, field) ((buf)->field ## _)
      in the display of this buffer.  */
   Lisp_Object extra_line_spacing_;
 
+  /* Non-nil means center the line content vertically. To be used
+     along with `line-spacing'.  */
+  Lisp_Object line_spacing_vertical_center_;
+
   /* Cursor type to display in non-selected windows.
      t means to use hollow box cursor.
      See `cursor-type' for other values.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index 95895ec3ac..e2c7309f32 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -29307,7 +29307,15 @@ gui_produce_glyphs (struct it *it)
 
   if (extra_line_spacing > 0)
     {
-      it->descent += extra_line_spacing;
+      if (! NILP (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;
+        }
       if (extra_line_spacing > it->max_extra_line_spacing)
 	it->max_extra_line_spacing = extra_line_spacing;
     }
-- 
2.23.0

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

[Robert Pluim]

>>>>> On Sun, 29 Sep 2019 20:54:01 -0300, Jesse Medeiros <jessenzr@gmail.com> said:

    Jesse> I've worked the on these issues you raised. Hopefully it's all ok now.

Nit-picking below, plus one question about the code.

    Jesse> From 9bc5cbb727e5dcac9f774028858e2608d1a23033 Mon Sep 17 00:00:00 2001
    Jesse> From: Jesse Nazario <jessenzr@gmail.com>
    Jesse> Date: Sun, 8 Sep 2019 20:06:15 -0300
    Jesse> Subject: [PATCH] Center lines vertically with line-spacing-vertical-center

    Jesse> When using line-spacing, the new variable line-spacing-vertical-center
    Jesse> can be set to non-nil to center the line content
    Jesse> vertically.

We use ChangeLog format commit messages, see CONTRIBUTE for
details. One easy way to get the format right is:

C-x v d      ; runs vc-dir
m            ; mark the file(s) youʼre committing
v            ; runs vc-next-action, which pops up a commit buffer,
             ; where you can write the one-line commit summary,
             ; and then
C-c C-w      ; runs log-edit-generate-changelog-from-diff, which scrapes
             ; the file and function names from the diff
             ; fill in the details about the changes

    Jesse> ---
    Jesse>  doc/lispref/display.texi |  5 +++++
    Jesse>  etc/NEWS                 |  4 ++++
    Jesse>  src/buffer.c             | 14 ++++++++++++++
    Jesse>  src/buffer.h             |  4 ++++
    Jesse>  src/xdisp.c              | 10 +++++++++-
    Jesse>  5 files changed, 36 insertions(+), 1 deletion(-)

    Jesse> diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
    Jesse> index fd6820897f..875c50c6ef 100644
    Jesse> --- a/doc/lispref/display.texi
    Jesse> +++ b/doc/lispref/display.texi
    Jesse> @@ -2195,6 +2195,11 @@ Line Height
    Jesse>  number of pixels put below lines.  A floating-point number specifies
    Jesse>  the spacing relative to the frame's default line height.
 
    Jesse> +  The @code{line-spacing} parameter creates the space by putting some
    Jesse> +margin only below the line. If you wish to center the line
    Jesse> vertically

Two spaces after '.'

    Jesse> +instead, you can set the variable @code{line-spacing-vertical-center}
    Jesse> +as non-nin.
    Jesse> +

non-nil

    Jesse> diff --git a/src/xdisp.c b/src/xdisp.c
    Jesse> index 95895ec3ac..e2c7309f32 100644
    Jesse> --- a/src/xdisp.c
    Jesse> +++ b/src/xdisp.c
    Jesse> @@ -29307,7 +29307,15 @@ gui_produce_glyphs (struct it *it)
 
    Jesse>    if (extra_line_spacing > 0)
    Jesse>      {
    Jesse> -      it->descent += extra_line_spacing;
    Jesse> +      if (! NILP (BVAR (XBUFFER (it->w->contents),
    Jesse> +                        line_spacing_vertical_center)))

I think you've inverted the test here, should this not be

    if (NILP ....)

?

Robert

Patch to vertically center line content when using line-spacing variable

[조성빈]

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

Hello, there was a thread[0,1] before which introduced a patch to  
vertically center line content. I decided to fix the leftover nitpicks. Can  
anybody check this patch and give any feedback?


[-- Attachment #2: 0001-Add-new-variable-for-centering-lines-vertically.patch --]
[-- Type: application/octet-stream, Size: 6219 bytes --]

From 1302c040a23837b80a53416421914f3fe8ba1926 Mon Sep 17 00:00:00 2001
From: Sungbin Jo <pcr910303@icloud.com>
Date: Fri, 24 Jan 2020 00:56:38 +0900
Subject: [PATCH] Add new variable for centering lines vertically.

Co-authored-by: Jesse Nazario <jessenzr@gmail.com>

When using line-spacing, the new variable line-spacing-vertical-center
can be set to non-nil to center the line content vertically.

* src/buffer.h (struct buffer): New member line_spacing_vertical_center.
* src/buffer.c (bset_line_spacing_vertical_center): New setter for
line_spacing_vertical_center.
(reset_buffer): Reset line_spacing_vertical_center.
(init_buffer_once): Initialize line_spacing_vertical_center.
(syms_of_buffer): Declare Lisp variable line-spacing-vertical-center.
* src/xdisp.c (gui_produce_glyphs): Center the text when
line-spacing-vertical-center is non-nil.
* doc/lispref/display.texi (Line Height): Document the new variable.
* etc/NEWS: Announce the new variable.
---
 doc/lispref/display.texi |  5 +++++
 etc/NEWS                 |  4 ++++
 src/buffer.c             | 14 ++++++++++++++
 src/buffer.h             |  4 ++++
 src/xdisp.c              | 10 +++++++++-
 5 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index e4980fe4c3..2fa63ddb56 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2231,6 +2231,11 @@ Line Height
 number of pixels put below lines.  A floating-point number specifies
 the spacing relative to the frame's default line height.
 
+  The @code{line-spacing} parameter creates the space by putting some
+margin only below the line.  If you wish to center the line vertically
+instead, you can set the variable @code{line-spacing-vertical-center}
+as non-nil.
+
 @vindex line-spacing
   You can specify the line spacing for all lines in a buffer via the
 buffer-local @code{line-spacing} variable.  An integer specifies
diff --git a/etc/NEWS b/etc/NEWS
index 11ef31b2c8..fb85131f07 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -51,6 +51,10 @@ It was declared obsolete in Emacs 27.1.
 \f
 * Changes in Emacs 28.1
 
+** New variable line-spacing-vertical-center.
+This variable, if non-nil, centers the line content vertically when
+using the 'line-spacing' variable.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/src/buffer.c b/src/buffer.c
index 5c65d4d4d1..234745a66a 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -219,6 +219,11 @@ bset_extra_line_spacing (struct buffer *b, Lisp_Object val)
   b->extra_line_spacing_ = val;
 }
 static void
+bset_line_spacing_vertical_center (struct buffer *b, Lisp_Object val)
+{
+  b->line_spacing_vertical_center_ = val;
+}
+static void
 bset_file_format (struct buffer *b, Lisp_Object val)
 {
   b->file_format_ = val;
@@ -967,6 +972,8 @@ 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));
 
   b->display_error_modiff = 0;
 }
@@ -5202,6 +5209,7 @@ init_buffer_once (void)
   XSETFASTINT (BVAR (&buffer_local_flags, tab_line_format), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_type), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, extra_line_spacing), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, line_spacing_vertical_center), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_in_non_selected_windows), idx); ++idx;
 
   /* buffer_local_flags contains no pointers, so it's safe to treat it
@@ -5272,6 +5280,7 @@ init_buffer_once (void)
   bset_bidi_paragraph_separate_re (&buffer_defaults, Qnil);
   bset_cursor_type (&buffer_defaults, Qt);
   bset_extra_line_spacing (&buffer_defaults, Qnil);
+  bset_line_spacing_vertical_center (&buffer_defaults, Qnil);
   bset_cursor_in_non_selected_windows (&buffer_defaults, Qt);
 
   bset_enable_multibyte_characters (&buffer_defaults, Qt);
@@ -6268,6 +6277,11 @@ from (abs POSITION).  If POSITION is positive, point was at the front
 If value is a floating point number, it specifies the spacing relative
 to the default frame line height.  A value of nil means add no extra space.  */);
 
+  DEFVAR_PER_BUFFER ("line-spacing-vertical-center",
+		     &BVAR (current_buffer, line_spacing_vertical_center), Qnil,
+                     doc: /* Non-nil means center the line content vertically
+when using `line-spacing' variable.  */);
+
   DEFVAR_PER_BUFFER ("cursor-in-non-selected-windows",
 		     &BVAR (current_buffer, cursor_in_non_selected_windows), Qnil,
 		     doc: /* Non-nil means show a cursor in non-selected windows.
diff --git a/src/buffer.h b/src/buffer.h
index fd05fdd37d..c15065b599 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -553,6 +553,10 @@ #define BVAR(buf, field) ((buf)->field ## _)
      in the display of this buffer.  */
   Lisp_Object extra_line_spacing_;
 
+  /* Non-nil means center the line content vertically. To be used
+     along with `line-spacing'.  */
+  Lisp_Object line_spacing_vertical_center_;
+
   /* Cursor type to display in non-selected windows.
      t means to use hollow box cursor.
      See `cursor-type' for other values.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index a5efbb39be..44727cdd64 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -30527,7 +30527,15 @@ gui_produce_glyphs (struct it *it)
 
   if (extra_line_spacing > 0)
     {
-      it->descent += extra_line_spacing;
+      if (NILP (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;
+        }
       if (extra_line_spacing > it->max_extra_line_spacing)
 	it->max_extra_line_spacing = extra_line_spacing;
     }
-- 
2.21.1 (Apple Git-122.3)


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]




[0] https://lists.gnu.org/archive/html/emacs-devel/2019-08/msg00659.html
[1] https://lists.gnu.org/archive/html/emacs-devel/2019-09/msg00049.html

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

[Eli Zaretskii]

> From: 조성빈 <pcr910303@icloud.com>
> Date: Fri, 24 Jan 2020 01:32:21 +0900
> Cc: jessenzr@gmail.com
> 
> From 1302c040a23837b80a53416421914f3fe8ba1926 Mon Sep 17 00:00:00 2001
> From: Sungbin Jo <pcr910303@icloud.com>
> Date: Fri, 24 Jan 2020 00:56:38 +0900
> Subject: [PATCH] Add new variable for centering lines vertically.
> 
> Co-authored-by: Jesse Nazario <jessenzr@gmail.com>

Most of the patch was written by Jesse, and we don't have a copyright
assignment for Jesse on file.  We need that to accept this
contribution.

> +      if (NILP (BVAR (XBUFFER (it->w->contents),
> +                      line_spacing_vertical_center)))

No need to use BVAR here, since the buffer whose window is being
displayed is always the current buffer when the display code runs.

Thanks.

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

[Jesse Medeiros]

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

How can offer a copyright assignment? I give permission to do whatever you want with this patch. I don't claim any copyright over it.

On Jan 23 2020, at 5:22 pm, Eli Zaretskii <eliz@gnu.org> wrote:
> > From: 조성빈 <pcr910303@icloud.com>
> > Date: Fri, 24 Jan 2020 01:32:21 +0900
> > Cc: jessenzr@gmail.com
> >
> > From 1302c040a23837b80a53416421914f3fe8ba1926 Mon Sep 17 00:00:00 2001
> > From: Sungbin Jo <pcr910303@icloud.com>
> > Date: Fri, 24 Jan 2020 00:56:38 +0900
> > Subject: [PATCH] Add new variable for centering lines vertically.
> >
> > Co-authored-by: Jesse Nazario <jessenzr@gmail.com>
> Most of the patch was written by Jesse, and we don't have a copyright
> assignment for Jesse on file. We need that to accept this
> contribution.
>
> > + if (NILP (BVAR (XBUFFER (it->w->contents),
> > + line_spacing_vertical_center)))
>
>
> No need to use BVAR here, since the buffer whose window is being
> displayed is always the current buffer when the display code runs.
>
> Thanks.

[-- Attachment #2: Type: text/html, Size: 1551 bytes --]

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

[Eli Zaretskii]

> Date: Fri, 7 Feb 2020 14:06:59 -0300
> From: Jesse Medeiros <jessenzr@gmail.com>
> Cc: 조성빈 <pcr910303@icloud.com>, 
>  "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> How can offer a copyright assignment? I give permission to do whatever you want with this patch. I don't claim
> any copyright over it.

We need either a copyright assignment (preferred) or a disclaimer.  If
you'd like to assign the copyright to the FSF, I can send you the form
to fill.

Thanks.

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

[조성빈 via "Emacs development discussions.]

Jesse Medeiros <jessenzr@gmail.com> 작성:

> How can offer a copyright assignment? I give permission to do whatever  
> you want with this patch. I don't claim any copyright over it.

Ping :-)
May I ask whether you have signed the assignment?

> On Jan 23 2020, at 5:22 pm, Eli Zaretskii <eliz@gnu.org> wrote:
> From: 조성빈 <pcr910303@icloud.com>
> Date: Fri, 24 Jan 2020 01:32:21 +0900
> Cc: jessenzr@gmail.com
>
> From 1302c040a23837b80a53416421914f3fe8ba1926 Mon Sep 17 00:00:00 2001
> From: Sungbin Jo <pcr910303@icloud.com>
> Date: Fri, 24 Jan 2020 00:56:38 +0900
> Subject: [PATCH] Add new variable for centering lines vertically.
>
> Co-authored-by: Jesse Nazario <jessenzr@gmail.com>
>
> Most of the patch was written by Jesse, and we don't have a copyright
> assignment for Jesse on file. We need that to accept this
> contribution.
>
> + if (NILP (BVAR (XBUFFER (it->w->contents),
> + line_spacing_vertical_center)))
>
> No need to use BVAR here, since the buffer whose window is being
> displayed is always the current buffer when the display code runs.
>
> Thanks.

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

[Jesse Medeiros]

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

I have not. I think I missed this email. Sure, just send it to me and I'll sign it.


[-- Attachment #2: Type: text/html, Size: 98 bytes --]

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

[Eli Zaretskii]

> Date: Mon, 6 Apr 2020 18:13:13 -0300
> From: Jesse Medeiros <jessenzr@gmail.com>
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> I have not. I think I missed this email. Sure, just send it to me and I'll sign it.

Thanks, form sent off-list.

Patch to vertically center line content when using line-spacing variable

[email]

I am interested in this landing and contacted (and cc'd) Jesse and the
FSF paperwork was completed around June 2020. Is there anything else
holding this back now?

https://lists.gnu.org/archive/html/emacs-devel/2020-01/msg00721.html
has links to the previous discussion.

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

[Lars Ingebrigtsen]

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

email@johnmuhl.me writes:

> I am interested in this landing and contacted (and cc'd) Jesse and the
> FSF paperwork was completed around June 2020. Is there anything else
> holding this back now?
>
> https://lists.gnu.org/archive/html/emacs-devel/2020-01/msg00721.html
> has links to the previous discussion.

I've included the patch below.

I've never used the `line-spacing' variable myself (and my guess is that
few do, which is probably why your message didn't get any responses),
but I did a

  (setq line-spacing 20)

now, and I see that this puts more space at the bottom of the line.  The
proposed patch allows centring instead, which seems eminently reasonable
to me.  (Perhaps this should even be the default?)

Does anybody have any comments here before I apply the patch?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: line.patch --]
[-- Type: text/x-diff, Size: 6068 bytes --]

From 1302c040a23837b80a53416421914f3fe8ba1926 Mon Sep 17 00:00:00 2001
From: Sungbin Jo <pcr910303@icloud.com>
Date: Fri, 24 Jan 2020 00:56:38 +0900
Subject: [PATCH] Add new variable for centering lines vertically.

Co-authored-by: Jesse Nazario <jessenzr@gmail.com>

When using line-spacing, the new variable line-spacing-vertical-center
can be set to non-nil to center the line content vertically.

* src/buffer.h (struct buffer): New member line_spacing_vertical_center.
* src/buffer.c (bset_line_spacing_vertical_center): New setter for
line_spacing_vertical_center.
(reset_buffer): Reset line_spacing_vertical_center.
(init_buffer_once): Initialize line_spacing_vertical_center.
(syms_of_buffer): Declare Lisp variable line-spacing-vertical-center.
* src/xdisp.c (gui_produce_glyphs): Center the text when
line-spacing-vertical-center is non-nil.
* doc/lispref/display.texi (Line Height): Document the new variable.
* etc/NEWS: Announce the new variable.
---
 doc/lispref/display.texi |  5 +++++
 etc/NEWS                 |  4 ++++
 src/buffer.c             | 14 ++++++++++++++
 src/buffer.h             |  4 ++++
 src/xdisp.c              | 10 +++++++++-
 5 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index e4980fe4c3..2fa63ddb56 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2231,6 +2231,11 @@ Line Height
 number of pixels put below lines.  A floating-point number specifies
 the spacing relative to the frame's default line height.
 
+  The @code{line-spacing} parameter creates the space by putting some
+margin only below the line.  If you wish to center the line vertically
+instead, you can set the variable @code{line-spacing-vertical-center}
+as non-nil.
+
 @vindex line-spacing
   You can specify the line spacing for all lines in a buffer via the
 buffer-local @code{line-spacing} variable.  An integer specifies
diff --git a/etc/NEWS b/etc/NEWS
index 11ef31b2c8..fb85131f07 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -51,6 +51,10 @@ It was declared obsolete in Emacs 27.1.
 \f
 * Changes in Emacs 28.1
 
+** New variable line-spacing-vertical-center.
+This variable, if non-nil, centers the line content vertically when
+using the 'line-spacing' variable.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/src/buffer.c b/src/buffer.c
index 5c65d4d4d1..234745a66a 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -219,6 +219,11 @@ bset_extra_line_spacing (struct buffer *b, Lisp_Object val)
   b->extra_line_spacing_ = val;
 }
 static void
+bset_line_spacing_vertical_center (struct buffer *b, Lisp_Object val)
+{
+  b->line_spacing_vertical_center_ = val;
+}
+static void
 bset_file_format (struct buffer *b, Lisp_Object val)
 {
   b->file_format_ = val;
@@ -967,6 +972,8 @@ 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));
 
   b->display_error_modiff = 0;
 }
@@ -5202,6 +5209,7 @@ init_buffer_once (void)
   XSETFASTINT (BVAR (&buffer_local_flags, tab_line_format), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_type), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, extra_line_spacing), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, line_spacing_vertical_center), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, cursor_in_non_selected_windows), idx); ++idx;
 
   /* buffer_local_flags contains no pointers, so it's safe to treat it
@@ -5272,6 +5280,7 @@ init_buffer_once (void)
   bset_bidi_paragraph_separate_re (&buffer_defaults, Qnil);
   bset_cursor_type (&buffer_defaults, Qt);
   bset_extra_line_spacing (&buffer_defaults, Qnil);
+  bset_line_spacing_vertical_center (&buffer_defaults, Qnil);
   bset_cursor_in_non_selected_windows (&buffer_defaults, Qt);
 
   bset_enable_multibyte_characters (&buffer_defaults, Qt);
@@ -6268,6 +6277,11 @@ from (abs POSITION).  If POSITION is positive, point was at the front
 If value is a floating point number, it specifies the spacing relative
 to the default frame line height.  A value of nil means add no extra space.  */);
 
+  DEFVAR_PER_BUFFER ("line-spacing-vertical-center",
+		     &BVAR (current_buffer, line_spacing_vertical_center), Qnil,
+                     doc: /* Non-nil means center the line content vertically
+when using `line-spacing' variable.  */);
+
   DEFVAR_PER_BUFFER ("cursor-in-non-selected-windows",
 		     &BVAR (current_buffer, cursor_in_non_selected_windows), Qnil,
 		     doc: /* Non-nil means show a cursor in non-selected windows.
diff --git a/src/buffer.h b/src/buffer.h
index fd05fdd37d..c15065b599 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -553,6 +553,10 @@ #define BVAR(buf, field) ((buf)->field ## _)
      in the display of this buffer.  */
   Lisp_Object extra_line_spacing_;
 
+  /* Non-nil means center the line content vertically. To be used
+     along with `line-spacing'.  */
+  Lisp_Object line_spacing_vertical_center_;
+
   /* Cursor type to display in non-selected windows.
      t means to use hollow box cursor.
      See `cursor-type' for other values.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index a5efbb39be..44727cdd64 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -30527,7 +30527,15 @@ gui_produce_glyphs (struct it *it)
 
   if (extra_line_spacing > 0)
     {
-      it->descent += extra_line_spacing;
+      if (NILP (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;
+        }
       if (extra_line_spacing > it->max_extra_line_spacing)
 	it->max_extra_line_spacing = extra_line_spacing;
     }
-- 
2.21.1 (Apple Git-122.3)

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

[john muhl]

On Sun, 2021-04-25 at 21:41 +0200, Lars Ingebrigtsen wrote:
> email@johnmuhl.me writes:
> 
> > I am interested in this landing and contacted (and cc'd) Jesse and
> > the
> > FSF paperwork was completed around June 2020. Is there anything else
> > holding this back now?
> > 
> > https://lists.gnu.org/archive/html/emacs-devel/2020-01/msg00721.html
> > has links to the previous discussion.
> 
> I've included the patch below.
> 
> I've never used the `line-spacing' variable myself (and my guess is
> that
> few do, which is probably why your message didn't get any responses),
> but I did a
> 
>   (setq line-spacing 20)
> 
> now, and I see that this puts more space at the bottom of the line. 
> The
> proposed patch allows centring instead, which seems eminently
> reasonable
> to me.  (Perhaps this should even be the default?)
> 
> Does anybody have any comments here before I apply the patch?
> 

Thanks for taking look. I've been using the proposed patch the last
couple of weeks and it works for me. The echo area does not handle
it perfectly but it also doesn't perfectly handle the current behavior
of non-nil line-spacing so hopefully not a blocker for landing.

I added the following line to make it settable from customize.

---
 lisp/cus-start.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index b7afef6516..21ad616cc9 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -154,6 +154,7 @@ minibuffer-prompt-properties--setter
                                       "21.1")
             (line-spacing display (choice (const :tag "none" nil)
number)
                           "22.1")
+             (line-spacing-vertical-center display boolean "28.1")
             (cursor-in-non-selected-windows
              cursor ,cursor-type-types nil
              :tag "Cursor In Non-selected Windows"
-- 

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 25 Apr 2021 21:41:09 +0200
> Cc: jessenzr@gmail.com, emacs-devel@gnu.org
> 
> Does anybody have any comments here before I apply the patch?

Please wait for a day or two, I would like to take a look at this.

Thanks.

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

[Fu Yuan]

> 在 2021年4月25日，下午4:24，Eli Zaretskii <eliz@gnu.org> 写道：
> 
> 
>> 
>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Date: Sun, 25 Apr 2021 21:41:09 +0200
>> Cc: jessenzr@gmail.com, emacs-devel@gnu.org
>> 
>> Does anybody have any comments here before I apply the patch?
> 
> Please wait for a day or two, I would like to take a look at this.
> 
> Thanks.
> 

Without this patch, one can center a line by using line-spacing and line-height together. However line-height has to be applied as a text property, while line-spacing can be set as a variable. So this patch is useful in that regard. Has anyone looked at how does this patch interact with line-height?

Yuan

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

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I've never used the `line-spacing' variable myself (and my guess is that
> few do, which is probably why your message didn't get any responses),

I use it, but I had missed this patch.

> now, and I see that this puts more space at the bottom of the line.  The
> proposed patch allows centring instead, which seems eminently reasonable
> to me.  (Perhaps this should even be the default?)

I think it should be the default, yes.

(I would just treat the old behavior as a bug and not even add a
variable for it, but that's me.  Perhaps there is some rationale for
why someone would want to set this to nil?)

> Does anybody have any comments here before I apply the patch?

It works here, thanks!

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

[Clément Pit-Claudel]

On 4/25/21 7:28 PM, Stefan Kangas wrote:
> (I would just treat the old behavior as a bug and not even add a
> variable for it, but that's me.  Perhaps there is some rationale for
> why someone would want to set this to nil?)

Does this patch change the behavior of the line-spacing text property, too, or just the line-spacing variable?

I use the line property in multiple packages, and it would break these packages is line-spacing suddenly added space above the line, not just below.

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

[Stefan Kangas]

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> On 4/25/21 7:28 PM, Stefan Kangas wrote:
>> (I would just treat the old behavior as a bug and not even add a
>> variable for it, but that's me.  Perhaps there is some rationale for
>> why someone would want to set this to nil?)
>
> Does this patch change the behavior of the line-spacing text property,
> too, or just the line-spacing variable?

It seems to affect the 'line-spacing' text property too.

> I use the line property in multiple packages, and it would break these
> packages is line-spacing suddenly added space above the line, not just
> below.

I'll take your word for it.  It sounds like I was wrong and there should
be a variable that you could set buffer locally to nil.

Out of curiosity, could you briefly explain how this would break your
use of this text property?  And/or could you link one of your packages?

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

[Clément Pit-Claudel]

On 4/25/21 9:00 PM, Stefan Kangas wrote:
> Clément Pit-Claudel <cpitclaudel@gmail.com> writes:
> 
>> On 4/25/21 7:28 PM, Stefan Kangas wrote:
>>> (I would just treat the old behavior as a bug and not even add a
>>> variable for it, but that's me.  Perhaps there is some rationale for
>>> why someone would want to set this to nil?)
>>
>> Does this patch change the behavior of the line-spacing text property,
>> too, or just the line-spacing variable?
> 
> It seems to affect the 'line-spacing' text property too.
> 
>> I use the line property in multiple packages, and it would break these
>> packages is line-spacing suddenly added space above the line, not just
>> below.
> 
> I'll take your word for it.  It sounds like I was wrong and there should
> be a variable that you could set buffer locally to nil.

I'm not sure that would be sufficient: the user might want vertical centering for some instances of 'line-spacing and not others.

> Out of curiosity, could you briefly explain how this would break your
> use of this text property?

I use it in combination with other properties to achieve specific spacing around certain elements or lines; it would change the way the result looks.  Right now we have a property that adds space above a line and one that adds space below, and with the change enabled it becomes impossible to add space just below the line, right?

> And/or could you link one of your packages? 

https://github.com/FStarLang/fstar-mode.el
Another package that's not mine: https://github.com/aaronbieber/sunshine.el

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

[Clément Pit-Claudel]

On 4/25/21 3:41 PM, Lars Ingebrigtsen wrote:
> +          int spacing = extra_line_spacing / 2;
> +          it->ascent += spacing;
> +          it->descent += spacing;

Wouldn't it be better to use `it->descent += extra_line_spacing - spacing'?  Otherwise the lsb of extra_line_spacing is ignored.

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 25 Apr 2021 21:41:09 +0200
> Cc: jessenzr@gmail.com, emacs-devel@gnu.org
> 
> I've included the patch below.
> 
> I've never used the `line-spacing' variable myself (and my guess is that
> few do, which is probably why your message didn't get any responses),
> but I did a
> 
>   (setq line-spacing 20)
> 
> now, and I see that this puts more space at the bottom of the line.  The
> proposed patch allows centring instead, which seems eminently reasonable
> to me.  (Perhaps this should even be the default?)
> 
> Does anybody have any comments here before I apply the patch?

I have a few comments, some (most) of them were already voiced, either
in this discussion or in the past ones on the same theme:

 . I don't see a need for a separate new variable (and a built-in one
   on top of that).  I think it is cleaner to extend the line-spacing
   property, parameter and variables to support a cons of two numbers,
   (ABOVE . TOTAL), which would allow users and Lisp programs to
   specify how to subdivide the spacing between the ascent and descent
   parts.  This would then also solve the problem with the proposed
   change, whereby the vertical position of the text inside the
   spacing can only be controlled per buffer, but not on every level
   where the line-spacing is supported
 . The code misses one other place where line-spacing takes effect,
   see the function append_space_for_newline.
 . The division between ascent and descent in the patch loses 1 pixel
   if the value of line-spacing is an odd number of pixels, so the
   code needs to be fixed not to do that.

Thanks.

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

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>  . I don't see a need for a separate new variable (and a built-in one
>    on top of that).  I think it is cleaner to extend the line-spacing
>    property, parameter and variables to support a cons of two numbers,
>    (ABOVE . TOTAL), which would allow users and Lisp programs to
>    specify how to subdivide the spacing between the ascent and descent
>    parts.

Yeah, that seems like a more flexible solution.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no