bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher (CIRES/NOAA) via Bug reports for GNU Emacs, the Swiss army knife of text editors]

linum mode has several performance issues but has the convenient ability
to format the line numbers in a way chosen by the user. It would be
wonderful to be able to add a "separator" character of choosing to
display-line-numbers-mode. I would be happy to try to contribute code
but I'm unsure of where to start with such a task. Thanks everyone for
all the incredible work, it has greatly enabled my research as a scientist


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.17.4)
 of 2021-08-18 built on slytux
Repository revision: 0d5ff75e944e765dce0e3a93ebdefa4fb6d5f356
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --mandir=/usr/share/man --with-gameuser=:games
 --with-sound=alsa --with-modules --without-gconf --without-gsettings
 --with-libsystemd --localstatedir=/var --with-compress-install
 --with-x-toolkit --without-sound --without-xpm --without-imagemagick
 --with-mailutils --with-pop --without-xml2 --without-json
 --without-harfbuzz --without-libotf --without-m17n-flt
 --without-toolkit-scroll-bars --without-xaw3d --without-xim
 --without-gpm --without-dbus --without-gsettings --without-selinux
 --with-modules --without-makeinfo --without-libgmp --with-cairo
 --disable-largefile --enable-link-time-optimization
 --with-native-compilation --enable-link-time-optimization
 '--program-transform-name=s/\([ec]tags\)/\1.emacs/' 'CFLAGS=-pipe
 -march=native -mtune=native -Ofast -fomit-frame-pointer
 -fno-finite-math-only -flto -fuse-linker-plugin' 'LDFLAGS=-pipe
 -march=native -mtune=native -Ofast -fno-finite-math-only''

Configured features:
ACL CAIRO FREETYPE GIF GLIB GNUTLS JPEG LCMS2 LIBSYSTEMD MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP THREADS TIFF X11
XDBE XIM GTK3 ZLIB

Important settings:
  value of $LANG: C
  locale-coding-system: utf-8

Memory information:
((conses 16 572042 263662)
 (symbols 48 39163 3)
 (strings 32 142081 23304)
 (string-bytes 1 5048285)
 (vectors 16 72877)
 (vector-slots 8 1976472 259237)
 (floats 8 1268 582)
 (intervals 56 18624 15713)
 (buffers 992 25))


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> Date: Fri, 10 Sep 2021 01:40:45 -0600
> From:  "Michael Gallagher (CIRES/NOAA)" via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> linum mode has several performance issues but has the convenient ability
> to format the line numbers in a way chosen by the user. It would be
> wonderful to be able to add a "separator" character of choosing to
> display-line-numbers-mode.

Please tell more about this "separator character": what should it be
and how will it be displayed?  And what is its purpose in the first
place?

I'd like to avoid adding a general format-style extension to the
native line numbers, that's a kind of creeping featurism that will
eventually slow down redisplay too much, something that the
display-line-numbers implementation explicitly attempts to avoid.
People who must have the full-fledged format capability could simply
use linum-mode instead.

> I would be happy to try to contribute code but I'm unsure of where
> to start with such a task.

The code is in xdisp.c, if that's what you meant to ask.

Thanks.

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli,

Thanks for taking the time to respond. The separator character I use is
simply the 'pipe' character, "|". The purpose for me is to clearly
delineate the text/code from the line numbers in a way that makes it easier
to grok the text being displayed. My brain sees the numbers as part of the
code with the current formatting of display-line-numbers, but maybe I'm
uniquely incapable.

I completely understand feature creep, and the reason I've switched to
display-line-numbers *because* of performance. Using a combination of
outline-minor-mode (with folding) and linum-mode made emacs run at 100% cpu
while moving the cursor in the terminal. Not to mention other linum bugs.

Best.

On Fri, Sep 10, 2021 at 6:31 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Fri, 10 Sep 2021 01:40:45 -0600
> > From:  "Michael Gallagher (CIRES/NOAA)" via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >
> > linum mode has several performance issues but has the convenient ability
> > to format the line numbers in a way chosen by the user. It would be
> > wonderful to be able to add a "separator" character of choosing to
> > display-line-numbers-mode.
>
> Please tell more about this "separator character": what should it be
> and how will it be displayed?  And what is its purpose in the first
> place?
>
> I'd like to avoid adding a general format-style extension to the
> native line numbers, that's a kind of creeping featurism that will
> eventually slow down redisplay too much, something that the
> display-line-numbers implementation explicitly attempts to avoid.
> People who must have the full-fledged format capability could simply
> use linum-mode instead.
>
> > I would be happy to try to contribute code but I'm unsure of where
> > to start with such a task.
>
> The code is in xdisp.c, if that's what you meant to ask.
>
> Thanks.
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

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

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Fri, 10 Sep 2021 10:00:47 -0600
> Cc: 50506@debbugs.gnu.org
> 
> Thanks for taking the time to respond. The separator character I use is simply the 'pipe' character, "|". The
> purpose for me is to clearly delineate the text/code from the line numbers in a way that makes it easier to
> grok the text being displayed. My brain sees the numbers as part of the code with the current formatting of
> display-line-numbers, but maybe I'm uniquely incapable. 

If the intent is to provide a new user option to display such a
character after the number, I don't object.  But please note the
following subtle aspects of such a change:

  . since you'd probably want to make the character customizable, and
    non-ASCII characters are sometimes desired, there will be a need
    to make sure such a non-ASCII character can be displayed before
    honoring the customization

  . on some displays, the pipe character "|" isn't tall enough, so the
    vertical line is not contiguous

  . there's a problem with doing this for right-to-left lines, because
    the character should then be before the number, not after it, but
    (due to complicated reasons I will not describe here) when the
    line number is prepared for display, the display code doesn't
    necessarily know whether the line is R2L or L2R

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Lars Ingebrigtsen]

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

Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
writes:

> The purpose for me is to clearly delineate the
> text/code from the line numbers in a way that makes it easier to grok the
> text being displayed. My brain sees the numbers as part of the code with the
> current formatting of display-line-numbers, but maybe I'm uniquely
> incapable. 

Does it help to customize the `line-number' face to something that has a
different background colour than the main text?  For instance, setting
the background colour to black gives me this:


[-- Attachment #2: Type: image/png, Size: 4262 bytes --]

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


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

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Thank you all for the replies.

Lars, I do this already but it only really helps most when it's high
contrast (white --> black) like you've shown here. And with that, it hurts
my brain for other reasons... I swear I'm not overly sensitive.

Eli, as far as your suggestions, these requirements make sense to me. I
took a crack at modifying xdisp.c and was able to display ascii characters
by appending to lnum_buf before the numbers are displayed. However, when I
tried to hook that into the lisp code and add a variable so that it
wouldn't be hardcoded (a must, of course) I fell flat on my face.
Converting between character lisp variables to the appropriate C types is
above my "undergrad CSCI refresher" pay grade. I wish I could be more
helpful but if I were to be the one to submit a pull request I would
probably need someone to hold my hand.

Best,
Michael

On Sat, Sep 11, 2021 at 6:03 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> writes:
>
> > The purpose for me is to clearly delineate the
> > text/code from the line numbers in a way that makes it easier to grok the
> > text being displayed. My brain sees the numbers as part of the code with
> the
> > current formatting of display-line-numbers, but maybe I'm uniquely
> > incapable.
>
> Does it help to customize the `line-number' face to something that has a
> different background colour than the main text?  For instance, setting
> the background colour to black gives me this:
>
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

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

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Mon, 13 Sep 2021 11:54:59 -0600
> Cc: Eli Zaretskii <eliz@gnu.org>, 50506@debbugs.gnu.org
> 
> Eli, as far as your suggestions, these requirements make sense to me. I took a crack at modifying xdisp.c
> and was able to display ascii characters by appending to lnum_buf before the numbers are displayed.
> However, when I tried to hook that into the lisp code and add a variable so that it wouldn't be hardcoded (a
> must, of course) I fell flat on my face. Converting between character lisp variables to the appropriate C types
> is above my "undergrad CSCI refresher" pay grade. I wish I could be more helpful but if I were to be the one
> to submit a pull request I would probably need someone to hold my hand. 

It's not too complicated.  A character is just an integer in Emacs,
and the way to get a C integer from a Lisp integer is by using the
XFIXNAT macro.

Look for the code which handles display-fill-column-indicator-mode, in
particular how it takes the indicator character from
Vdisplay_fill_column_indicator_character and adds it to a screen line.
What you need to do is very similar.

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Funny enough, that's *exactly* what I tried to do, but I ran into problems
getting the variable to display in customize-variable (a problem with what
I put in cus-start.el?) and if I manually used setq it didn't seem to get
translated correctly by the way I coded the macro.

However, I'll try again next weekend and maybe you'll see a pull request
from me sometime.

Michael

On Mon, Sep 13, 2021 at 12:05 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> > Date: Mon, 13 Sep 2021 11:54:59 -0600
> > Cc: Eli Zaretskii <eliz@gnu.org>, 50506@debbugs.gnu.org
> >
> > Eli, as far as your suggestions, these requirements make sense to me. I
> took a crack at modifying xdisp.c
> > and was able to display ascii characters by appending to lnum_buf before
> the numbers are displayed.
> > However, when I tried to hook that into the lisp code and add a variable
> so that it wouldn't be hardcoded (a
> > must, of course) I fell flat on my face. Converting between character
> lisp variables to the appropriate C types
> > is above my "undergrad CSCI refresher" pay grade. I wish I could be more
> helpful but if I were to be the one
> > to submit a pull request I would probably need someone to hold my hand.
>
> It's not too complicated.  A character is just an integer in Emacs,
> and the way to get a C integer from a Lisp integer is by using the
> XFIXNAT macro.
>
> Look for the code which handles display-fill-column-indicator-mode, in
> particular how it takes the indicator character from
> Vdisplay_fill_column_indicator_character and adds it to a screen line.
> What you need to do is very similar.
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

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

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Mon, 13 Sep 2021 12:17:09 -0600
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> 
> Funny enough, that's *exactly* what I tried to do, but I ran into problems getting the variable to display in
> customize-variable (a problem with what I put in cus-start.el?) and if I manually used setq it didn't seem to
> get translated correctly by the way I coded the macro. 

I think if you look up all the pieces that handle
display-fill-column-indicator-character, both in C and in Lisp, you
will have your answers.  Feel free to ask specific questions if
something is still unclear after that.

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 1315 bytes --]

OK. Here we go. I found some time last night. Attached is a git patch file
for changes that implement this feature request as well as (GNU) diffs for
the individual files in question. It's a pretty simple commit. It's worth
noting that I'm probably overlooking something in how I create the glyph, I
definitely don't understand the nuances of the display code. But I hope
it's helpful.

On Mon, Sep 13, 2021 at 12:27 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> > Date: Mon, 13 Sep 2021 12:17:09 -0600
> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> >
> > Funny enough, that's *exactly* what I tried to do, but I ran into
> problems getting the variable to display in
> > customize-variable (a problem with what I put in cus-start.el?) and if I
> manually used setq it didn't seem to
> > get translated correctly by the way I coded the macro.
>
> I think if you look up all the pieces that handle
> display-fill-column-indicator-character, both in C and in Lisp, you
> will have your answers.  Feel free to ask specific questions if
> something is still unclear after that.
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

[-- Attachment #1.2: Type: text/html, Size: 2348 bytes --]

[-- Attachment #2: individual_diffs.tgz --]
[-- Type: application/x-compressed-tar, Size: 1203 bytes --]

[-- Attachment #3: 0021-add-new-variable-for-optional-character-to-end-of-li.patch --]
[-- Type: text/x-patch, Size: 3089 bytes --]

From f4c5fd9a8d49f6c1be66c2fddd3f506dcc92d547 Mon Sep 17 00:00:00 2001
From: polair <michael.r.gallagher@noaa.gov>
Date: Tue, 14 Sep 2021 10:12:14 -0600
Subject: [PATCH 21/21] add new variable for optional character to end of
 line-numbers line

---
 lisp/cus-start.el |  6 ++++++
 lisp/frame.el     |  1 +
 src/xdisp.c       | 16 ++++++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 1a3e5682bba..c755aaa41cf 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -740,6 +740,12 @@ minibuffer-prompt-properties--setter
                (const :tag "Visually relative line numbers"
                       :value visual))
               "26.1")
+             (display-line-numbers-separator-character
+              display-line-numbers
+              (character :tag "Character to separate numbers from buffer")
+              :safe (lambda (value) (or (characterp value) (null value)))
+              "28.05")
+              ;:safe (lambda (value) (or (characterp value) (null value))))
              (display-line-numbers-width
               display-line-numbers
               (choice
diff --git a/lisp/frame.el b/lisp/frame.el
index 60234fc2ae9..c6975a71cf2 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2876,6 +2876,7 @@ set-frame-property--interactive
         header-line-format
         tab-line-format
         display-line-numbers
+        display-line-numbers-separator-character
         display-line-numbers-width
         display-line-numbers-current-absolute
         display-line-numbers-widen
diff --git a/src/xdisp.c b/src/xdisp.c
index d30a68570f0..92225af685a 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -23296,6 +23296,15 @@ maybe_produce_line_number (struct it *it)
 	}
     }
 
+  /* if last character in string, and separator is set, draw separator glyph */
+  if (FIXNATP (Vdisplay_line_numbers_separator_character))
+    {
+      tem_it.c = tem_it.char_to_display = XFIXNAT (Vdisplay_line_numbers_separator_character);
+      tem_it.len = 1;
+      SET_TEXT_POS (tem_it.position, -1, -1);
+      PRODUCE_GLYPHS (&tem_it);
+    }
+
   inhibit_free_realized_faces = save_free_realized_faces;
 
   /* Record the width in pixels we need for the line number display.  */
@@ -35509,6 +35518,13 @@ syms_of_xdisp (void)
   DEFSYM (Qrelative, "relative");
   DEFSYM (Qvisual, "visual");
 
+  DEFVAR_LISP ("display-line-numbers-separator-character", Vdisplay_line_numbers_separator_character,
+    doc: /* Character to draw between line numbers and buffer when non-nil. Some interesting
+options are: ├, ╞, │, ┃, ║, ▶, ⧽, and ⟫ */);
+  Vdisplay_line_numbers_separator_character = Qnil;
+  DEFSYM (Qdisplay_line_numbers_separator_character, "display-line-numbers-separator-character");
+  //Fmake_variable_buffer_local (Qdisplay_line_numbers_separator_character);
+
   DEFVAR_LISP ("display-line-numbers-width", Vdisplay_line_numbers_width,
     doc: /* Minimum width of space reserved for line number display.
 A positive number means reserve that many columns for line numbers,
-- 
2.33.0

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 1635 bytes --]

Here's a screenshot.

On Tue, Sep 14, 2021 at 10:17 AM Michael Gallagher - NOAA Affiliate <
michael.r.gallagher@noaa.gov> wrote:

> OK. Here we go. I found some time last night. Attached is a git patch file
> for changes that implement this feature request as well as (GNU) diffs for
> the individual files in question. It's a pretty simple commit. It's worth
> noting that I'm probably overlooking something in how I create the glyph, I
> definitely don't understand the nuances of the display code. But I hope
> it's helpful.
>
> On Mon, Sep 13, 2021 at 12:27 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
>> > Date: Mon, 13 Sep 2021 12:17:09 -0600
>> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
>> >
>> > Funny enough, that's *exactly* what I tried to do, but I ran into
>> problems getting the variable to display in
>> > customize-variable (a problem with what I put in cus-start.el?) and if
>> I manually used setq it didn't seem to
>> > get translated correctly by the way I coded the macro.
>>
>> I think if you look up all the pieces that handle
>> display-fill-column-indicator-character, both in C and in Lisp, you
>> will have your answers.  Feel free to ask specific questions if
>> something is still unclear after that.
>>
>
>
> --
> Michael Gallagher, PhD
> CIRES Research Scientist
> Polar Observations and Processes Team (ESRL/NOAA/PSD)
> 325 Broadway, Boulder, Colorado 80305
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

[-- Attachment #1.2: Type: text/html, Size: 3389 bytes --]

[-- Attachment #2: disp_nums_screenshot.png --]
[-- Type: image/png, Size: 433946 bytes --]

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Tue, 14 Sep 2021 10:17:31 -0600
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> 
> OK. Here we go. I found some time last night. Attached is a git patch file for changes that implement this
> feature request as well as (GNU) diffs for the individual files in question. It's a pretty simple commit. It's worth
> noting that I'm probably overlooking something in how I create the glyph, I definitely don't understand the
> nuances of the display code. But I hope it's helpful. 

Thanks.  But I don't see that you did anything about the problem with
R2L lines.  What happens if you turn on display-line-numbers-mode in
the buffer that visits TUTORIAL.he, and then turn on this feature in
that buffer? don't you see the separator line on the wrong side of the
line numbers?

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 1250 bytes --]

Eli, I did indeed. I've added corrections to account for L2R vs R2L and the
updated patch is attached. Let me know what you think

On Tue, Sep 14, 2021 at 11:35 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> > Date: Tue, 14 Sep 2021 10:17:31 -0600
> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> >
> > OK. Here we go. I found some time last night. Attached is a git patch
> file for changes that implement this
> > feature request as well as (GNU) diffs for the individual files in
> question. It's a pretty simple commit. It's worth
> > noting that I'm probably overlooking something in how I create the
> glyph, I definitely don't understand the
> > nuances of the display code. But I hope it's helpful.
>
> Thanks.  But I don't see that you did anything about the problem with
> R2L lines.  What happens if you turn on display-line-numbers-mode in
> the buffer that visits TUTORIAL.he, and then turn on this feature in
> that buffer? don't you see the separator line on the wrong side of the
> line numbers?
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

[-- Attachment #1.2: Type: text/html, Size: 2287 bytes --]

[-- Attachment #2: 0021-inclusion-of-line-number-separator-character.patch --]
[-- Type: text/x-patch, Size: 3631 bytes --]

From 550d2716feb78cf6353e8d9a8f63b7fd12adfd83 Mon Sep 17 00:00:00 2001
From: polair <michael.r.gallagher@noaa.gov>
Date: Wed, 15 Sep 2021 09:18:58 -0600
Subject: [PATCH 21/21] inclusion of line number separator character

---
 lisp/cus-start.el |  6 ++++++
 lisp/frame.el     |  1 +
 src/xdisp.c       | 24 ++++++++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 1a3e5682bba..c755aaa41cf 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -740,6 +740,12 @@ minibuffer-prompt-properties--setter
                (const :tag "Visually relative line numbers"
                       :value visual))
               "26.1")
+             (display-line-numbers-separator-character
+              display-line-numbers
+              (character :tag "Character to separate numbers from buffer")
+              :safe (lambda (value) (or (characterp value) (null value)))
+              "28.05")
+              ;:safe (lambda (value) (or (characterp value) (null value))))
              (display-line-numbers-width
               display-line-numbers
               (choice
diff --git a/lisp/frame.el b/lisp/frame.el
index 60234fc2ae9..c6975a71cf2 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2876,6 +2876,7 @@ set-frame-property--interactive
         header-line-format
         tab-line-format
         display-line-numbers
+        display-line-numbers-separator-character
         display-line-numbers-width
         display-line-numbers-current-absolute
         display-line-numbers-widen
diff --git a/src/xdisp.c b/src/xdisp.c
index d30a68570f0..32fc6089292 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -23265,6 +23265,14 @@ maybe_produce_line_number (struct it *it)
 	tem_it.face_id = merge_faces (it->w, Qline_number_minor_tick,
 				      0, DEFAULT_FACE_ID);
     }
+  if (FIXNATP (Vdisplay_line_numbers_separator_character)
+      && it->paragraph_embedding != L2R)
+    {
+      tem_it.c = tem_it.char_to_display = XFIXNAT (Vdisplay_line_numbers_separator_character);
+      tem_it.len = 1;
+      SET_TEXT_POS (tem_it.position, -1, -1);
+      PRODUCE_GLYPHS (&tem_it);
+    }
 
   /* Produce glyphs for the line number in a scratch glyph_row.  */
   for (const char *p = lnum_buf; *p; p++)
@@ -23296,6 +23304,15 @@ maybe_produce_line_number (struct it *it)
 	}
     }
 
+  if (FIXNATP (Vdisplay_line_numbers_separator_character)
+      && it->paragraph_embedding == L2R)
+    {
+      tem_it.c = tem_it.char_to_display = XFIXNAT (Vdisplay_line_numbers_separator_character);
+      tem_it.len = 1;
+      SET_TEXT_POS (tem_it.position, -1, -1);
+      PRODUCE_GLYPHS (&tem_it);
+    }
+
   inhibit_free_realized_faces = save_free_realized_faces;
 
   /* Record the width in pixels we need for the line number display.  */
@@ -35509,6 +35526,13 @@ syms_of_xdisp (void)
   DEFSYM (Qrelative, "relative");
   DEFSYM (Qvisual, "visual");
 
+  DEFVAR_LISP ("display-line-numbers-separator-character", Vdisplay_line_numbers_separator_character,
+    doc: /* Character to draw between line numbers and buffer when non-nil. Some interesting
+options are: ├, ╞, │, ┃, ║, ▶, ⧽, and ⟫ */);
+  Vdisplay_line_numbers_separator_character = Qnil;
+  DEFSYM (Qdisplay_line_numbers_separator_character, "display-line-numbers-separator-character");
+  //Fmake_variable_buffer_local (Qdisplay_line_numbers_separator_character);
+
   DEFVAR_LISP ("display-line-numbers-width", Vdisplay_line_numbers_width,
     doc: /* Minimum width of space reserved for line number display.
 A positive number means reserve that many columns for line numbers,
-- 
2.33.0

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Wed, 15 Sep 2021 09:22:18 -0600
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> 
> Eli, I did indeed. I've added corrections to account for L2R vs R2L and the updated patch is attached. Let me
> know what you think

Thanks, but does this work in buffers where bidi-paragraph-direction
is nil?  AFAIR, it->paragraph_embedding is only set when the buffer
has a non-nil value of bidi-paragraph-direction.

Did you try that in TUTORIAL.he, as I suggested?

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

The code as it stands displays correctly with TUTORIAL.he. As I'm not an
expert in the emacs display code, I was confused about the relationship
between paragraph_embedding and bidi-paragraph_direction. I'm assuming
you're saying there needs to be an additional check on the value of
para_direction in the if statements?

On Wed, Sep 15, 2021 at 9:50 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> > Date: Wed, 15 Sep 2021 09:22:18 -0600
> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> >
> > Eli, I did indeed. I've added corrections to account for L2R vs R2L and
> the updated patch is attached. Let me
> > know what you think
>
> Thanks, but does this work in buffers where bidi-paragraph-direction
> is nil?  AFAIR, it->paragraph_embedding is only set when the buffer
> has a non-nil value of bidi-paragraph-direction.
>
> Did you try that in TUTORIAL.he, as I suggested?
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

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

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Wed, 15 Sep 2021 10:08:49 -0600
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> 
> 
> [1:text/plain Show]
> 
> 
> [2:text/html Hide Save:noname (5kB)]
> 
> The code as it stands displays correctly with TUTORIAL.he.

What is the value of bidi-paragraph-direction in the buffer where you
visit TUTORIAL.he for testing this feature?  It should be nil.

Your code does this:

  +  if (FIXNATP (Vdisplay_line_numbers_separator_character)
  +      && it->paragraph_embedding != L2R)
  +    {
  +      tem_it.c = tem_it.char_to_display = XFIXNAT (Vdisplay_line_numbers_separator_character);

That is, it treats any value of it->paragraph_embedding that is not
L2R as if it were R2L.  But that is incorrect, because the usual value
of it->paragraph_embedding is NEUTRAL_DIR, which corresponds to the
nil value of bidi-paragraph-direction, the default.  That nil value
means to determine the actual paragraph direction dynamically from the
contents of the paragraph.  It is this nil value that presents the
problem.  Your code treats that value as meaning right-to-left, but
that is wrong.

For example, all of the paragraphs on TUTORIAL.he are displayed
right-to-left (because they include right-to-left text), but the last
paragraph, the one which shows the file-local variables, is displayed
left-to-right.  Try turning on this feature and look at that last part
of the buffer to see if your code works correctly.

> I was confused about the relationship between paragraph_embedding and bidi-paragraph_direction.

bidi-paragraph-direction determines the value of it->paragraph_embedding.
However, the default value of bidi-paragraph-direction is nil, and
that is translated into NEUTRAL_DIR value of it->paragraph_embedding.

> I'm
> assuming you're saying there needs to be an additional check on the value of para_direction in the if
> statements?

I don't think you can base this on it->paragraph_embedding, because it
doesn't reflect the actual paragraph direction on display when
bidi-paragraph-direction is nil, the default.  The display code
computes the actual direction of each screen line dynamically in that
case, and stores it in the reversed_p flag of the glyph row.  The
problem is that when the code generates the line number of the first
screen line it needs to produce, that flag is not yet computed, it
will only be computed when the first character on the screen line, the
one after the line number, will be laid out.  And by that time, the
line number was already generated.  This is the problem that needs
fixing for this to work reliably with all scripts.

Thanks.

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

So, in concept then, as the display-line-numbers code operates now adding a
separator character that respects direction isn't possible because when
maybe_produce_line_number is called the code doesn't yet know the direction
of the text. The correct fix is to somehow have the function call for
generating the line number glyphs after the buffer glyphs are computed...
or to generate both L2R and R2L line numbers and then let the code decide
what to display once reversed_p is decided.

This is verified by the fact that if I make a check on paragraph_direction
instead of embedding, the first line number displays incorrectly because
this flag has yet to be set.

Either way, I hate to admit it, but any solution to that problem is way
beyond my skillset and you'd have to spend a lot of time checking/fixing
any my work if I did make the attempt.

On Wed, Sep 15, 2021 at 10:26 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> > Date: Wed, 15 Sep 2021 10:08:49 -0600
> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> >
> >
> > [1:text/plain Show]
> >
> >
> > [2:text/html Hide Save:noname (5kB)]
> >
> > The code as it stands displays correctly with TUTORIAL.he.
>
> What is the value of bidi-paragraph-direction in the buffer where you
> visit TUTORIAL.he for testing this feature?  It should be nil.
>
> Your code does this:
>
>   +  if (FIXNATP (Vdisplay_line_numbers_separator_character)
>   +      && it->paragraph_embedding != L2R)
>   +    {
>   +      tem_it.c = tem_it.char_to_display = XFIXNAT
> (Vdisplay_line_numbers_separator_character);
>
> That is, it treats any value of it->paragraph_embedding that is not
> L2R as if it were R2L.  But that is incorrect, because the usual value
> of it->paragraph_embedding is NEUTRAL_DIR, which corresponds to the
> nil value of bidi-paragraph-direction, the default.  That nil value
> means to determine the actual paragraph direction dynamically from the
> contents of the paragraph.  It is this nil value that presents the
> problem.  Your code treats that value as meaning right-to-left, but
> that is wrong.
>
> For example, all of the paragraphs on TUTORIAL.he are displayed
> right-to-left (because they include right-to-left text), but the last
> paragraph, the one which shows the file-local variables, is displayed
> left-to-right.  Try turning on this feature and look at that last part
> of the buffer to see if your code works correctly.
>
> > I was confused about the relationship between paragraph_embedding and
> bidi-paragraph_direction.
>
> bidi-paragraph-direction determines the value of it->paragraph_embedding.
> However, the default value of bidi-paragraph-direction is nil, and
> that is translated into NEUTRAL_DIR value of it->paragraph_embedding.
>
> > I'm
> > assuming you're saying there needs to be an additional check on the
> value of para_direction in the if
> > statements?
>
> I don't think you can base this on it->paragraph_embedding, because it
> doesn't reflect the actual paragraph direction on display when
> bidi-paragraph-direction is nil, the default.  The display code
> computes the actual direction of each screen line dynamically in that
> case, and stores it in the reversed_p flag of the glyph row.  The
> problem is that when the code generates the line number of the first
> screen line it needs to produce, that flag is not yet computed, it
> will only be computed when the first character on the screen line, the
> one after the line number, will be laid out.  And by that time, the
> line number was already generated.  This is the problem that needs
> fixing for this to work reliably with all scripts.
>
> Thanks.
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

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

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Wed, 15 Sep 2021 10:45:01 -0600
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> 
> So, in concept then, as the display-line-numbers code operates now adding a separator character that
> respects direction isn't possible because when maybe_produce_line_number is called the code doesn't yet
> know the direction of the text. The correct fix is to somehow have the function call for generating the line
> number glyphs after the buffer glyphs are computed... or to generate both L2R and R2L line numbers and
> then let the code decide what to display once reversed_p is decided.

Yes.

> This is verified by the fact that if I make a check on paragraph_direction instead of embedding, the first line
> number displays incorrectly because this flag has yet to be set. 

Exactly.

> Either way, I hate to admit it, but any solution to that problem is way beyond my skillset and you'd have to
> spend a lot of time checking/fixing any my work if I did make the attempt.

The idea I had, which is somewhat ugly, is to rearrange the glyphs in
the line-number part if the value of the reversed_p flag changes
between the time the line number was produced and the time the first
following glyph is produced in display_line.

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Michael Gallagher - NOAA Affiliate via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Ok. Correct me if I'm wrong, but this is extremely ugly in that it requires
a deep knowledge of how the glyphs are drawn and how move_it_by_lines
operates on the glyphs. Because, as it appears to me, once the line number
glyphs are sent to what I will call the "glyph queue" via PRODUCE_GLYPHS
you have very little (no?) information about what is responsible for those
glyphs? Actually. I don't fully understand where in the code the line
numbers get moved to the right side. Because it should be, conceptually,
easy to swap the last glyph to the first glyph if there is a separator
character at that point in the code. But when I "M-s o" for occurences of
R2L and then look for the place where the lnum glyphs are moved to the
right side, I can't find it.

On Wed, Sep 15, 2021 at 10:53 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> > Date: Wed, 15 Sep 2021 10:45:01 -0600
> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> >
> > So, in concept then, as the display-line-numbers code operates now
> adding a separator character that
> > respects direction isn't possible because when maybe_produce_line_number
> is called the code doesn't yet
> > know the direction of the text. The correct fix is to somehow have the
> function call for generating the line
> > number glyphs after the buffer glyphs are computed... or to generate
> both L2R and R2L line numbers and
> > then let the code decide what to display once reversed_p is decided.
>
> Yes.
>
> > This is verified by the fact that if I make a check on
> paragraph_direction instead of embedding, the first line
> > number displays incorrectly because this flag has yet to be set.
>
> Exactly.
>
> > Either way, I hate to admit it, but any solution to that problem is way
> beyond my skillset and you'd have to
> > spend a lot of time checking/fixing any my work if I did make the
> attempt.
>
> The idea I had, which is somewhat ugly, is to rearrange the glyphs in
> the line-number part if the value of the reversed_p flag changes
> between the time the line number was produced and the time the first
> following glyph is produced in display_line.
>


-- 
Michael Gallagher, PhD
CIRES Research Scientist
Polar Observations and Processes Team (ESRL/NOAA/PSD)
325 Broadway, Boulder, Colorado 80305

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

bug#50506: 28.0.50; display-line-numbers equivalent for linum-format?

[Eli Zaretskii]

> From: Michael Gallagher - NOAA Affiliate <michael.r.gallagher@noaa.gov>
> Date: Wed, 15 Sep 2021 11:32:17 -0600
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 50506@debbugs.gnu.org
> 
> Ok. Correct me if I'm wrong, but this is extremely ugly in that it requires a deep knowledge of how the glyphs
> are drawn and how move_it_by_lines operates on the glyphs.

move_it_by_lines is not relevant here.  The code to look at is
display_line and the few of the functions it calls.  display_line is
where the display engine generates screen lines, called "glyph rows".
A glyph row is just an array of 'struct glyph', or "glyphs" for short.
Each one of these glyphs describes a single "display element": a
character, an image, a stretch of whitespace, etc.  We are interested
in character glyphs here.

> Because, as it appears to me, once the line
> number glyphs are sent to what I will call the "glyph queue" via PRODUCE_GLYPHS you have very little
> (no?) information about what is responsible for those glyphs?

PRODUCE_GLYPHS is a macro that (on GUI frames) calls
gui_produce_glyphs.  It generates one glyph and adds it to the array
of glyphs of the glyph row.  At any point during the time display_line
works on a single glyph row (a.k.a. "screen line"), you can reach all
the glyphs produced upto that point by looking at the glyphs in the
it->glyph_row array indexed by their horizontal position.

A glyph row has 3 areas: 2 areas for the left and right margins, and
one area called TEXT_AREA, which is where the line numbers and the
text of the line are laid out.  So the i-th glyph in the text area of
the glyph row is it->glyph_row->glyphs[TEXT_AREA][i-1], and the last
glyph produced so far has its index in it->glyph_row->used[TEXT_AREA]-1.

> Actually. I don't fully understand where in the
> code the line numbers get moved to the right side. Because it should be, conceptually, easy to swap the last
> glyph to the first glyph if there is a separator character at that point in the code. But when I "M-s o" for
> occurences of R2L and then look for the place where the lnum glyphs are moved to the right side, I can't find
> it.  

When gui_produce_glyphs and its subroutines realize that the glyph row
has its reversed_p flag set, they start pushing (prepending) glyphs to
the glyphs already there, instead of appending them.  That's how the
line number winds up at the right edge of the screen line: it is
pushed there by the following glyphs when those are prepended.  Look
in append_glyph to see how this is done.

If what I told above and the code I pointed to leave something
unclear, please ask specific questions.