* bug#28246: display line number width != length of line number at eob
2017-08-26 21:57 bug#28246: display line number width != length of line number at eob Keith David Bershatsky
@ 2017-08-27 14:23 ` Eli Zaretskii
2017-08-28 1:46 ` Keith David Bershatsky
2017-08-28 18:50 ` Keith David Bershatsky
2 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2017-08-27 14:23 UTC (permalink / raw)
To: Keith David Bershatsky; +Cc: 28246
> Date: Sat, 26 Aug 2017 14:57:56 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
>
> I would like to configure native line numbers to dynamically adjust the width
> (smaller/larger) so that it is equal to the length of the last line in the
> buffer.
AFAIU, this should be already supported by customizing to non-nil
values 2 options: display-line-numbers-width-start and
display-line-numbers-grow-only. If it doesn't work for you, please
show a reproducible recipe which exhibits the wrong or unexpected
behavior.
> Emacs is erroneously increasing the line number width before there are
> sufficient lines in the buffer to merit such an increase in width.
>
> Emacs fails to decrease the line number width when lines are removed from the
> buffer that merit a decrease in the width.
These are not errors, they are deliberate behavior. With the default
automatic adjustment of the width used for line-number display, Emacs
needs to estimate the largest line number to be displayed in the
window _before_ it actually displays all the lines in the window. The
estimation is a conservative one, and errs on the safe side, so that
the computed width is never too small, because that would produce a
horizontal shift of lines starting from some point in the middle of a
window.
This is done for speed, because the alternative would be to redisplay
the window twice, the first time to know what is the number of the
last visible line, the second time to actually display the numbers
using the right width. We would be then back at the slow operation of
linum.el, to say nothing of the fact that such double redisplay
disrupts the normal flow of redisplay.
I don't see why the way the current implementation works is a problem,
since the point in the buffer where the width is dynamically changed
is unimportant in practice, the only requirement is that it is never
too narrow. IOW, there's no error here, and nothing to fix.
> The desired behavior can be achieved with the Lisp code below AND by adding the
> following lines of code to maybe_produce_line_number just above the comment /*
> Compute the required width if needed. */
> /* example modification to achieve desired behavior */
> if (NATNUMP (Vdisplay_line_numbers_width)
> && !EQ (Vdisplay_line_numbers, Qrelative)
> && !EQ (Vdisplay_line_numbers, Qvisual))
> it->lnum_width = XFASTINT (Vdisplay_line_numbers_width);
I don't understand why you need this. it->lnum_width is calculated
once for each window, at the beginning of every redisplay, and there
should be no need to recompute it for each screen line, like your
proposal does, because the results will be identical. So the above is
a waste of cycles, AFAICT.
> I was unable to achieve the desired behavior by customizing Lisp variables such
> as display-line-numbers-grow-only and/or display-line-numbers-width-start.
What part(s) of the desired behavior you were unable to achieve?
> Here is the Lisp code that I am using:
This code will count lines in the buffer upon each command, which
could significantly slow down Emacs responsiveness in large buffers.
It will also increase the line-number display width when stuff is
added to the end of buffer even if that portion of the buffer is never
displayed, something that might surprise users. By contrast, the
existing facilities recount the line numbers only when needed, and
make a point of reusing the results of previous such calculation when
possible (compare your display-line-numbers--update-width-fn with
display-line-numbers-update-width in display-line-numbers.el).
So while it is okay to have this in your personal customizations, I'm
not sure others will want this behavior, and it certainly cannot be
the default.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#28246: display line number width != length of line number at eob
2017-08-26 21:57 bug#28246: display line number width != length of line number at eob Keith David Bershatsky
2017-08-27 14:23 ` Eli Zaretskii
@ 2017-08-28 1:46 ` Keith David Bershatsky
2017-08-28 17:27 ` Eli Zaretskii
2017-08-28 18:50 ` Keith David Bershatsky
2 siblings, 1 reply; 6+ messages in thread
From: Keith David Bershatsky @ 2017-08-28 1:46 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 28246
Thank you, Eli, for reviewing #28246 and for explaining that automatic adjustment of the width for line-number display is *estimated* based upon certain criteria.
I do not know whether any other Emacs users would appreciate *precision* width based upon the length of the last line in the buffer.
Inasmuch as I was hoping to achieve *precision* width every command loop, display-line-numbers-width-start and/or display-line-numbers-grow-only would not suffice in that regard. I am uncertain why it->lnum_width is *not* always equal to
(save-excursion
(goto-char (point-max))
(length (format-mode-line "%l"))
The difference between it->lnum_width and the Lisp code above can be seen with messages in the echo area after evaluating the code in the opening bug report for #28246, and thereafter manually adding/removing lines from the buffer.
linum.el is slow in large buffers primarily because it uses count-lines. In an emacs.stackexchange.com thread entitled "A faster method to obtain `line-number-at-pos` in large buffers" -- https://emacs.stackexchange.com/q/3821/2287 -- I learned that format-mode-line with the "%l" argument can be used to greatly enhance speed versus using count-lines. However, the window must be visible and some other limitations apply as discussed in the comments of the SE thread.
After reading your comments (a few times to let it all sink in), I understand your concern about unnecessarily calculating the width each time that maybe_produce_line_number is called. In the example that was used at the outset of #28246, the width is calculated only once per command loop and stored in display_line_numbers_width.
The following example function is a condensed version of what the mode-line code uses to determine the line number, which will be substantially faster than count-lines and/or line-number-at-pos. In addition, I found a little snippet on stackoverflow.com to get the length of an integer. Inasmuch as maybe_produce_line_number may be called more than one time each command loop, a different location needs to be found for internal_line_number_at_position -- so that it gets called just for each window per command loop -- and that value would need to be stored and subsequently used. I understand that internal_line_number_at_position may not be well written, but it is a working proof concept to demonstrate where I am coming from.
/* Length of an Integer: https://stackoverflow.com/a/3068420/2112489
The log10, abs, and floor functions are provided by math.h */
TARGET_WIDTH = floor (log10 (abs (internal_line_number_at_position (XWINDOW (selected_window), ZV)))) + 1;
static ptrdiff_t
internal_line_number_at_position (struct window *w, ptrdiff_t pos)
{
Lisp_Object buf, value;
struct buffer *b;
struct buffer *old_buffer = NULL;
buf = w->contents;
CHECK_BUFFER (buf);
b = XBUFFER (buf);
ptrdiff_t line_number;
if (b != current_buffer)
{
old_buffer = current_buffer;
set_buffer_internal (b);
}
ptrdiff_t startpos, startpos_byte, line, linepos, linepos_byte,
topline, nlines, height, junk, opoint;
opoint = PT;
if (opoint != pos)
SET_PT (pos);
startpos = marker_position (w->start);
startpos_byte = marker_byte_position (w->start);
/* If we decided that this buffer isn't suitable for line numbers, don't forget that too fast. */
if (MINI_WINDOW_P (w))
{
line_number = 0;
goto done;
}
if (w->base_line_pos == -1)
{
line_number = 0;
goto done;
}
/* If the buffer is very big, don't waste time. */
if (INTEGERP (Vline_number_display_limit)
&& BUF_ZV (b) - BUF_BEGV (b) > XINT (Vline_number_display_limit))
{
w->base_line_pos = 0;
w->base_line_number = 0;
line_number = 0;
goto done;
}
if (w->base_line_number > 0
&& w->base_line_pos > 0
&& w->base_line_pos <= startpos)
{
line = w->base_line_number;
linepos = w->base_line_pos;
linepos_byte = buf_charpos_to_bytepos (b, linepos);
}
else
{
line = 1;
linepos = BUF_BEGV (b);
linepos_byte = BUF_BEGV_BYTE (b);
}
/* Count lines from base line to window start position. */
nlines = display_count_lines (linepos_byte, startpos_byte, startpos, &junk);
topline = nlines + line;
/* Determine a new base line, if the old one is too close
or too far away, or if we did not have one.
"Too close" means it's plausible a scroll-down would go back past it. */
if (startpos == BUF_BEGV (b))
{
w->base_line_number = topline;
w->base_line_pos = BUF_BEGV (b);
}
else if (nlines < height + 25 || nlines > height * 3 + 50 || linepos == BUF_BEGV (b))
{
ptrdiff_t limit = BUF_BEGV (b);
ptrdiff_t limit_byte = BUF_BEGV_BYTE (b);
ptrdiff_t position;
ptrdiff_t distance = (height * 2 + 30) * line_number_display_limit_width;
if (startpos - distance > limit)
{
limit = startpos - distance;
limit_byte = CHAR_TO_BYTE (limit);
}
nlines = display_count_lines (startpos_byte, limit_byte, - (height * 2 + 30), &position);
/* If we couldn't find the lines we wanted within
line_number_display_limit_width chars per line,
give up on line numbers for this window. */
if (position == limit_byte && limit == startpos - distance)
{
w->base_line_pos = -1;
w->base_line_number = 0;
line_number = 0;
goto done;
}
w->base_line_number = topline - nlines;
w->base_line_pos = BYTE_TO_CHAR (position);
}
/* Now count lines from the start pos to point. */
nlines = display_count_lines (startpos_byte, PT_BYTE, PT, &junk);
if (opoint != pos)
SET_PT (opoint);
line_number = topline + nlines;
done:
if (old_buffer)
set_buffer_internal (old_buffer);
return line_number;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#28246: display line number width != length of line number at eob
2017-08-28 1:46 ` Keith David Bershatsky
@ 2017-08-28 17:27 ` Eli Zaretskii
0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2017-08-28 17:27 UTC (permalink / raw)
To: Keith David Bershatsky; +Cc: 28246
> Date: Sun, 27 Aug 2017 18:46:42 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
> Cc: 28246@debbugs.gnu.org
>
> I do not know whether any other Emacs users would appreciate *precision* width based upon the length of the last line in the buffer.
You are the first to raise this issue, and frankly I still don't
understand why it's important to you.
> Inasmuch as I was hoping to achieve *precision* width every command loop, display-line-numbers-width-start and/or display-line-numbers-grow-only would not suffice in that regard. I am uncertain why it->lnum_width is *not* always equal to
>
> (save-excursion
> (goto-char (point-max))
> (length (format-mode-line "%l"))
Because we only make the line-number display as wide as needed for
line numbers seen so far, while the Lisp above wants to make it as
wide as required by the last line in the buffer. You can get the same
with the above customization variables if you visit the bottom of the
buffer just once.
> linum.el is slow in large buffers primarily because it uses count-lines. In an emacs.stackexchange.com thread entitled "A faster method to obtain `line-number-at-pos` in large buffers" -- https://emacs.stackexchange.com/q/3821/2287 -- I learned that format-mode-line with the "%l" argument can be used to greatly enhance speed versus using count-lines. However, the window must be visible and some other limitations apply as discussed in the comments of the SE thread.
As that discussion reveals, this method is not 100% reliable in
practice.
Anyway, is there anything else to do in this bug report, or can we
close it?
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#28246: display line number width != length of line number at eob
2017-08-26 21:57 bug#28246: display line number width != length of line number at eob Keith David Bershatsky
2017-08-27 14:23 ` Eli Zaretskii
2017-08-28 1:46 ` Keith David Bershatsky
@ 2017-08-28 18:50 ` Keith David Bershatsky
2017-08-29 15:00 ` Eli Zaretskii
2 siblings, 1 reply; 6+ messages in thread
From: Keith David Bershatsky @ 2017-08-28 18:50 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 28246
Thank you, Eli, for having taken the time to explain the fundamentals of how Emacs calculates the width for display of native line numbers.
I have been using the point-max approach (each command loop) in my own setup since I first started using Emacs a few years ago (with a variation of linum.el for just the visible window), and I became accustomed to seeing the width decrease/increase accordingly. I instantly noticed the difference with native line numbers, and my OCDC went into action looking for a fix. :)
I created a Qprecision symbol to be used with Vdisplay_line_numbers_width, and I can turn the precision width feature on/off. The revised snippet is listed below in the event that anyone is interested in this thread in the future.
Thank you again for all your help and explanations. At your convenience, please feel free to close out bug #28246.
Keith
/* @lawlist modification -- precision width for line numbers. */
struct window *w = decode_live_window (selected_window);
Lisp_Object buf = w->contents;
CHECK_BUFFER (buf);
struct buffer *b = XBUFFER (buf);
if (!it->lnum_width
&& EQ (Vdisplay_line_numbers_width, Qprecision)
&& !EQ (Vdisplay_line_numbers, Qrelative)
&& !EQ (Vdisplay_line_numbers, Qvisual)
&& !MINI_WINDOW_P (XWINDOW (selected_window))
&& BUF_BEG (b) <= ZV
&& ZV <= BUF_Z (b))
{
/* Length of an Integer: https://stackoverflow.com/a/3068420/2112489
The log10, abs, and floor functions are provided by math.h */
it->lnum_width = floor (log10 (abs (internal_line_number_at_position (w, ZV)))) + 1;
eassert (it->lnum_width > 0);
}
/* Compute the required width if needed. */
else if (!it->lnum_width)
{
if (NATNUMP (Vdisplay_line_numbers_width))
it->lnum_width = XFASTINT (Vdisplay_line_numbers_width);
/* Max line number to be displayed cannot be more than the one
corresponding to the last row of the desired matrix. */
ptrdiff_t max_lnum;
if (NILP (Vdisplay_line_numbers_current_absolute)
&& (EQ (Vdisplay_line_numbers, Qrelative)
|| EQ (Vdisplay_line_numbers, Qvisual)))
/* We subtract one more because the current line is always zero in this mode. */
max_lnum = it->w->desired_matrix->nrows - 2;
else if (EQ (Vdisplay_line_numbers, Qvisual))
max_lnum = it->pt_lnum + it->w->desired_matrix->nrows - 1;
else
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);
eassert (it->lnum_width > 0);
}
^ permalink raw reply [flat|nested] 6+ messages in thread