Re: Native line numbers, final testing

[Alex <agrambot@gmail.com>]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sat, 01 Jul 2017 23:16:11 -0600
>> > I couldn't (yet) find a way of doing that.
>> 
>> That's unfortunate; hopefully this can be fixed.
>
> I think I fixed that now.

The issue is a bit harder to encounter now, but it appears to still be
present. In xdisp.c:

M-g c 2970 RET
C-u 16 C-n

Then hit C-n a few times to reach line 87. The cursor will now be on
column 26 instead of column 27.

You can also test this by holding C-n or C-p and noticing the goal
column changing. This also affects C-v and M-v with
scroll-preserve-screen-position set to 'always.

>> > I firmly believe that line numbers should not be displayed on the
>> > margins, because that produces problems for packages that want to
>> > display stuff there.
>> 
>> That seems to me to be an extensibility problem rather than line numbers
>> not belonging in the margins. Without fixing that problem, then there
>> will still be problems when multiple packages attempt to use the margins
>> simultaneously.
>
> That's correct, but since coexistence in the margin is problematic,
> core features, especially popular ones, should not use the margins, so
> as not to exacerbate the problems.
>
> And note that displaying the numbers in the margin would not have
> solved the issue, since the width of the margins would still be
> estimated by the same heuristics.

So there's no reliable way to get the x-coordinate of the end of the
left margin/fringe? Why don't these issues affect nlinum, since it sets
the width dynamically?

>> That way, line numbers can have its own special area, and there will
>> be a clear x-coordinate for vertical-motion to start from, avoiding
>> a whole class of errors that you've had to deal with so far.
>
> See above: these problems won't be solved by going to the margin.  The
> only way to solve them is to have a fixed width for line numbers,
> something that can be done, if desired, by suitable setting of
> display-line-number-width.

That will definitely alleviate the issues, but won't completely solve
them on its own (plus, one shouldn't have to set that to avoid them).

On this note, I'd like to again ask for dynamic growing of the width,
but not shrinking. That should also help towards avoiding this problem
in growing buffers.

I've edited and attached my previous proof of concept, but it uses
Fmake_local_variable, which doesn't look like it's used a lot in the C
side of Emacs. Is there a better way to make buffer local internal
variables?


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

diff --git a/src/xdisp.c b/src/xdisp.c
index 47b8141463..14f070d487 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -20872,7 +20872,12 @@ maybe_produce_line_number (struct it *it)
   /* Compute the required width if needed.  */
   if (!it->lnum_width)
     {
-      if (NATNUMP (Vdisplay_line_number_width))
+      Lisp_Object cache = buffer_local_value (Qdisplay_line_number_width_cache,
+                                              it->w->contents);
+
+      if (NATNUMP (cache))
+        it->lnum_width = XFASTINT (cache);
+      else if (NATNUMP (Vdisplay_line_number_width))
 	it->lnum_width = XFASTINT (Vdisplay_line_number_width);
 
       /* Max line number to be displayed cannot be more than the one
@@ -20891,6 +20896,9 @@ maybe_produce_line_number (struct it *it)
 	max_lnum = this_line + it->w->desired_matrix->nrows - 1 - it->vpos;
       max_lnum = max (1, max_lnum);
       it->lnum_width = max (it->lnum_width, log10 (max_lnum) + 1);
+      if (display_line_numbers_grow_only)
+        Fset (Fmake_local_variable (Qdisplay_line_number_width_cache),
+              make_number (it->lnum_width));
       eassert (it->lnum_width > 0);
     }
   if (EQ (Vdisplay_line_numbers, Qrelative))
@@ -32594,6 +32602,18 @@ Any other value is treated as nil.  */);
   DEFSYM (Qdisplay_line_number_width, "display-line-number-width");
   Fmake_variable_buffer_local (Qdisplay_line_number_width);
 
+  DEFVAR_BOOL ("display-line-numbers-grow-only", display_line_numbers_grow_only,
+    doc: /* Non-nil means only dynamically grow the display,
+and never shrink. */);
+  display_line_numbers_grow_only = false;
+
+  DEFVAR_LISP ("display-line-number-width-cache", Vdisplay_line_number_width_cache,
+               doc: /* Stores the maximum line number width seen. */);
+  Vdisplay_line_number_width_cache = Qnil;
+  DEFSYM (Qdisplay_line_number_width_cache, "display-line-number-width-cache");
+  Fmake_variable_buffer_local (Qdisplay_line_number_width_cache);
+  Funintern (Qdisplay_line_number_width_cache, Qnil);
+
   DEFVAR_LISP ("display-line-numbers-current-absolute",
 	       Vdisplay_line_numbers_current_absolute,
     doc: /* Non-nil means display absolute number of current line.