Native line numbers, final testing

Native line numbers, final testing

[Eli Zaretskii]

I've finished development of this feature and pushed the changes to
the scratch/line-numbers branch.  "Visual" line numbers are
implemented, and all the known bugs should be fixed.  I also added
documentation.

Please test and report any problems you find.  If no grave bugs show
up, I will probably land this on master in a few days.

Thanks to everybody who sent feedback and reported problems and
feature requests.

Re: Native line numbers, final testing

[Alex]

There's an issue with relative line numbers with a non-nil
`display-line-numbers-current-absolute'.

If the current line number is at least 4 digits long, then the text in
the current line is shifted over.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 30 Jun 2017 11:51:55 -0600
> 
> There's an issue with relative line numbers with a non-nil
> `display-line-numbers-current-absolute'.
> 
> If the current line number is at least 4 digits long, then the text in
> the current line is shifted over.

Thanks, but I don't understand what you mean, and I tried one file
with 5-digit line number and didn't see what I think you described.

Can you show a screenshot?  Are you using some special font where this
happens?

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 30 Jun 2017 11:51:55 -0600
>> 
>> There's an issue with relative line numbers with a non-nil
>> `display-line-numbers-current-absolute'.
>> 
>> If the current line number is at least 4 digits long, then the text in
>> the current line is shifted over.
>
> Thanks, but I don't understand what you mean, and I tried one file
> with 5-digit line number and didn't see what I think you described.
>
> Can you show a screenshot?  Are you using some special font where this
> happens?

I forgot that just before testing relative line numbers, I set
`display-line-number-width' to a non-nil value. For immediate results,
try setting it to 0 and hit C-n/C-p a few times.

P.S. I noticed that with display-line-numbers set to 'visual, goto-line
is much slower compared to 'relative. Is this expected? M-g 25000 in
xdisp.c with 'relative is almost insant for me, but quite a few seconds
(the first time) with 'visual. This is in emacs -Q.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 30 Jun 2017 13:06:27 -0600
> 
> >> If the current line number is at least 4 digits long, then the text in
> >> the current line is shifted over.
> >
> > Thanks, but I don't understand what you mean, and I tried one file
> > with 5-digit line number and didn't see what I think you described.
> >
> > Can you show a screenshot?  Are you using some special font where this
> > happens?
> 
> I forgot that just before testing relative line numbers, I set
> `display-line-number-width' to a non-nil value. For immediate results,
> try setting it to 0 and hit C-n/C-p a few times.

Ah, okay.  Thanks, should be fixed now.

(Btw zero is not valid as the value of display-line-number-width,
although the code before the last changes mistakenly tried using it.
Starting from now, zero and any other non-positive value are
interpreted as nil.)

> P.S. I noticed that with display-line-numbers set to 'visual, goto-line
> is much slower compared to 'relative. Is this expected?

Yes.  The initial naïve implementation was unbearably slow, but I was
lucky to find a shortcut.  It is still sometimes slow, and a few
redisplay optimizations are disabled with this mode, because moving
point vertically needs to redisplay the entire window, since the line
numbers change.  If someone has ideas for speeding it up, I'm all
ears.

At least it is significantly faster than the only add-on package which
implemented something similar to visual mode.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 30 Jun 2017 13:06:27 -0600
>>
>> I forgot that just before testing relative line numbers, I set
>> `display-line-number-width' to a non-nil value. For immediate results,
>> try setting it to 0 and hit C-n/C-p a few times.
>
> Ah, okay.  Thanks, should be fixed now.

Thanks, it appears to be fixed.

I noticed another bug. With `display-line-numbers' set to t or 'relative
in xdisp.c:

M-g c 2970 RET
C-n

Notice that C-n moved back one column. This does not happen with
'visual, nor does it happen when the window is sufficiently small. This
is in emacs -Q, and only happens with non-nil line-move-visual.

>> P.S. I noticed that with display-line-numbers set to 'visual, goto-line
>> is much slower compared to 'relative. Is this expected?
>
> Yes.  The initial naïve implementation was unbearably slow, but I was
> lucky to find a shortcut.  It is still sometimes slow, and a few
> redisplay optimizations are disabled with this mode, because moving
> point vertically needs to redisplay the entire window, since the line
> numbers change.  If someone has ideas for speeding it up, I'm all
> ears.

I'm not planning to use 'visual, and I of course don't claim to know
anything about this, but I'm curious as to why it works like this.
goto-line in either style always goes to the same absolute line number,
right? Does the line number display calculation occur after the point is
already moved to this line? If so, then doesn't the display engine just
need to calculate the visual line numbers around this point? If so, then
why does a large jump like M-g g 25000 take so much longer comparatively
than M-g g 1000? If these assumptions are wrong (they likely are), why
are they wrong?

Re: Native line numbers, final testing

[Filipe Silva]

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

Awesome Eli! I should be able to perform testing tomorrow

On Fri, Jun 30, 2017 at 11:49 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> I've finished development of this feature and pushed the changes to
> the scratch/line-numbers branch.  "Visual" line numbers are
> implemented, and all the known bugs should be fixed.  I also added
> documentation.
>
> Please test and report any problems you find.  If no grave bugs show
> up, I will probably land this on master in a few days.
>
> Thanks to everybody who sent feedback and reported problems and
> feature requests.
>
>

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

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 30 Jun 2017 15:15:00 -0600
> 
> I noticed another bug. With `display-line-numbers' set to t or 'relative
> in xdisp.c:
> 
> M-g c 2970 RET
> C-n
> 
> Notice that C-n moved back one column. This does not happen with
> 'visual, nor does it happen when the window is sufficiently small. This
> is in emacs -Q, and only happens with non-nil line-move-visual.

Such problems (which IMO are minor and quite rare) cannot be helped,
unfortunately.  They happen near the point where the line numbers will
soon need one more or one less column.

The underlying issue is how this feature calculates the number of
columns required for displaying all the numbers in the visible portion
of a window.  The naïve method would be to perform the window display
twice, because you only know the largest line number in view after
actually displaying all the lines.  But that is too slow, and would
land us right back at the slow operation of linum-mode and friends.

So instead we guess the maximum line number without actually counting
the lines in the window.  That guess must be conservative, otherwise
you'll see the text shift to the right in the middle of a window.
Therefore the guess uses the maximum number of screen lines a window
could possibly display without resizing, by using the number of rows
in the underlying display structure called the "glyph matrix".  This
assumes that (1) all of the glyph-matrix rows will be used (usually
false, except on TTYs), and (2) that all the shown lines are
non-continuation lines.

This is why many times you see the number of columns grow before the
additional column is actually needed by the last line number displayed
at window's bottom.

The same code, and the same guessing, is also used by various
functions that simulate display, such as vertical-motion (which is
called internally by C-n).  Except that the vertical position of point
in the virtual "window" they consider is taken as zero, because in
general it cannot be known (the function could be called in the middle
of a Lisp program that moved point, and the updated vertical position
will not be determined until the next redisplay cycle).  So the
resulting number of columns these functions compute could be different
from what actually is, or will be, on the screen.  And therefore
column calculations under line-move-visual will err by the same
amount.

IOW, these issues are a (small, IMO) fine we need to pay for having
such fast line-number display.

I will keep this issue in mind, in case some idea emerges for how to
fix them, thanks for pointing it out.

> >> P.S. I noticed that with display-line-numbers set to 'visual, goto-line
> >> is much slower compared to 'relative. Is this expected?
> >
> > Yes.  The initial naïve implementation was unbearably slow, but I was
> > lucky to find a shortcut.  It is still sometimes slow, and a few
> > redisplay optimizations are disabled with this mode, because moving
> > point vertically needs to redisplay the entire window, since the line
> > numbers change.  If someone has ideas for speeding it up, I'm all
> > ears.
> 
> I'm not planning to use 'visual, and I of course don't claim to know
> anything about this, but I'm curious as to why it works like this.
> goto-line in either style always goes to the same absolute line number,
> right? Does the line number display calculation occur after the point is
> already moved to this line?

Yes.  Line numbers are calculated as part of redisplay, and that
happens when point already moved.

> If so, then doesn't the display engine just need to calculate the
> visual line numbers around this point? If so, then why does a large
> jump like M-g g 25000 take so much longer comparatively than M-g g
> 1000? If these assumptions are wrong (they likely are), why are they
> wrong?

Because the display engine employs a lot of optimizations, to try and
shortcut expensive redrawing of the entire window, and to comply with
user expectations.  It doesn't, and cannot, know that buffer position
25000 is necessarily outside the window where the command was invoked.
E.g., you could have some text-folding mode, such as Outline, active
in the buffer; or you could have lines truncated with only a few very
long lines between the window-start and position 25000.

So redisplay first tries to reuse the same window-start point as it
used in the previous cycle, because that's what you as a user would
expect: if the new point is inside the same window, you don't expect
the window to scroll.  Deciding whether the previous window-start is
still usable needs to check whether point will be shown inside the
window, and that requires to try reaching point from the original
window-start, in this case position 1, and counting screen lines as we
do, until we either hit point or the window's bottom.

It turns out that the 'visual' mode of line numbers made this test
unnecessarily slow, so a simple heuristic was enough to avoid that.
See the latest branch, where 'visual' and 'relative' should now
redisplay at the same speed in this case.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 30 Jun 2017 15:15:00 -0600
>> 
>> I noticed another bug. With `display-line-numbers' set to t or 'relative
>> in xdisp.c:
>> 
>> M-g c 2970 RET
>> C-n
>> 
>> Notice that C-n moved back one column. This does not happen with
>> 'visual, nor does it happen when the window is sufficiently small. This
>> is in emacs -Q, and only happens with non-nil line-move-visual.
>
> Such problems (which IMO are minor and quite rare) cannot be helped,
> unfortunately.  They happen near the point where the line numbers will
> soon need one more or one less column.
>
> The underlying issue is how this feature calculates the number of
> columns required for displaying all the numbers in the visible portion
> of a window.  The naïve method would be to perform the window display
> twice, because you only know the largest line number in view after
> actually displaying all the lines.  But that is too slow, and would
> land us right back at the slow operation of linum-mode and friends.
>
> So instead we guess the maximum line number without actually counting
> the lines in the window.  That guess must be conservative, otherwise
> you'll see the text shift to the right in the middle of a window.
> Therefore the guess uses the maximum number of screen lines a window
> could possibly display without resizing, by using the number of rows
> in the underlying display structure called the "glyph matrix".  This
> assumes that (1) all of the glyph-matrix rows will be used (usually
> false, except on TTYs), and (2) that all the shown lines are
> non-continuation lines.
>
> This is why many times you see the number of columns grow before the
> additional column is actually needed by the last line number displayed
> at window's bottom.
>
> The same code, and the same guessing, is also used by various
> functions that simulate display, such as vertical-motion (which is
> called internally by C-n).  Except that the vertical position of point
> in the virtual "window" they consider is taken as zero, because in
> general it cannot be known (the function could be called in the middle
> of a Lisp program that moved point, and the updated vertical position
> will not be determined until the next redisplay cycle).  So the
> resulting number of columns these functions compute could be different
> from what actually is, or will be, on the screen.  And therefore
> column calculations under line-move-visual will err by the same
> amount.
>
> IOW, these issues are a (small, IMO) fine we need to pay for having
> such fast line-number display.
>
> I will keep this issue in mind, in case some idea emerges for how to
> fix them, thanks for pointing it out.

Thanks for explaining, but I disagree on the severity of this bug. It is
relatively rare, but navigating a buffer with C-n/C-p just before the
line 100/1000 isn't exactly uncommon. I believe consistency of C-n/C-p,
at least with the same size window, is quite important. Imagine if
C-f/C-b also went up/down a line if the column number was, e.g., "a bit"
before 10^k, for integers k > 1. That would break users' expectations of
these commands considerably, even if, with the exception of the column
"a bit" before 100, these column positions are rare. This is worsened by
the fact that "a bit" is dependent on the window size, meaning it's
harder to reason about exactly.

One could argue that if you like consistency, then you should set
line-move-visual to nil. I suppose that's true in some cases, but that
doesn't mean that a non-nil value should just ignore consistency if it
can somehow be preserved. Also, visual-line-mode is helpful for some
types of documents (e.g. LaTeX), and it's natural to have
line-move-visual be non-nil in this case. It would be a shame to have to
choose between native display line numbers and visual-line-mode in those
documents.

If no good solution can be found, then I think even a slowdown for
C-n/C-p would be acceptable if it meant a fix for this issue.

I have a couple more questions if that's alright:

It's not intuitive to me why this occurs between lines with the same
display line number width. If vertical-motion uses the same heuristic as
display of line numbers, then why is the column changing between lines
even when the width of line numbers isn't? Is it because it's using
the heuristic with different inputs? If so, can't they be modified to
achieve the same results as the display of line numbers?

Why is it necessary for line numbers to actually affect the vertical
position of characters in the buffer? I suppose it's a bit late to be
asking this question, but the approach from an outside view feels odd. I
don't know what the options were, but it's odd that line numbers aren't
in their own special area like in (n)linum. Does the display engine not
work well with margins?

>> I'm not planning to use 'visual, and I of course don't claim to know
>> anything about this, but I'm curious as to why it works like this.
>> goto-line in either style always goes to the same absolute line number,
>> right? Does the line number display calculation occur after the point is
>> already moved to this line?
>
> Yes.  Line numbers are calculated as part of redisplay, and that
> happens when point already moved.

>> If so, then doesn't the display engine just need to calculate the
>> visual line numbers around this point? If so, then why does a large
>> jump like M-g g 25000 take so much longer comparatively than M-g g
>> 1000? If these assumptions are wrong (they likely are), why are they
>> wrong?
>
> Because the display engine employs a lot of optimizations, to try and
> shortcut expensive redrawing of the entire window, and to comply with
> user expectations.  It doesn't, and cannot, know that buffer position
> 25000 is necessarily outside the window where the command was invoked.
> E.g., you could have some text-folding mode, such as Outline, active
> in the buffer; or you could have lines truncated with only a few very
> long lines between the window-start and position 25000.
>
> So redisplay first tries to reuse the same window-start point as it
> used in the previous cycle, because that's what you as a user would
> expect: if the new point is inside the same window, you don't expect
> the window to scroll.  Deciding whether the previous window-start is
> still usable needs to check whether point will be shown inside the
> window, and that requires to try reaching point from the original
> window-start, in this case position 1, and counting screen lines as we
> do, until we either hit point or the window's bottom.
>
> It turns out that the 'visual' mode of line numbers made this test
> unnecessarily slow, so a simple heuristic was enough to avoid that.
> See the latest branch, where 'visual' and 'relative' should now
> redisplay at the same speed in this case.

Thanks for the explanation and fix. The heuristic does indeed make it a
lot faster.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 01 Jul 2017 15:00:50 -0600
> 
> It's not intuitive to me why this occurs between lines with the same
> display line number width. If vertical-motion uses the same heuristic as
> display of line numbers, then why is the column changing between lines
> even when the width of line numbers isn't?

Because vertical-motion thinks the display uses N+1 columns whereas it
actually uses only N.

> Is it because it's using the heuristic with different inputs?

Yes.  The important input is the actual window-start point.

> If so, can't they be modified to achieve the same results as the
> display of line numbers?

I couldn't (yet) find a way of doing that.

> Why is it necessary for line numbers to actually affect the vertical
> position of characters in the buffer?

Because vertical-motion does its column calculation in pixels, C-n/C-p
need to adjust the calculations due to the pixels occupied by line
numbers.

> I suppose it's a bit late to be asking this question, but the
> approach from an outside view feels odd. I don't know what the
> options were, but it's odd that line numbers aren't in their own
> special area like in (n)linum. Does the display engine not work well
> with margins?

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.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sat, 01 Jul 2017 15:00:50 -0600
>> 
>> It's not intuitive to me why this occurs between lines with the same
>> display line number width. If vertical-motion uses the same heuristic as
>> display of line numbers, then why is the column changing between lines
>> even when the width of line numbers isn't?
>
> Because vertical-motion thinks the display uses N+1 columns whereas it
> actually uses only N.
>
>> Is it because it's using the heuristic with different inputs?
>
> Yes.  The important input is the actual window-start point.
>
>> If so, can't they be modified to achieve the same results as the
>> display of line numbers?
>
> I couldn't (yet) find a way of doing that.

That's unfortunate; hopefully this can be fixed.

>> Why is it necessary for line numbers to actually affect the vertical
>> position of characters in the buffer?
>
> Because vertical-motion does its column calculation in pixels, C-n/C-p
> need to adjust the calculations due to the pixels occupied by line
> numbers.
>
>> I suppose it's a bit late to be asking this question, but the
>> approach from an outside view feels odd. I don't know what the
>> options were, but it's odd that line numbers aren't in their own
>> special area like in (n)linum. Does the display engine not work well
>> with margins?
>
> 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.

Can there not be multiple margins on one side? 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.

If not, why? That seems like a clear win to me, if it's possible. As a
bonus, it would allow for users to customize the positions of elements
in the margins (and fringes, hopefully).

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 01 Jul 2017 23:16:11 -0600
> 
> > Because vertical-motion thinks the display uses N+1 columns whereas it
> > actually uses only N.
> >
> >> Is it because it's using the heuristic with different inputs?
> >
> > Yes.  The important input is the actual window-start point.
> >
> >> If so, can't they be modified to achieve the same results as the
> >> display of line numbers?
> >
> > I couldn't (yet) find a way of doing that.
> 
> That's unfortunate; hopefully this can be fixed.

I think I fixed that now.

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

> Can there not be multiple margins on one side?

No, not without a thorough redesign and rewrite of the display code.
The fact that there's at most one margin on either side of the text
area is hard-coded in many places.

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

Re: Native line numbers, final testing

[Stefan Monnier]

>> Can there not be multiple margins on one side?
> No, not without a thorough redesign and rewrite of the display code.
> The fact that there's at most one margin on either side of the text
> area is hard-coded in many places.

FWIW, the issue of virtualizing the margins is one that would be good to
solve (better than to circumvent the problem every time we hit it).


        Stefan

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 02 Jul 2017 12:47:41 -0400
> 
> FWIW, the issue of virtualizing the margins is one that would be good to
> solve (better than to circumvent the problem every time we hit it).

What do you mean by "virtualizing the margins"?

Re: Native line numbers, final testing

[Stefan Monnier]

>> FWIW, the issue of virtualizing the margins is one that would be good to
>> solve (better than to circumvent the problem every time we hit it).
> What do you mean by "virtualizing the margins"?

Provide some way to share access to the one resource (the margin [OK,
there are two, but that doesn't change the argument very much]).
In OSes, this is usually solved by virtualization.


        Stefan

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 02 Jul 2017 13:38:12 -0400
> 
> >> FWIW, the issue of virtualizing the margins is one that would be good to
> >> solve (better than to circumvent the problem every time we hit it).
> > What do you mean by "virtualizing the margins"?
> 
> Provide some way to share access to the one resource (the margin [OK,
> there are two, but that doesn't change the argument very much]).

Sure, if someone figures a good way of doing that, and then actually
does it... should be a Good Thing.

Re: Native line numbers, final testing

[James Nguyen]

Eli Zaretskii <eliz@gnu.org> writes:

> I've finished development of this feature and pushed the changes to
> the scratch/line-numbers branch.  "Visual" line numbers are
> implemented, and all the known bugs should be fixed.  I also added
> documentation.
>
> Please test and report any problems you find.  If no grave bugs show
> up, I will probably land this on master in a few days.
>
> Thanks to everybody who sent feedback and reported problems and
> feature requests.

Pulling the latest changes this morning (I believe I was 11-13 commits
behind before I pulled.)

This breaks a crucial setting on evil-mode.

Setting this variable.

(require 'evil)
(setq evil-move-beyond-eol t)
(evil-mode)

Documentation: (for evil-move-beyond-eol)
Whether the cursor is allowed to move past the last character of a line.

Go to any line. Press $ in 'normal' mode. It will go to the end of the
line past the last character in the line. Trying to go backwards now
will error 'args out of range'.

For context, that variable is very important to get to cursor point to
evaluate the last s-expression properly. Sorry if that's a little vague.

Turning off display-line-numbers makes it work as expected.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: James Nguyen <james@jojojames.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 02 Jul 2017 12:27:30 -0700
> 
> This breaks a crucial setting on evil-mode.
> 
> Setting this variable.
> 
> (require 'evil)
> (setq evil-move-beyond-eol t)
> (evil-mode)
> 
> Documentation: (for evil-move-beyond-eol)
> Whether the cursor is allowed to move past the last character of a line.
> 
> Go to any line. Press $ in 'normal' mode. It will go to the end of the
> line past the last character in the line. Trying to go backwards now
> will error 'args out of range'.
> 
> For context, that variable is very important to get to cursor point to
> evaluate the last s-expression properly. Sorry if that's a little vague.
> 
> Turning off display-line-numbers makes it work as expected.

Are you sure this isn't a bug in evil-mode?  Does this feature behave
correctly in a buffer with a non-nil line-prefix?

Can you show a backtrace from the args out of range error?

Re: Native line numbers, final testing

[James Nguyen]

Eli Zaretskii <eliz@gnu.org> writes:

> Are you sure this isn't a bug in evil-mode?  Does this feature behave
> correctly in a buffer with a non-nil line-prefix?
>
> Can you show a backtrace from the args out of range error?

I haven't been keeping track of the line-prefix conversations around
this feature but setting it and then trying to do same thing (as
reported above) works in previous version of this line number branch.

(setq line-prefix "1")
16d2695674 * @ Minor change in NEWS.
^ This combination works.

Here's the trace on the latest commit.
4c9353a584 * scratch/line-numbers origin/scratch/line-numbers

Debugger entered--Lisp error: (args-out-of-range 1)
  signal(args-out-of-range (1))
  (condition-case err (let* ((range (evil-expand (point) (point) (quote line))) (beg (evil-range-beginning range)) (end (evil-range-end range)) (min (point-min)) (max (point-max))) (if (save-excursion (goto-char end) (bolp)) (progn (setq end (max beg (1- end))))) (if (and evil-move-cursor-back (not evil-move-beyond-eol) (not (evil-visual-state-p)) (not (evil-operator-state-p))) (progn (setq end (max beg (1- end))))) (save-restriction (let ((evil-restriction-stack (cons (cons ... ...) evil-restriction-stack))) (evil-narrow beg end) (let ((p (point))) (condition-case err (progn (condition-case err ... ... ...)) (error (if ... ...))))))) (error (setq this-command last-command) (signal (car err) (cdr err))))
  (let ((evil-restriction-stack (cons (cons (point-min) (point-max)) evil-restriction-stack))) (evil-narrow (save-excursion (evil-backward-char (1+ (or count 1)) t t) (point)) (1+ (point))) (condition-case err (let* ((range (evil-expand (point) (point) (quote line))) (beg (evil-range-beginning range)) (end (evil-range-end range)) (min (point-min)) (max (point-max))) (if (save-excursion (goto-char end) (bolp)) (progn (setq end (max beg (1- end))))) (if (and evil-move-cursor-back (not evil-move-beyond-eol) (not (evil-visual-state-p)) (not (evil-operator-state-p))) (progn (setq end (max beg (1- end))))) (save-restriction (let ((evil-restriction-stack (cons ... evil-restriction-stack))) (evil-narrow beg end) (let ((p ...)) (condition-case err (progn ...) (error ...)))))) (error (setq this-comm
 and last-command) (signal (car err) (cdr err)))))
  (save-restriction (let ((evil-restriction-stack (cons (cons (point-min) (point-max)) evil-restriction-stack))) (evil-narrow (save-excursion (evil-backward-char (1+ (or count 1)) t t) (point)) (1+ (point))) (condition-case err (let* ((range (evil-expand (point) (point) (quote line))) (beg (evil-range-beginning range)) (end (evil-range-end range)) (min (point-min)) (max (point-max))) (if (save-excursion (goto-char end) (bolp)) (progn (setq end (max beg ...)))) (if (and evil-move-cursor-back (not evil-move-beyond-eol) (not (evil-visual-state-p)) (not (evil-operator-state-p))) (progn (setq end (max beg ...)))) (save-restriction (let ((evil-restriction-stack ...)) (evil-narrow beg end) (let (...) (condition-case err ... ...))))) (error (setq this-command last-command) (signal (car err) (cdr e
 rr))))))
  (cond (noerror (condition-case nil (evil-backward-char count crosslines nil) (error nil))) ((not crosslines) (save-restriction (let ((evil-restriction-stack (cons (cons ... ...) evil-restriction-stack))) (evil-narrow (save-excursion (evil-backward-char (1+ ...) t t) (point)) (1+ (point))) (condition-case err (let* ((range ...) (beg ...) (end ...) (min ...) (max ...)) (if (save-excursion ... ...) (progn ...)) (if (and evil-move-cursor-back ... ... ...) (progn ...)) (save-restriction (let ... ... ...))) (error (setq this-command last-command) (signal (car err) (cdr err))))))) (t (let* ((countvar (or count 1)) (unitvar (if (< countvar 0) -1 1))) (catch (quote donevar) (let ((loopvar countvar)) (setq loopvar loopvar) (while (/= loopvar 0) (let (...) (backward-char) (if ... nil ...) (if ... .
 ..)) (if (> loopvar 0) (setq loopvar ...) (setq loopvar ...)) (setq loopvar loopvar)) loopvar)))))
  evil-backward-char(nil nil nil)
  funcall-interactively(evil-backward-char nil nil nil)
  call-interactively(evil-backward-char nil nil)
  command-execute(evil-backward-char)

Re: Native line numbers, final testing

[Alex]

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

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 02 Jul 2017 23:06:29 -0600
> 
> > 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.

No, that's an entirely different issue, and arguably not a bug at all.
In line-move-visual mode, C-n/C-p (and vertical-motion they call under
the hood) keep the horizontal screen coordinate as much as they
possibly can, and this is what happens here.

You can see the same "bug" with this simple recipe:

  emacs -Q
  M-: (add-text-properties 72 73 '(line-prefix "12345")) RET
  C-u 10 M-g c
  C-=
  C-n
  C-=

The last command will say "column=4" although you started with
column=9.

Maybe with C-n/C-p people will expect what you expected in the case of
line numbers (but I'd like to hear more opinions before I'm convinced
to change the code to do that), but you will have a much harder time
convincing me to make a change in how scroll-preserve-screen-position
behaves in this case.

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

Of course there is: call posn-at-point and its ilk.

The problem is to know that x coordinate _before_ the margins actually
change their width.  That is the problem to be solved in this case.

> Why don't these issues affect nlinum, since it sets
> the width dynamically?

Because nlinum and similar modes change the width of the margin
_after_ C-n already moved point.  So C-n does its thing with the
margin still at its old width, and doesn't need to deal with the width
changing under its feet.

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

I don't think this should be done in C.  I can provide a function to
obtain the current width of the line-number display, and then a Lisp
program, called from some suitable hook, could notice when the value
becomes larger, and set display-line-number-width to that same value.
Would that be satisfactory?

P.S. Btw, I think these are all minor issues that shouldn't prevent
landing the feature on master.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: James Nguyen <james@jojojames.com>
> Cc: emacs-devel@gnu.org
> Cc: 
> Date: Sun, 02 Jul 2017 20:22:34 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Are you sure this isn't a bug in evil-mode?  Does this feature behave
> > correctly in a buffer with a non-nil line-prefix?
> >
> > Can you show a backtrace from the args out of range error?
> 
> I haven't been keeping track of the line-prefix conversations around
> this feature but setting it and then trying to do same thing (as
> reported above) works in previous version of this line number branch.
> 
> (setq line-prefix "1")
> 16d2695674 * @ Minor change in NEWS.
> ^ This combination works.
> 
> Here's the trace on the latest commit.
> 4c9353a584 * scratch/line-numbers origin/scratch/line-numbers
> 
> Debugger entered--Lisp error: (args-out-of-range 1)
>   signal(args-out-of-range (1))

Thanks, I fixed this now.

Re: Native line numbers, final testing

[James Nguyen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Debugger entered--Lisp error: (args-out-of-range 1)
>>   signal(args-out-of-range (1))
>
> Thanks, I fixed this now.

Thanks, works for me!

Re: Native line numbers, final testing

[Filipe Silva]

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

Eli, I was just able to begin some testing on visual relative line numbers
now and I must say that is truly joyful. The speed, the precision, coupled
with evil mode, absolutety fantastic. What a great feature. What a great
editor.

Just opened a huge org mode file with the visual option and it is working
just as expected. One thing I noticed though that can be an improvement:

When working with really large files, 10k+ lines long, having the current
line display the abs line number can feel "to much" because all lines have
max 2 number and the current line has 5 number so the fringe has to grow
extra 3 columns just to acommodate the current abs line number of the
current line.

Would it be possible to have like a toggle mechanism to switch the current
line number of the current line from abs to nothing or even to zero? that'd
be a great addition to this already awesome feature.

Thanks again Eli,will continue testing.



On Mon, Jul 3, 2017 at 2:04 PM, James Nguyen <james@jojojames.com> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> Debugger entered--Lisp error: (args-out-of-range 1)
> >>   signal(args-out-of-range (1))
> >
> > Thanks, I fixed this now.
>
> Thanks, works for me!
>
>

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

Re: Native line numbers, final testing

[Filipe Silva]

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

Eli, one more question are we able to customize the how the faces of the
numbers look? It would be great if the numbers had one face and current
line number had another face so we could assign different colors to each in
our current theme.

thanks again.

On Tue, Jul 4, 2017 at 7:57 AM, Filipe Silva <filipe.silva@gmail.com> wrote:

> Eli, I was just able to begin some testing on visual relative line numbers
> now and I must say that is truly joyful. The speed, the precision, coupled
> with evil mode, absolutety fantastic. What a great feature. What a great
> editor.
>
> Just opened a huge org mode file with the visual option and it is working
> just as expected. One thing I noticed though that can be an improvement:
>
> When working with really large files, 10k+ lines long, having the current
> line display the abs line number can feel "to much" because all lines have
> max 2 number and the current line has 5 number so the fringe has to grow
> extra 3 columns just to acommodate the current abs line number of the
> current line.
>
> Would it be possible to have like a toggle mechanism to switch the current
> line number of the current line from abs to nothing or even to zero? that'd
> be a great addition to this already awesome feature.
>
> Thanks again Eli,will continue testing.
>
>
>
> On Mon, Jul 3, 2017 at 2:04 PM, James Nguyen <james@jojojames.com> wrote:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> >> Debugger entered--Lisp error: (args-out-of-range 1)
>> >>   signal(args-out-of-range (1))
>> >
>> > Thanks, I fixed this now.
>>
>> Thanks, works for me!
>>
>>
>

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

Re: Native line numbers, final testing

[Kaushal Modi]

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

On Tue, Jul 4, 2017, 7:00 AM Filipe Silva <filipe.silva@gmail.com> wrote:

> Eli, one more question are we able to customize the how the faces of the
> numbers look?
>

Yes, a separate face is already there, but by default it is set to look
just like the other line numbers.

The face is line-number-current-line. The default line number face is
line-number.

> --

Kaushal Modi

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

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Filipe Silva <filipe.silva@gmail.com>
> Date: Tue, 4 Jul 2017 08:00:00 -0300
> Cc: Eli Zaretskii <eliz@gnu.org>, Emacs developers <emacs-devel@gnu.org>
> 
> Eli, one more question are we able to customize the how the faces of the numbers look? It would be great if
> the numbers had one face and current line number had another face so we could assign different colors to
> each in our current theme.

As Kaushal pointed out, you can do that.

I wrote full documentation for this feature, both in NEWS and in the
Emacs manual.  I suggest to take a look at the docs, it will probably
answer many questions.

Thanks for testing the branch.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Filipe Silva <filipe.silva@gmail.com>
> Date: Tue, 4 Jul 2017 07:57:10 -0300
> Cc: Eli Zaretskii <eliz@gnu.org>, Emacs developers <emacs-devel@gnu.org>
> 
> When working with really large files, 10k+ lines long, having the current line display the abs line number can
> feel "to much" because all lines have max 2 number and the current line has 5 number so the fringe has to
> grow extra 3 columns just to acommodate the current abs line number of the current line. 
> 
> Would it be possible to have like a toggle mechanism to switch the current line number of the current line from
> abs to nothing or even to zero? that'd be a great addition to this already awesome feature.

You can customize display-line-numbers-current-absolute to do what you
want.

Thank you for the feedback.

Re: Native line numbers, final testing

[Filipe Silva]

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

Eli thanks you for the information. will test that.

I found a bug regarding the line-number-current-line face. Here's a
screenshot: http://i.imgur.com/I2sneeL.png

I'm using this configuration for relative visual numbers:

(custom-set-variables
...
 '(display-line-numbers 'visual)
...)

And here's my setup for the faces:

  ;; MODE SUPPORT: line numbers and relative line numbers

  (line-number              (:inherit 'highlight-numbers-number))
  (line-number-current-line (:foreground gruvbox-neutral_red))

In my testing, I found that when point is on line 1, everything is fine. As
you go down line by line, the line-number-current-face skips ahead by one
so by the time point is on line 10, line-number-current-line face is really
acting on a number 9 lines below what should be the current line.


On Tue, Jul 4, 2017 at 11:32 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Filipe Silva <filipe.silva@gmail.com>
> > Date: Tue, 4 Jul 2017 07:57:10 -0300
> > Cc: Eli Zaretskii <eliz@gnu.org>, Emacs developers <emacs-devel@gnu.org>
> >
> > When working with really large files, 10k+ lines long, having the
> current line display the abs line number can
> > feel "to much" because all lines have max 2 number and the current line
> has 5 number so the fringe has to
> > grow extra 3 columns just to acommodate the current abs line number of
> the current line.
> >
> > Would it be possible to have like a toggle mechanism to switch the
> current line number of the current line from
> > abs to nothing or even to zero? that'd be a great addition to this
> already awesome feature.
>
> You can customize display-line-numbers-current-absolute to do what you
> want.
>
> Thank you for the feedback.
>

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

Re: Native line numbers, final testing

[Filipe Silva]

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

I don't know why the message was garbled, before. here's another try:

Eli thanks you for the information. will test that.

I found a bug regarding the line-number-current-line face. Here's a
screenshot: http://i.imgur.com/I2sneeL.png


I'm using this configuration for relative visual numbers:

(custom-set-variables
...
 '(display-line-numbers 'visual)
...)

And here's my setup for the faces:

  ;; MODE SUPPORT: line numbers and relative line numbers

  (line-number              (:inherit 'highlight-numbers-number))
  (line-number-current-line (:foreground gruvbox-neutral_red))

In my testing, I found that when point is on line 1, everything is fine. As
you go down line by line, the line-number-current-face skips ahead by one
so by the time point is on line 10, line-number-current-line face is really
acting on a number 9 lines below what should be the current line.


On Tue, Jul 4, 2017 at 11:48 AM, Filipe Silva <filipe.silva@gmail.com>
wrote:

> Eli thanks you for the information. will test that.
>
> I found a bug regarding the line-number-current-line face. Here's a
> screenshot: http://i.imgur.com/I2sneeL.png
>
> I'm using this configuration for relative visual numbers:
>
> (custom-set-variables
> ...
>  '(display-line-numbers 'visual)
> ...)
>
> And here's my setup for the faces:
>
>   ;; MODE SUPPORT: line numbers and relative line numbers
>
>   (line-number              (:inherit 'highlight-numbers-number))
>   (line-number-current-line (:foreground gruvbox-neutral_red))
>
> In my testing, I found that when point is on line 1, everything is fine.
> As you go down line by line, the line-number-current-face skips ahead by
> one so by the time point is on line 10, line-number-current-line face is
> really acting on a number 9 lines below what should be the current line.
>
>
> On Tue, Jul 4, 2017 at 11:32 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Filipe Silva <filipe.silva@gmail.com>
>> > Date: Tue, 4 Jul 2017 07:57:10 -0300
>> > Cc: Eli Zaretskii <eliz@gnu.org>, Emacs developers <emacs-devel@gnu.org
>> >
>> >
>> > When working with really large files, 10k+ lines long, having the
>> current line display the abs line number can
>> > feel "to much" because all lines have max 2 number and the current line
>> has 5 number so the fringe has to
>> > grow extra 3 columns just to acommodate the current abs line number of
>> the current line.
>> >
>> > Would it be possible to have like a toggle mechanism to switch the
>> current line number of the current line from
>> > abs to nothing or even to zero? that'd be a great addition to this
>> already awesome feature.
>>
>> You can customize display-line-numbers-current-absolute to do what you
>> want.
>>
>> Thank you for the feedback.
>>
>
>

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

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Filipe Silva <filipe.silva@gmail.com>
> Date: Tue, 4 Jul 2017 11:50:23 -0300
> Cc: James Nguyen <james@jojojames.com>, Emacs developers <emacs-devel@gnu.org>
> 
> In my testing, I found that when point is on line 1, everything is fine. As you go down line by line, the
> line-number-current-face skips ahead by one so by the time point is on line 10, line-number-current-line face
> is really acting on a number 9 lines below what should be the current line. 

Ha-ha, what a stupid bug!  Thanks, now fixed.

Re: Native line numbers, final testing

[Filipe Silva]

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

hehe thaks Eli. Are you sure you pushed your commit?

On Jul 4, 2017 12:44 PM, "Eli Zaretskii" <eliz@gnu.org> wrote:

> > From: Filipe Silva <filipe.silva@gmail.com>
> > Date: Tue, 4 Jul 2017 11:50:23 -0300
> > Cc: James Nguyen <james@jojojames.com>, Emacs developers <
> emacs-devel@gnu.org>
> >
> > In my testing, I found that when point is on line 1, everything is fine.
> As you go down line by line, the
> > line-number-current-face skips ahead by one so by the time point is on
> line 10, line-number-current-line face
> > is really acting on a number 9 lines below what should be the current
> line.
>
> Ha-ha, what a stupid bug!  Thanks, now fixed.
>

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

Re: Native line numbers, final testing

[Filipe Silva]

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

Ok. your last commit on savanah is not being registered in github emacs
mirror. Oh well...

On Tue, Jul 4, 2017 at 1:22 PM, Filipe Silva <filipe.silva@gmail.com> wrote:

> hehe thaks Eli. Are you sure you pushed your commit?
>
> On Jul 4, 2017 12:44 PM, "Eli Zaretskii" <eliz@gnu.org> wrote:
>
>> > From: Filipe Silva <filipe.silva@gmail.com>
>> > Date: Tue, 4 Jul 2017 11:50:23 -0300
>> > Cc: James Nguyen <james@jojojames.com>, Emacs developers <
>> emacs-devel@gnu.org>
>> >
>> > In my testing, I found that when point is on line 1, everything is
>> fine. As you go down line by line, the
>> > line-number-current-face skips ahead by one so by the time point is on
>> line 10, line-number-current-line face
>> > is really acting on a number 9 lines below what should be the current
>> line.
>>
>> Ha-ha, what a stupid bug!  Thanks, now fixed.
>>
>

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

Re: Native line numbers, final testing

[Richard Copley]

On 3 July 2017 at 16:24, Eli Zaretskii <eliz@gnu.org> wrote:

> Maybe with C-n/C-p people will expect what you expected in the case of
> line numbers (but I'd like to hear more opinions before I'm convinced
> to change the code to do that)

In my opinion it is important for the column position resulting from
C-n and C-p to be simple and predictable. I often rely on those
commands in keyboard macros. A simple example:

C-d 0 C-b C-n
C-d 1 C-b C-n
C-d 2 C-p C-p

This is supposed to change the character at point to 0 and the
characters in the corresponding column in the next two lines to 1
and 2.

I think that's a common use case and it won't work if
display-line-numbers is on ...

Or will it? I'm afraid I got quite confused trying to test this.
I think I found a different bug. Recipe:

Open xdisp.c and go to line 30, column 30.
Type "C-d C-n C-d C-n C-d C-n C-d C-n"
The first C-n goes to column 30 on line 31 as expected.
The 2nd C-n goes to column 29 on line 31 (expected column 30).
The 3rd C-n goes to column 28 on line 31 (expected 30 (or 29?)).
The 4th C-n goes to column 27 on line 31.

Not sure if this is the same on master.


On 4 July 2017 at 17:22, Filipe Silva <filipe.silva@gmail.com> wrote:
> hehe thaks Eli. Are you sure you pushed your commit?
>
> On Jul 4, 2017 12:44 PM, "Eli Zaretskii" <eliz@gnu.org> wrote:
>>
>> > From: Filipe Silva <filipe.silva@gmail.com>
>> > Date: Tue, 4 Jul 2017 11:50:23 -0300
>> > Cc: James Nguyen <james@jojojames.com>, Emacs developers
>> > <emacs-devel@gnu.org>
>> >
>> > In my testing, I found that when point is on line 1, everything is fine.
>> > As you go down line by line, the
>> > line-number-current-face skips ahead by one so by the time point is on
>> > line 10, line-number-current-line face
>> > is really acting on a number 9 lines below what should be the current
>> > line.
>>
>> Ha-ha, what a stupid bug!  Thanks, now fixed.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Tue, 4 Jul 2017 17:35:16 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, james@jojojames.com, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> C-d 0 C-b C-n
> C-d 1 C-b C-n
> C-d 2 C-p C-p
> 
> This is supposed to change the character at point to 0 and the
> characters in the corresponding column in the next two lines to 1
> and 2.
> 
> I think that's a common use case and it won't work if
> display-line-numbers is on ...

Does it really not work for you with display-line-numbers on?

> Open xdisp.c and go to line 30, column 30.
> Type "C-d C-n C-d C-n C-d C-n C-d C-n"
> The first C-n goes to column 30 on line 31 as expected.
> The 2nd C-n goes to column 29 on line 31 (expected column 30).
> The 3rd C-n goes to column 28 on line 31 (expected 30 (or 29?)).
> The 4th C-n goes to column 27 on line 31.

I cannot reproduce this, I get column 30 all the time, as expected.

Are you running the latest branch?  If so, what are the values of the
related variables?

Re: Native line numbers, final testing

[Richard Copley]

On 4 July 2017 at 17:44, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Tue, 4 Jul 2017 17:35:16 +0100
>> Cc: Eli Zaretskii <eliz@gnu.org>, james@jojojames.com,
>>       Emacs developers <emacs-devel@gnu.org>
>>
>> C-d 0 C-b C-n
>> C-d 1 C-b C-n
>> C-d 2 C-p C-p
>>
>> This is supposed to change the character at point to 0 and the
>> characters in the corresponding column in the next two lines to 1
>> and 2.
>>
>> I think that's a common use case and it won't work if
>> display-line-numbers is on ...
>
> Does it really not work for you with display-line-numbers on?

In fact it does work fine, when I test in a new Emacs session
which isn't affected by the bug I mentioned before.

>> Open xdisp.c and go to line 30, column 30.
>> Type "C-d C-n C-d C-n C-d C-n C-d C-n"
>> The first C-n goes to column 30 on line 31 as expected.
>> The 2nd C-n goes to column 29 on line 31 (expected column 30).
>> The 3rd C-n goes to column 28 on line 31 (expected 30 (or 29?)).
>> The 4th C-n goes to column 27 on line 31.
>
> I cannot reproduce this, I get column 30 all the time, as expected.

I can't reproduce it either, any more. Sorry about that, the recipe
seemed stable, but it didn't happen when I tried just now in
emacs -Q and no longer happens even with my usual customizations.

If I see it again _and_ find a reliable recipe, I'll get back to you.

> Are you running the latest branch?

I was -- it was from just before your latest commit (committer time 18:43).

> If so, what are the values of the
> related variables?

`display-line-numbers' was `t', `column-number-mode' was turned on
and every other relevant variable (as far as I know) had its default value.

Re: Native line numbers, final testing

[Filipe Silva]

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

About my previous point, this little function takes care of business when
you want
to toggle the behaviour of display 0 or the absolute line number for the
current line with display-line-numbers set to 'visual or 'relative:

(defun ninrod/toggle-absolute-current-relative-visual-line-number ()
       "Toogle showing absolute line number or 0 for the current line when
display-line-numbers is set to visual or relative"
       (interactive)
       (if display-line-numbers-current-absolute
           (setq display-line-numbers-current-absolute nil)
         (setq display-line-numbers-current-absolute t)))

Boy, is this editor powerful. Holy cow.

Eli, the bug is fixed. All seems fine with the face system now.

On Tue, Jul 4, 2017 at 2:13 PM, Richard Copley <rcopley@gmail.com> wrote:

> On 4 July 2017 at 17:44, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Richard Copley <rcopley@gmail.com>
> >> Date: Tue, 4 Jul 2017 17:35:16 +0100
> >> Cc: Eli Zaretskii <eliz@gnu.org>, james@jojojames.com,
> >>       Emacs developers <emacs-devel@gnu.org>
> >>
> >> C-d 0 C-b C-n
> >> C-d 1 C-b C-n
> >> C-d 2 C-p C-p
> >>
> >> This is supposed to change the character at point to 0 and the
> >> characters in the corresponding column in the next two lines to 1
> >> and 2.
> >>
> >> I think that's a common use case and it won't work if
> >> display-line-numbers is on ...
> >
> > Does it really not work for you with display-line-numbers on?
>
> In fact it does work fine, when I test in a new Emacs session
> which isn't affected by the bug I mentioned before.
>
> >> Open xdisp.c and go to line 30, column 30.
> >> Type "C-d C-n C-d C-n C-d C-n C-d C-n"
> >> The first C-n goes to column 30 on line 31 as expected.
> >> The 2nd C-n goes to column 29 on line 31 (expected column 30).
> >> The 3rd C-n goes to column 28 on line 31 (expected 30 (or 29?)).
> >> The 4th C-n goes to column 27 on line 31.
> >
> > I cannot reproduce this, I get column 30 all the time, as expected.
>
> I can't reproduce it either, any more. Sorry about that, the recipe
> seemed stable, but it didn't happen when I tried just now in
> emacs -Q and no longer happens even with my usual customizations.
>
> If I see it again _and_ find a reliable recipe, I'll get back to you.
>
> > Are you running the latest branch?
>
> I was -- it was from just before your latest commit (committer time 18:43).
>
> > If so, what are the values of the
> > related variables?
>
> `display-line-numbers' was `t', `column-number-mode' was turned on
> and every other relevant variable (as far as I know) had its default value.
>

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

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Tue, 4 Jul 2017 18:13:23 +0100
> Cc: Filipe Silva <filipe.silva@gmail.com>, james@jojojames.com, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> If I see it again _and_ find a reliable recipe, I'll get back to you.

Thanks.

> > If so, what are the values of the
> > related variables?
> 
> `display-line-numbers' was `t', `column-number-mode' was turned on
> and every other relevant variable (as far as I know) had its default value.

That's what I tried -- among other things.  And it worked as expected
here.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Filipe Silva <filipe.silva@gmail.com>
> Date: Tue, 4 Jul 2017 14:35:55 -0300
> Cc: Eli Zaretskii <eliz@gnu.org>, James Nguyen <james@jojojames.com>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> Eli, the bug is fixed. All seems fine with the face system now.

Thanks again for testing.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Richard Copley <rcopley@gmail.com>
>> Date: Tue, 4 Jul 2017 17:35:16 +0100
>> Cc: Eli Zaretskii <eliz@gnu.org>, james@jojojames.com, 
>> 	Emacs developers <emacs-devel@gnu.org>
>> 
>> C-d 0 C-b C-n
>> C-d 1 C-b C-n
>> C-d 2 C-p C-p
>> 
>> This is supposed to change the character at point to 0 and the
>> characters in the corresponding column in the next two lines to 1
>> and 2.
>> 
>> I think that's a common use case and it won't work if
>> display-line-numbers is on ...
>
> Does it really not work for you with display-line-numbers on?
>
>> Open xdisp.c and go to line 30, column 30.
>> Type "C-d C-n C-d C-n C-d C-n C-d C-n"
>> The first C-n goes to column 30 on line 31 as expected.
>> The 2nd C-n goes to column 29 on line 31 (expected column 30).
>> The 3rd C-n goes to column 28 on line 31 (expected 30 (or 29?)).
>> The 4th C-n goes to column 27 on line 31.
>
> I cannot reproduce this, I get column 30 all the time, as expected.
>
> Are you running the latest branch?  If so, what are the values of the
> related variables?

Here's an attempt at a relevant macro:

C-n C-d 1
C-p DEL 0
C-b

This replaces the character at point to 0, and the one below it to 1.

Now go into xdisp.c and set display-line-numbers to t:

M-g c 2960
C-17 C-n
C-x e

Notice that it's the previous columns that are changed, and not the
current column.

Re: Native line numbers, final testing

[Stefan Monnier]

>        (if display-line-numbers-current-absolute
>            (setq display-line-numbers-current-absolute nil)
>          (setq display-line-numbers-current-absolute t)))

aka
        (setq display-line-numbers-current-absolute
              (not display-line-numbers-current-absolute))
aka
        (cl-callf not display-line-numbers-current-absolute)


-- Stefan

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: Richard Copley <rcopley@gmail.com>,  emacs-devel@gnu.org,  james@jojojames.com,  filipe.silva@gmail.com
> Date: Tue, 04 Jul 2017 11:50:44 -0600
> 
> Here's an attempt at a relevant macro:
> 
> C-n C-d 1
> C-p DEL 0
> C-b
> 
> This replaces the character at point to 0, and the one below it to 1.
> 
> Now go into xdisp.c and set display-line-numbers to t:
> 
> M-g c 2960
> C-17 C-n
> C-x e
> 
> Notice that it's the previous columns that are changed, and not the
> current column.

Isn't that the same issue we were discussing lately, with C-n keeping
the horizontal coordinate?

Re: Native line numbers, final testing

[Richard Copley]

On 4 July 2017 at 19:24, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Alex <agrambot@gmail.com>
>> Cc: Richard Copley <rcopley@gmail.com>,  emacs-devel@gnu.org,  james@jojojames.com,  filipe.silva@gmail.com
>> Date: Tue, 04 Jul 2017 11:50:44 -0600
>>
>> Here's an attempt at a relevant macro:
>>
>> C-n C-d 1
>> C-p DEL 0
>> C-b
>>
>> This replaces the character at point to 0, and the one below it to 1.
>>
>> Now go into xdisp.c and set display-line-numbers to t:
>>
>> M-g c 2960
>> C-17 C-n
>> C-x e
>>
>> Notice that it's the previous columns that are changed, and not the
>> current column.
>
> Isn't that the same issue we were discussing lately, with C-n keeping
> the horizontal coordinate?

Yes, it seems to be. I think Alex's recipe does support my opinion
that this behaviour (unpredictable column position after vertical motion)
is, to a degree, in some respects, undesirable. (The keyboard macro
I suggested completely failed to support my point.)

It's just my opinion. (It was a response to your request for opinions.)

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Tue, 4 Jul 2017 19:37:10 +0100
> Cc: Alex <agrambot@gmail.com>, Emacs Development <emacs-devel@gnu.org>, james@jojojames.com, 
> 	Filipe Silva <filipe.silva@gmail.com>
> 
> It's just my opinion. (It was a response to your request for opinions.)

Sure, thanks for providing one.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sun, 02 Jul 2017 23:06:29 -0600
>> 
>> > 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.
>
> No, that's an entirely different issue, and arguably not a bug at all.
> In line-move-visual mode, C-n/C-p (and vertical-motion they call under
> the hood) keep the horizontal screen coordinate as much as they
> possibly can, and this is what happens here.
>
> You can see the same "bug" with this simple recipe:
>
>   emacs -Q
>   M-: (add-text-properties 72 73 '(line-prefix "12345")) RET
>   C-u 10 M-g c
>   C-=
>   C-n
>   C-=
>
> The last command will say "column=4" although you started with
> column=9.
>
> Maybe with C-n/C-p people will expect what you expected in the case of
> line numbers (but I'd like to hear more opinions before I'm convinced
> to change the code to do that),

I've never heard of line-prefix before this discussion, so I don't know
what the expected behaviour of it is/should be. However, I don't believe
the width of the line numbers should have any bearing on the column
position of a particular character in the buffer. Indeed, C-x = at the
beginning of a line with display-line-numbers correctly shows
"column=0". So I don't see the link between your recipe and mine.

I highly doubt many, if anyone else, expect line numbers to behave like
this.

> but you will have a much harder time
> convincing me to make a change in how scroll-preserve-screen-position
> behaves in this case.

Perhaps this one makes sense, since it is preserving screen position
rather than column position. I'll wait and see if this part bothers me
in the future.

>> > 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?
>
> Of course there is: call posn-at-point and its ilk.
>
> The problem is to know that x coordinate _before_ the margins actually
> change their width.  That is the problem to be solved in this case.

I don't understand. If the x-coordinate can be calculated at the start
of vertical-motion, then isn't that the x-coordinate before the margins
change their width?

As a thought for the current problem, could you adjust the position
after vertical-motion is called if it turns out that the
line-number-width/margin-size changed?

>> Why don't these issues affect nlinum, since it sets
>> the width dynamically?
>
> Because nlinum and similar modes change the width of the margin
> _after_ C-n already moved point.  So C-n does its thing with the
> margin still at its old width, and doesn't need to deal with the width
> changing under its feet.

Didn't you write before (when talking about 'visual) that line number
calculation/display was done after the point is moved? In that case, how
is display-line-numbers different to what you just described (outside of
not using the margin)?

>> 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?
>
> I don't think this should be done in C.  I can provide a function to
> obtain the current width of the line-number display, and then a Lisp
> program, called from some suitable hook, could notice when the value
> becomes larger, and set display-line-number-width to that same value.
> Would that be satisfactory?

If the performance and convenience is about the same, then I suppose it
doesn't matter where it's implemented. What would be a suitable hook? I
see a pre-redisplay-functions, but not a post-redisplay-functions. Or
would you provide a specific display-line-number hook?

> P.S. Btw, I think these are all minor issues that shouldn't prevent
> landing the feature on master.

I believe that the C-n/C-p issue should at least be a blocker for 26.1.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 04 Jul 2017 13:36:03 -0600
> 
> I've never heard of line-prefix before this discussion, so I don't know
> what the expected behaviour of it is/should be. However, I don't believe
> the width of the line numbers should have any bearing on the column
> position of a particular character in the buffer. Indeed, C-x = at the
> beginning of a line with display-line-numbers correctly shows
> "column=0". So I don't see the link between your recipe and mine.

"C-x =" just counts characters.  Try "M-: (posn-at-point) RET" for
more insight.

> I highly doubt many, if anyone else, expect line numbers to behave like
> this.

It's quite possible.  I just want to hear that before I write the
code.  Since I probably won't use line numbers (except for debugging
problems they cause ;-), I don't think my judgment is good enough
here.

> >> > 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?
> >
> > Of course there is: call posn-at-point and its ilk.
> >
> > The problem is to know that x coordinate _before_ the margins actually
> > change their width.  That is the problem to be solved in this case.
> 
> I don't understand. If the x-coordinate can be calculated at the start
> of vertical-motion, then isn't that the x-coordinate before the margins
> change their width?

Yes.  But I think we are talking past each other, see below.

> As a thought for the current problem, could you adjust the position
> after vertical-motion is called if it turns out that the
> line-number-width/margin-size changed?

Sure.  I already write code to do that, I just stashed it waiting to
see what this discussion concludes.  (And it isn't enough to make the
correction inside vertical-motion: temporary-goal-column also needs to
be adjusted.)

> >> Why don't these issues affect nlinum, since it sets
> >> the width dynamically?
> >
> > Because nlinum and similar modes change the width of the margin
> > _after_ C-n already moved point.  So C-n does its thing with the
> > margin still at its old width, and doesn't need to deal with the width
> > changing under its feet.
> 
> Didn't you write before (when talking about 'visual) that line number
> calculation/display was done after the point is moved? In that case, how
> is display-line-numbers different to what you just described (outside of
> not using the margin)?

I think I lost the focus of this discussion, so I'm not sure anymore
I'm answering your questions correctly.  What is it that you are
getting at with these questions?

> > I don't think this should be done in C.  I can provide a function to
> > obtain the current width of the line-number display, and then a Lisp
> > program, called from some suitable hook, could notice when the value
> > becomes larger, and set display-line-number-width to that same value.
> > Would that be satisfactory?
> 
> If the performance and convenience is about the same, then I suppose it
> doesn't matter where it's implemented. What would be a suitable hook? I
> see a pre-redisplay-functions, but not a post-redisplay-functions.

pre-redisplay-functions should be OK, but I think even
pre-command-hook is fine.  You really want to set this before
redisplay runs, because after it runs it might be too late.

> would you provide a specific display-line-number hook?

I don't see why we would want such a hook, and what would it do and
how.

> I believe that the C-n/C-p issue should at least be a blocker for 26.1.

We should be well past this long before we consider what blocks 26.1.

Re: Native line numbers, final testing

[Andy Moreton]

On Fri 30 Jun 2017, Eli Zaretskii wrote:

> I've finished development of this feature and pushed the changes to
> the scratch/line-numbers branch.  "Visual" line numbers are
> implemented, and all the known bugs should be fixed.  I also added
> documentation.
>
> Please test and report any problems you find.  If no grave bugs show
> up, I will probably land this on master in a few days.
>
> Thanks to everybody who sent feedback and reported problems and
> feature requests.

Hi Eli,

I've tested this for a couple of days on Windows and it seems to be very
good, with clear documentation in the manual.

Oone thing I often find useful is to view absolute line numbers from the
whole buffer while narrowed to a region (e.g. a single function of a
source file). This can be handy to limit the portion of a file for
navigation while displaying line numbers that match logs produced by
external tools.

linum-mode can be persuaded to do this with a little extra code, but I
could not see how to do this with the new feature. Please indicate if
this is possible, or consider adding it as another way to display line
numbers.

    AndyM

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Wed, 05 Jul 2017 21:24:53 +0100
> 
> Oone thing I often find useful is to view absolute line numbers from the
> whole buffer while narrowed to a region (e.g. a single function of a
> source file). This can be handy to limit the portion of a file for
> navigation while displaying line numbers that match logs produced by
> external tools.
> 
> linum-mode can be persuaded to do this with a little extra code, but I
> could not see how to do this with the new feature. Please indicate if
> this is possible, or consider adding it as another way to display line
> numbers.

Thanks, I've now added a customizable variable to get that behavior.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 04 Jul 2017 13:36:03 -0600
>> 
>> I've never heard of line-prefix before this discussion, so I don't know
>> what the expected behaviour of it is/should be. However, I don't believe
>> the width of the line numbers should have any bearing on the column
>> position of a particular character in the buffer. Indeed, C-x = at the
>> beginning of a line with display-line-numbers correctly shows
>> "column=0". So I don't see the link between your recipe and mine.
>
> "C-x =" just counts characters.  Try "M-: (posn-at-point) RET" for
> more insight.

OK, so C-n/C-p currently try to preserve the column calculated by either
posn-col-row or posn-actual-col-row? I believe I understand now, though
I still disagree that line numbers should behave similarly.

It appears that both nlinum and display-line-numbers contradict the 2nd
sentence of the documentation for line-move-visual. When the line number
width in nlinum increases, everything including the cursor is shifted
over. So if C-n triggers this change, then one could consider this to be
wrong behaviour.

In display-line-numbers, then C-n and C-p both may not preserve the
current horizontal screen position even when the line number width does
not (noticeably) change. Here, the horizontal screen position of the
cursor is moved by 1 character, but in contrast to the nlinum case, the
rest of the buffer contents remain horizontally stationary.

If one has to choose between these two behaviours, then I believe the
former is significantly more user-friendly. It is much easier to reason
about, and it does not change the horizontal cursor position relative to
buffer contents.

>> I highly doubt many, if anyone else, expect line numbers to behave like
>> this.
>
> It's quite possible.  I just want to hear that before I write the
> code.  Since I probably won't use line numbers (except for debugging
> problems they cause ;-), I don't think my judgment is good enough
> here.

I think a reasonable default is to behave similarly to the previous line
number implementations where it makes sense, and to change it if users
tend to dislike it. I suppose the contention here may be "where it makes
sense". I think it makes sense for the basic navigation commands to
behave similarly between the different implementations.

>> As a thought for the current problem, could you adjust the position
>> after vertical-motion is called if it turns out that the
>> line-number-width/margin-size changed?
>
> Sure.  I already write code to do that, I just stashed it waiting to
> see what this discussion concludes.  (And it isn't enough to make the
> correction inside vertical-motion: temporary-goal-column also needs to
> be adjusted.)

It's good to know that there's a working solution. I hope it will land
in Emacs.

>> >> Why don't these issues affect nlinum, since it sets
>> >> the width dynamically?
>> >
>> > Because nlinum and similar modes change the width of the margin
>> > _after_ C-n already moved point.  So C-n does its thing with the
>> > margin still at its old width, and doesn't need to deal with the width
>> > changing under its feet.
>> 
>> Didn't you write before (when talking about 'visual) that line number
>> calculation/display was done after the point is moved? In that case, how
>> is display-line-numbers different to what you just described (outside of
>> not using the margin)?
>
> I think I lost the focus of this discussion, so I'm not sure anymore
> I'm answering your questions correctly.  What is it that you are
> getting at with these questions?

Sorry, I think we both lost focus a bit since there's a few different
topics in this thread. Here, I believe we were talking about line
numbers in the margin, and if the property of being in the margin by
itself would be able to avoid this issue. I was under the assumption
that it would be, but assuming strict compliance to the documentation of
line-move-visual, then I think I get now why it wouldn't.

>> > I don't think this should be done in C.  I can provide a function to
>> > obtain the current width of the line-number display, and then a Lisp
>> > program, called from some suitable hook, could notice when the value
>> > becomes larger, and set display-line-number-width to that same value.
>> > Would that be satisfactory?
>> 
>> If the performance and convenience is about the same, then I suppose it
>> doesn't matter where it's implemented. What would be a suitable hook? I
>> see a pre-redisplay-functions, but not a post-redisplay-functions.
>
> pre-redisplay-functions should be OK, but I think even
> pre-command-hook is fine.  You really want to set this before
> redisplay runs, because after it runs it might be too late.
>
>> would you provide a specific display-line-number hook?
>
> I don't see why we would want such a hook, and what would it do and
> how.

I figured it would be best to change display-line-number-width as soon
as possible, and for the check to be made as infrequent as possible. A
hook called in the same area that it->lnum_width is changed would be the
best for that, though perhaps that's going overboard.

pre-command-hook definitely sounds better than pre-redisplay-functions,
but it would be nice if the check didn't have to be made every time a
command is issued. Although I suppose if the width accessor is a fast
constant lookup, then it doesn't really matter.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 06 Jul 2017 20:46:42 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Alex <agrambot@gmail.com>
> >> Cc: emacs-devel@gnu.org
> >> Date: Tue, 04 Jul 2017 13:36:03 -0600
> >> 
> >> I've never heard of line-prefix before this discussion, so I don't know
> >> what the expected behaviour of it is/should be. However, I don't believe
> >> the width of the line numbers should have any bearing on the column
> >> position of a particular character in the buffer. Indeed, C-x = at the
> >> beginning of a line with display-line-numbers correctly shows
> >> "column=0". So I don't see the link between your recipe and mine.
> >
> > "C-x =" just counts characters.  Try "M-: (posn-at-point) RET" for
> > more insight.
> 
> OK, so C-n/C-p currently try to preserve the column calculated by either
> posn-col-row or posn-actual-col-row?

More accurately, C-n/C-p try to preserve the horizontal coordinate
(which can be expressed either in pixels or in canonical column
units), and posn-col-row calculates and returns the same coordinates.

> If one has to choose between these two behaviours, then I believe the
> former is significantly more user-friendly. It is much easier to reason
> about, and it does not change the horizontal cursor position relative to
> buffer contents.

OK.

> Sorry, I think we both lost focus a bit since there's a few different
> topics in this thread. Here, I believe we were talking about line
> numbers in the margin, and if the property of being in the margin by
> itself would be able to avoid this issue. I was under the assumption
> that it would be, but assuming strict compliance to the documentation of
> line-move-visual, then I think I get now why it wouldn't.

Actually, I think it would be easier, because x coordinates are
counted from the text-area edge, so they are unaffected by the
margins.

But this fact alone is not enough to invalidate the design that
doesn't use the margin for line numbers.  A few complications like the
one we are talking about can always be solved by some support code,
they are not obstacles that cannot be negotiated.

> I figured it would be best to change display-line-number-width as soon
> as possible, and for the check to be made as infrequent as possible.

Line numbers are calculated every redisplay cycle (for those lines
that need to be redrawn), and a redisplay cycle always follows a
command, so the frequency is not affected by using the hooks I
proposed.

> pre-command-hook definitely sounds better than pre-redisplay-functions,
> but it would be nice if the check didn't have to be made every time a
> command is issued. Although I suppose if the width accessor is a fast
> constant lookup, then it doesn't really matter.

It cannot be an accessor to a value that is already stored (because it
currently isn't stored anywhere once redisplay finishes), but it
should be fast enough nonetheless.  And if it turns out it isn't, we
can always speed it up.

Re: Native line numbers, final testing

[Eli Zaretskii]

> Date: Fri, 07 Jul 2017 09:19:54 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > If one has to choose between these two behaviours, then I believe the
> > former is significantly more user-friendly. It is much easier to reason
> > about, and it does not change the horizontal cursor position relative to
> > buffer contents.
> 
> OK.

Since everyone who offered opinions about this agreed with you, I have
now made a change for C-n/C-p to behave like you expected.

Thanks.

Re: Native line numbers, final testing

[Eli Zaretskii]

> Date: Fri, 07 Jul 2017 09:19:54 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > pre-command-hook definitely sounds better than pre-redisplay-functions,
> > but it would be nice if the check didn't have to be made every time a
> > command is issued. Although I suppose if the width accessor is a fast
> > constant lookup, then it doesn't really matter.
> 
> It cannot be an accessor to a value that is already stored (because it
> currently isn't stored anywhere once redisplay finishes), but it
> should be fast enough nonetheless.  And if it turns out it isn't, we
> can always speed it up.

I timed the current implementation at 0.2 msec, in an optimized build;
should be at least twice faster in an optimized build.  This sounds
fast enough for now, I think.

Re: Native line numbers, final testing

[Eli Zaretskii]

> Date: Fri, 07 Jul 2017 12:47:35 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> I timed the current implementation at 0.2 msec, in an optimized build;
                                                        ^^^^^^^^^
Sorry, that should have been "unoptimized".

Re: Native line numbers, final testing

[Filipe Silva]

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

Eli, does this performance degenerate for for really large files? Like 25k
line files?

On Jul 7, 2017 6:50 AM, "Eli Zaretskii" <eliz@gnu.org> wrote:

> > Date: Fri, 07 Jul 2017 12:47:35 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: emacs-devel@gnu.org
> >
> > I timed the current implementation at 0.2 msec, in an optimized build;
>                                                         ^^^^^^^^^
> Sorry, that should have been "unoptimized".
>
>

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

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Filipe Silva <filipe.silva@gmail.com>
> Date: Fri, 7 Jul 2017 08:14:29 -0300
> Cc: Emacs developers <emacs-devel@gnu.org>, agrambot@gmail.com
> 
> Eli, does this performance degenerate for for really large files? Like 25k line files? 

First, I meant the time taken by line-number-display-width, something
you shouldn't be bothered unless you write Lisp which calls that
function.

And no, the performance doesn't seem to depend on the size of the
buffer, at least not for medium-size buffers (I tried a 33K-line
buffer).

Re: Native line numbers, final testing

[Eli Zaretskii]

> Date: Fri, 07 Jul 2017 12:47:35 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> I timed the current implementation at 0.2 msec, in an optimized build;

Trying to repeat this for several large buffers, I get much faster
performance: 0.04 micro-seconds per call.  I guess something was wrong
with my original timing.

In any case, it's _fast_.

Re: Native line numbers, final testing

[Filipe Silva]

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

That's very fast. Amazing work Eli.

On Jul 7, 2017 9:30 AM, "Eli Zaretskii" <eliz@gnu.org> wrote:

> Date: Fri, 07 Jul 2017 12:47:35 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
>
> I timed the current implementation at 0.2 msec, in an optimized build;

Trying to repeat this for several large buffers, I get much faster
performance: 0.04 micro-seconds per call.  I guess something was wrong
with my original timing.

In any case, it's _fast_.

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

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Fri, 07 Jul 2017 09:19:54 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > If one has to choose between these two behaviours, then I believe the
>> > former is significantly more user-friendly. It is much easier to reason
>> > about, and it does not change the horizontal cursor position relative to
>> > buffer contents.
>> 
>> OK.
>
> Since everyone who offered opinions about this agreed with you, I have
> now made a change for C-n/C-p to behave like you expected.
>
> Thanks.

Thanks for implementing this; it works great!

Re: Native line numbers, final testing

[James Cloos]

Eli,

Having re-compiled master, I tried out the native line numbers, by
chance in a buffer in variable-pitch-mode.  It looks like the display
uses SPACE (U+20) or SPACE's width to offset the digits.

It would look better if it used FIGURE SPACE (U+2007) or FIGURE SPACE's
width instead.  Then the line numbers would, err, line up.

The code might need to check whether the current face provides a glyph
for FIGURE SPACE.

Alternatively, use the width of one of the digits (such as DIGIT EIGHT)
to determine how many pixels to skip.

Otherwise, it looks and works great.

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 0x997A9F17ED7DAEA6

Re: Native line numbers, final testing

[Yuri Khan]

On Mon, Jul 10, 2017 at 3:16 AM, James Cloos <cloos@jhcloos.com> wrote:

> Alternatively, use the width of one of the digits (such as DIGIT EIGHT)
> to determine how many pixels to skip.

This is not going to work with fonts which do not make all digits the
same width, such as Georgia.

Why not just right-align line numbers?

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: James Cloos <cloos@jhcloos.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> Having re-compiled master, I tried out the native line numbers, by
> chance in a buffer in variable-pitch-mode.  It looks like the display
> uses SPACE (U+20) or SPACE's width to offset the digits.
> 
> It would look better if it used FIGURE SPACE (U+2007) or FIGURE SPACE's
> width instead.  Then the line numbers would, err, line up.

The doc string for the 'line-number' face explicitly tells to use
fixed-pitch font for the face.  Making the numbers line up is
impossible with the current algorithm of producing the glyphs for the
numbers on the fly, without knowing the width of the line-number
display on any other lines shown in the window.

> The code might need to check whether the current face provides a glyph
> for FIGURE SPACE.

And if not?

> Alternatively, use the width of one of the digits (such as DIGIT EIGHT)
> to determine how many pixels to skip.

Variable-pitch fonts can have different digits take different width.
So I don't think I understand how this could work.  What am I missing?

> Otherwise, it looks and works great.

Thanks.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Mon, 10 Jul 2017 04:45:19 +0700
> Cc: Emacs developers <emacs-devel@gnu.org>, Eli Zaretskii <eliz@gnu.org>
> 
> Why not just right-align line numbers?

I don't think I understand the idea, please elaborate.  E.g., what
will be to the left of the leftmost digit?

Please also don't forget that in R2L paragraphs the numbers appear on
the right.

Re: Native line numbers, final testing

[James Cloos]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> The doc string for the 'line-number' face explicitly tells to use
EZ> fixed-pitch font for the face.

I hadn't gotten that far; I only called M-x set-variable in a buffer
which was already in variable-pitch-mode, and then toggled v-p-m a
couple of time to compare w/ and w/o.  I hadn't gone looking for the
name of the face used by the line numbers.

I'd also -- until this thread reminded me -- forgotten about fonts which
do not have fixed width numerals by default.  It had been some time since
I last dealt with the four options for digits (proportional vs fixed on
one axis, and fixed-height vs oldstyle on the other).

Perhaps what I should have written is that the default face for line-
number should not be affected by enabling variable-pitch-mode.

But I think I've already gotten used to the variable width line numbers.

>> The code might need to check whether the current face provides a glyph
>> for FIGURE SPACE.

EZ> And if not?

Then it would have had to ignore FIGURE SPACE and fall back to SPACE...

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 0x997A9F17ED7DAEA6

Re: Native line numbers, final testing

[Yuri Khan]

On Mon, Jul 10, 2017 at 9:33 AM, Eli Zaretskii <eliz@gnu.org> wrote:

>> Why not just right-align line numbers?
>
> I don't think I understand the idea, please elaborate.  E.g., what
> will be to the left of the leftmost digit?

A “specified space” of width equal to the difference of the pixel
widths of the line numbers area and the current line number? If Emacs
does not have a notion of right-aligned text output, that is. Which
would be surprising in an application that supports RTL, but what do I
know.

> Please also don't forget that in R2L paragraphs the numbers appear on
> the right.

I am assuming that the motivation for aligning line numbers is the
same as for other columns of numeric data: putting digits of the same
order of magnitude under each other (as long as the font is
fixed-width wrt digits; otherwise, add “roughly”). This does not
change for Arabic and Hebrew, where, as far as I know, decimal numbers
are still written with the most significant digit to the left. So it
makes sense to me to right-align these too, although I’m not closely
acquainted with R2L typographic conventions.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: James Cloos <cloos@jhcloos.com>
> Cc: emacs-devel@gnu.org
> Date: Mon, 10 Jul 2017 01:35:12 -0400
> 
> >>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> EZ> The doc string for the 'line-number' face explicitly tells to use
> EZ> fixed-pitch font for the face.
> 
> I hadn't gotten that far; I only called M-x set-variable in a buffer
> which was already in variable-pitch-mode, and then toggled v-p-m a
> couple of time to compare w/ and w/o.  I hadn't gone looking for the
> name of the face used by the line numbers.

The line-number face by default inherits from the 'default' face.  So
how did you get into a situation where the frame's default face uses a
variable-pitch font?  Did you invoke Emacs with -fn option, or do you
have some mode which creates a frame with the face parameter that
specifies a variable-pitch font?

> Perhaps what I should have written is that the default face for line-
> number should not be affected by enabling variable-pitch-mode.

I didn't think people would have frames whose default face is
variable-pitch.

Does the patch below give good results?

> >> The code might need to check whether the current face provides a glyph
> >> for FIGURE SPACE.
> 
> EZ> And if not?
> 
> Then it would have had to ignore FIGURE SPACE and fall back to SPACE...

Btw, I don't really see how FIGURE SPACE could solve the problem at
hand anyway.  AFAIU, that character is simply a space whose width is
the same as that of digits.  So it's appropriate for when thousands
separator is a space, but it cannot magically align a number displayed
with a variable-pitch font.  Am I missing something?

Here's the patch I propose to fix your variable-pitch problem:

diff --git a/lisp/faces.el b/lisp/faces.el
index c3693d1..52a4db5 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2467,7 +2467,7 @@ trailing-whitespace
 
 ;; Definition stolen from linum.el.
 (defface line-number
-  '((t :inherit (shadow default)))
+  '((t :inherit (shadow default) :family "Monospace Serif"))
   "Face for displaying line numbers.
 This face is used when `display-line-numbers' is non-nil.
 

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Mon, 10 Jul 2017 14:09:00 +0700
> Cc: "cloos@jhcloos.com" <cloos@jhcloos.com>, Emacs developers <emacs-devel@gnu.org>
> 
> >> Why not just right-align line numbers?
> >
> > I don't think I understand the idea, please elaborate.  E.g., what
> > will be to the left of the leftmost digit?
> 
> A “specified space” of width equal to the difference of the pixel
> widths of the line numbers area and the current line number?

A.k.a. a "stretch glyph" of a suitably computed width.  Yes, this
would work, but it would require a thorough rewrite of the
number-generating code: where now we determine how many decimal places
we need, and then the pixel width follows from that, the modified code
will need to do it the other way around.  That will bring additional
complications with producing a blank field instead of a number (e.g.,
beyond EOB), with handling text-scale-adjust and friends, etc.

So I'd prefer not to go that way, unless strictly necessary.  If the
solution I just posted, with using a fixed-pitch font for numbers, is
good enough, let's not rush to make more radical changes for the
benefit of what I consider a niche use case wrt to line-number
display.

> If Emacs does not have a notion of right-aligned text output, that
> is. Which would be surprising in an application that supports RTL,
> but what do I know.

We are talking about coding at the same level at which these
alignments are implemented, i.e. where text layout is generated.  Any
text alignment we need on this level must be implemented via
pixel-width calculations; there's no support for that on lower levels.
That is how RTL lines are laid out, because the screen is still drawn
left-to-right even when we display RTL text (since anything else would
be extremely inefficient).

Re: Native line numbers, final testing

[Filipe Silva]

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

Eli, is there a way to disable line numbers, but just for somem modes?

Or better, to enable line numbers, only for certain types of modes or
filetypes?



On Mon, Jul 10, 2017 at 2:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: James Cloos <cloos@jhcloos.com>
> > Cc: emacs-devel@gnu.org
> > Date: Mon, 10 Jul 2017 01:35:12 -0400
> >
> > >>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:
> >
> > EZ> The doc string for the 'line-number' face explicitly tells to use
> > EZ> fixed-pitch font for the face.
> >
> > I hadn't gotten that far; I only called M-x set-variable in a buffer
> > which was already in variable-pitch-mode, and then toggled v-p-m a
> > couple of time to compare w/ and w/o.  I hadn't gone looking for the
> > name of the face used by the line numbers.
>
> The line-number face by default inherits from the 'default' face.  So
> how did you get into a situation where the frame's default face uses a
> variable-pitch font?  Did you invoke Emacs with -fn option, or do you
> have some mode which creates a frame with the face parameter that
> specifies a variable-pitch font?
>
> > Perhaps what I should have written is that the default face for line-
> > number should not be affected by enabling variable-pitch-mode.
>
> I didn't think people would have frames whose default face is
> variable-pitch.
>
> Does the patch below give good results?
>
> > >> The code might need to check whether the current face provides a glyph
> > >> for FIGURE SPACE.
> >
> > EZ> And if not?
> >
> > Then it would have had to ignore FIGURE SPACE and fall back to SPACE...
>
> Btw, I don't really see how FIGURE SPACE could solve the problem at
> hand anyway.  AFAIU, that character is simply a space whose width is
> the same as that of digits.  So it's appropriate for when thousands
> separator is a space, but it cannot magically align a number displayed
> with a variable-pitch font.  Am I missing something?
>
> Here's the patch I propose to fix your variable-pitch problem:
>
> diff --git a/lisp/faces.el b/lisp/faces.el
> index c3693d1..52a4db5 100644
> --- a/lisp/faces.el
> +++ b/lisp/faces.el
> @@ -2467,7 +2467,7 @@ trailing-whitespace
>
>  ;; Definition stolen from linum.el.
>  (defface line-number
> -  '((t :inherit (shadow default)))
> +  '((t :inherit (shadow default) :family "Monospace Serif"))
>    "Face for displaying line numbers.
>  This face is used when `display-line-numbers' is non-nil.
>
>
>

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

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Filipe Silva <filipe.silva@gmail.com>
> Date: Mon, 10 Jul 2017 15:15:02 -0300
> Cc: James Cloos <cloos@jhcloos.com>, Emacs developers <emacs-devel@gnu.org>
> 
> Eli, is there a way to disable line numbers, but just for somem modes? 
> 
> Or better, to enable line numbers, only for certain types of modes or filetypes? 

The variable display-line-numbers is buffer-local, so you can set it
to a non-nil value in a mode hook.  Do that for major modes which
correspond to files in which you want the line numbers, and don't turn
on the mode globally.

Re: Native line numbers, final testing

[Filipe Silva]

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

Stefan, while this is awesome, do I have to (require 'cl-macs) for this to
work properly?

On Tue, Jul 4, 2017 at 2:52 PM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> >        (if display-line-numbers-current-absolute
> >            (setq display-line-numbers-current-absolute nil)
> >          (setq display-line-numbers-current-absolute t)))
>
> aka
>         (setq display-line-numbers-current-absolute
>               (not display-line-numbers-current-absolute))
> aka
>         (cl-callf not display-line-numbers-current-absolute)
>
>
> -- Stefan
>
>
>

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

Re: Native line numbers, final testing

[Filipe Silva]

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

awesome thanks.

On Mon, Jul 10, 2017 at 3:18 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Filipe Silva <filipe.silva@gmail.com>
> > Date: Mon, 10 Jul 2017 15:15:02 -0300
> > Cc: James Cloos <cloos@jhcloos.com>, Emacs developers <
> emacs-devel@gnu.org>
> >
> > Eli, is there a way to disable line numbers, but just for somem modes?
> >
> > Or better, to enable line numbers, only for certain types of modes or
> filetypes?
>
> The variable display-line-numbers is buffer-local, so you can set it
> to a non-nil value in a mode hook.  Do that for major modes which
> correspond to files in which you want the line numbers, and don't turn
> on the mode globally.
>
>

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

Re: Native line numbers, final testing

[James Cloos]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> The line-number face by default inherits from the 'default' face.

Which here switches from DejaVu Sans Mono to DejaVu Serif when I enable
variable-pitch-mode.

EZ> So how did you get into a situation where the frame's default face
EZ> uses a variable-pitch font?

My (custom-set-faces) call in includes:

  '(variable-pitch ((t (:family "DejaVu Serif"))))

EZ> I didn't think people would have frames whose default face is
EZ> variable-pitch.

I only get that when I enable variable-pitch-mode in a buffer, and
it only affects that/those buffer(s).

EZ> Does the patch below give good results?

I'll test it out.

But as I wrote yesterday, I've gotten used to the current look.

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 0x997A9F17ED7DAEA6

Re: Native line numbers, final testing

[Stefan Monnier]

> Stefan, while this is awesome, do I have to (require 'cl-macs) for this to
> work properly?

For the cl-callf thingy, yes, you need (require 'cl-lib).


        Stefan

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

> Here's the patch I propose to fix your variable-pitch problem:
>
> diff --git a/lisp/faces.el b/lisp/faces.el
> index c3693d1..52a4db5 100644
> --- a/lisp/faces.el
> +++ b/lisp/faces.el
> @@ -2467,7 +2467,7 @@ trailing-whitespace
>  
>  ;; Definition stolen from linum.el.
>  (defface line-number
> -  '((t :inherit (shadow default)))
> +  '((t :inherit (shadow default) :family "Monospace Serif"))
>    "Face for displaying line numbers.
>  This face is used when `display-line-numbers' is non-nil.
>  

This gives me a smaller line numbers by default, since I apparently have
not configured an appropriate serif font (my default font is DejaVu Sans
Mono).

What about just inheriting from fixed-pitch rather than default?

Re: Native line numbers, final testing

[Filipe Silva]

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

Eli, just a thought:  wouldn't it be appropriate to ship a minor mode
wrapping this feature?

Or do you think it is better to just leave it to the community?



On Jul 11, 2017 5:59 PM, "Alex" <agrambot@gmail.com> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Here's the patch I propose to fix your variable-pitch problem:
> >
> > diff --git a/lisp/faces.el b/lisp/faces.el
> > index c3693d1..52a4db5 100644
> > --- a/lisp/faces.el
> > +++ b/lisp/faces.el
> > @@ -2467,7 +2467,7 @@ trailing-whitespace
> >
> >  ;; Definition stolen from linum.el.
> >  (defface line-number
> > -  '((t :inherit (shadow default)))
> > +  '((t :inherit (shadow default) :family "Monospace Serif"))
> >    "Face for displaying line numbers.
> >  This face is used when `display-line-numbers' is non-nil.
> >
>
> This gives me a smaller line numbers by default, since I apparently have
> not configured an appropriate serif font (my default font is DejaVu Sans
> Mono).
>
> What about just inheriting from fixed-pitch rather than default?
>
>

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

Re: Native line numbers, final testing

[Filipe Silva]

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

Sorry, ai mean the native line number feature, not this thing alex is
talking about.

On Jul 11, 2017 6:18 PM, "Filipe Silva" <filipe.silva@gmail.com> wrote:

Eli, just a thought:  wouldn't it be appropriate to ship a minor mode
wrapping this feature?

Or do you think it is better to just leave it to the community?



On Jul 11, 2017 5:59 PM, "Alex" <agrambot@gmail.com> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Here's the patch I propose to fix your variable-pitch problem:
> >
> > diff --git a/lisp/faces.el b/lisp/faces.el
> > index c3693d1..52a4db5 100644
> > --- a/lisp/faces.el
> > +++ b/lisp/faces.el
> > @@ -2467,7 +2467,7 @@ trailing-whitespace
> >
> >  ;; Definition stolen from linum.el.
> >  (defface line-number
> > -  '((t :inherit (shadow default)))
> > +  '((t :inherit (shadow default) :family "Monospace Serif"))
> >    "Face for displaying line numbers.
> >  This face is used when `display-line-numbers' is non-nil.
> >
>
> This gives me a smaller line numbers by default, since I apparently have
> not configured an appropriate serif font (my default font is DejaVu Sans
> Mono).
>
> What about just inheriting from fixed-pitch rather than default?
>
>

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

Re: Native line numbers, final testing

[Alex]

Filipe Silva <filipe.silva@gmail.com> writes:

> Sorry, ai mean the native line number feature, not this thing alex is talking about.
>
> On Jul 11, 2017 6:18 PM, "Filipe Silva" <filipe.silva@gmail.com> wrote:
>
>  Eli, just a thought:  wouldn't it be appropriate to ship a minor mode wrapping this feature? 
>
>  Or do you think it is better to just leave it to the community?

It's in the works. See this thread:
https://lists.gnu.org/archive/html/emacs-devel/2017-07/msg00275.html

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: James Cloos <cloos@jhcloos.com>,  emacs-devel@gnu.org
> Date: Tue, 11 Jul 2017 14:58:21 -0600
> 
> >  (defface line-number
> > -  '((t :inherit (shadow default)))
> > +  '((t :inherit (shadow default) :family "Monospace Serif"))
> >    "Face for displaying line numbers.
> >  This face is used when `display-line-numbers' is non-nil.
> >  
> 
> This gives me a smaller line numbers by default, since I apparently have
> not configured an appropriate serif font (my default font is DejaVu Sans
> Mono).

You can always customize the face.

> What about just inheriting from fixed-pitch rather than default?

I think if it doesn't inherit from default, it won't resize if you
invoke "C-x C-+" etc.  But inheritance from default is not your
problem anyway, it's the "Monospace Serif" vs "Monospace" that bothers
you.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: James Cloos <cloos@jhcloos.com>,  emacs-devel@gnu.org
>> Date: Tue, 11 Jul 2017 14:58:21 -0600
>> 
>> >  (defface line-number
>> > -  '((t :inherit (shadow default)))
>> > +  '((t :inherit (shadow default) :family "Monospace Serif"))
>> >    "Face for displaying line numbers.
>> >  This face is used when `display-line-numbers' is non-nil.
>> >  
>> 
>> This gives me a smaller line numbers by default, since I apparently have
>> not configured an appropriate serif font (my default font is DejaVu Sans
>> Mono).
>
> You can always customize the face.

Yes, but the default should be sensible. At least on the systems I use,
in emacs -Q, "Monospace" matches up with my default font.

>> What about just inheriting from fixed-pitch rather than default?
>
> I think if it doesn't inherit from default, it won't resize if you
> invoke "C-x C-+" etc.  But inheritance from default is not your
> problem anyway, it's the "Monospace Serif" vs "Monospace" that bothers
> you.

Is there a good reason why the fixed-pitch* faces don't inherit from
default?

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
> Date: Tue, 11 Jul 2017 20:53:17 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Alex <agrambot@gmail.com>
> >> Cc: James Cloos <cloos@jhcloos.com>,  emacs-devel@gnu.org
> >> Date: Tue, 11 Jul 2017 14:58:21 -0600
> >> 
> >> >  (defface line-number
> >> > -  '((t :inherit (shadow default)))
> >> > +  '((t :inherit (shadow default) :family "Monospace Serif"))
> >> >    "Face for displaying line numbers.
> >> >  This face is used when `display-line-numbers' is non-nil.
> >> >  
> >> 
> >> This gives me a smaller line numbers by default, since I apparently have
> >> not configured an appropriate serif font (my default font is DejaVu Sans
> >> Mono).
> >
> > You can always customize the face.
> 
> Yes, but the default should be sensible. At least on the systems I use,
> in emacs -Q, "Monospace" matches up with my default font.

It should be sensible, I agree.  But "sensible" doesn't necessarily
mean identical to the default face's font.

> Is there a good reason why the fixed-pitch* faces don't inherit from
> default?

How are fixed-pitch* different from any other face that doesn't
inherit from default?

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
>> Date: Tue, 11 Jul 2017 20:53:17 -0600
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Alex <agrambot@gmail.com>
>> >> Cc: James Cloos <cloos@jhcloos.com>,  emacs-devel@gnu.org
>> >> Date: Tue, 11 Jul 2017 14:58:21 -0600
>> >> 
>> >> >  (defface line-number
>> >> > -  '((t :inherit (shadow default)))
>> >> > +  '((t :inherit (shadow default) :family "Monospace Serif"))
>> >> >    "Face for displaying line numbers.
>> >> >  This face is used when `display-line-numbers' is non-nil.
>> >> >  
>> >> 
>> >> This gives me a smaller line numbers by default, since I apparently have
>> >> not configured an appropriate serif font (my default font is DejaVu Sans
>> >> Mono).
>> >
>> > You can always customize the face.
>> 
>> Yes, but the default should be sensible. At least on the systems I use,
>> in emacs -Q, "Monospace" matches up with my default font.
>
> It should be sensible, I agree.  But "sensible" doesn't necessarily
> mean identical to the default face's font.

Sure, but I maintain in this case that it does mean identical to the
default face's font. It ensures, usually, proper size and a non-clashing
style. If someone customizes their default face, then they could
customize the line-number face similarly. Everyone else shouldn't have
to do any customization.

>> Is there a good reason why the fixed-pitch* faces don't inherit from
>> default?
>
> How are fixed-pitch* different from any other face that doesn't
> inherit from default?

Going into this discussion, I figured that fixed-pitch* would be exactly
like default, except enforcing, well, fixed-pitch; similarly for
variable-pitch. If I customized the frame's default face to use, e.g.,
variable-pitch, then I would expect that "C-x +" increases its text
size, like default.

Actually, it seems to do that normally. Evaluate this in a buffer
without font-lock-mode:

(insert (propertize "hello there!" 'face 'fixed-pitch))

This text increases along with the default face with "C-x +". Why does
the same not apply to the line-number face?

Re: Native line numbers, final testing

[Alex]

Alex <agrambot@gmail.com> writes:

> variable-pitch, then I would expect that "C-x +" increases its text

> This text increases along with the default face with "C-x +". Why does

Sorry, these should be "C-x C-=".

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
> Date: Wed, 12 Jul 2017 11:22:51 -0600
> 
> > It should be sensible, I agree.  But "sensible" doesn't necessarily
> > mean identical to the default face's font.
> 
> Sure, but I maintain in this case that it does mean identical to the
> default face's font.

I don't understand why, and I don't think I agree.

> It ensures, usually, proper size and a non-clashing style.

No one said these are necessarily our goals.  Not everything shown on
the initial frame uses the same font, so why should line numbers be
required to?

> If someone customizes their default face, then they could customize
> the line-number face similarly.

That's harsh on users, and exactly the reason why I made this change.
It might be a mistake, but I'd like to hear more opinions before I
make up my mind.

> Everyone else shouldn't have to do any customization.

They (and you) don't need to.  I only mentioned customization because
you didn't like the default.  Good defaults are supposed to be good
for many users, but they aren't required to satisfy everyone, as long
as things are customizable.

> > How are fixed-pitch* different from any other face that doesn't
> > inherit from default?
> 
> Going into this discussion, I figured that fixed-pitch* would be exactly
> like default, except enforcing, well, fixed-pitch; similarly for
> variable-pitch.

But that's obviously not the intent, as the code shows and the
comments explain.

> (insert (propertize "hello there!" 'face 'fixed-pitch))
> 
> This text increases along with the default face with "C-x +". Why does
> the same not apply to the line-number face?

I don't know.  Feel free to investigate and publish the findings.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
>> Date: Wed, 12 Jul 2017 11:22:51 -0600
>> 
>> > It should be sensible, I agree.  But "sensible" doesn't necessarily
>> > mean identical to the default face's font.
>> 
>> Sure, but I maintain in this case that it does mean identical to the
>> default face's font.
>
> I don't understand why, and I don't think I agree.

I don't see a single reason to make it different. linum didn't, and I
don't know of a single editor/IDE that uses a different font than the
default for the line numbers.

>> It ensures, usually, proper size and a non-clashing style.
>
> No one said these are necessarily our goals.  Not everything shown on
> the initial frame uses the same font, so why should line numbers be
> required to?

I'd like to ask the converse: why shouldn't line numbers be in the same
style/size, by default, as long as it's fixed-with? It should only
deviate from the default if there's a good reason to, and I don't see a
good reason to throw in serifs if the default usually/always doesn't
have them.

>> If someone customizes their default face, then they could customize
>> the line-number face similarly.
>
> That's harsh on users, and exactly the reason why I made this change.
> It might be a mistake, but I'd like to hear more opinions before I
> make up my mind.

I don't see how defaulting to serifs is less harsh.

Wasn't the point of this change to stop variable-pitch-mode from
affecting line numbers? In this case, using :family "Monospace" works
just as well.

>> Everyone else shouldn't have to do any customization.
>
> They (and you) don't need to.  I only mentioned customization because
> you didn't like the default.  Good defaults are supposed to be good
> for many users, but they aren't required to satisfy everyone, as long
> as things are customizable.

I agree with the last sentence, but I don't think this is good for many
users. If it happens that most other users' systems default to a
monospaced serif font, then I'll concede. Looking at
face-font-family-alternatives, however, leads to me to believe that this
is not the case.

>> > How are fixed-pitch* different from any other face that doesn't
>> > inherit from default?
>> 
>> Going into this discussion, I figured that fixed-pitch* would be exactly
>> like default, except enforcing, well, fixed-pitch; similarly for
>> variable-pitch.
>
> But that's obviously not the intent, as the code shows and the
> comments explain.

I don't see any explanation around the definitions for fixed-pitch*.

>> (insert (propertize "hello there!" 'face 'fixed-pitch))
>> 
>> This text increases along with the default face with "C-x +". Why does
>> the same not apply to the line-number face?
>
> I don't know.  Feel free to investigate and publish the findings.

Well, it definitely seems like a bug to me. text-scale-mode uses
face-remap-add-relative on 'default, but I don't see why this would
cause an issue. The manual states that any 'unspecified attributes mean
that the attributes are taken from the default face, so I don't see why
adding :inherit 'default should even make a difference.

Perhaps I should file a bug report?

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
> Date: Wed, 12 Jul 2017 14:03:15 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Alex <agrambot@gmail.com>
> >> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
> >> Date: Wed, 12 Jul 2017 11:22:51 -0600
> >> 
> >> > It should be sensible, I agree.  But "sensible" doesn't necessarily
> >> > mean identical to the default face's font.
> >> 
> >> Sure, but I maintain in this case that it does mean identical to the
> >> default face's font.
> >
> > I don't understand why, and I don't think I agree.
> 
> I don't see a single reason to make it different.

It's for when the default face uses a variable-pitch font, as I
believe I already explained.

And instead of repeating the same arguments again and again, I think
we should simply agree to disagree, and wait for others to offer their
opinions and experiences about this.

> >> (insert (propertize "hello there!" 'face 'fixed-pitch))
> >> 
> >> This text increases along with the default face with "C-x +". Why does
> >> the same not apply to the line-number face?
> >
> > I don't know.  Feel free to investigate and publish the findings.
> 
> Well, it definitely seems like a bug to me. text-scale-mode uses
> face-remap-add-relative on 'default, but I don't see why this would
> cause an issue. The manual states that any 'unspecified attributes mean
> that the attributes are taken from the default face, so I don't see why
> adding :inherit 'default should even make a difference.
> 
> Perhaps I should file a bug report?

I don't think I understood what you consider a bug here.  Face
attributes are calculated when the face is created, they are not
updated whenever the default face changes.

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
>> Date: Wed, 12 Jul 2017 14:03:15 -0600
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Alex <agrambot@gmail.com>
>> >> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
>> >> Date: Wed, 12 Jul 2017 11:22:51 -0600
>> >> 
>> >> > It should be sensible, I agree.  But "sensible" doesn't necessarily
>> >> > mean identical to the default face's font.
>> >> 
>> >> Sure, but I maintain in this case that it does mean identical to the
>> >> default face's font.
>> >
>> > I don't understand why, and I don't think I agree.
>> 
>> I don't see a single reason to make it different.
>
> It's for when the default face uses a variable-pitch font, as I
> believe I already explained.

I wasn't referring to the fixed-pitch part, just the serif part. The
addition of serifs is orthogonal to fixing the case of a
variable-pitched default font. In your patch, you injected an unrelated
property into the face that has nothing to do with fixing the
variable-pitch issue.

Just to make myself extra clear: I am in favour of forcing line numbers
to be fixed-pitch by default. However, I'm not in favour of using
"Monospace Serif" instead of "Monospace" considering that the default is
the latter.

> And instead of repeating the same arguments again and again, I think
> we should simply agree to disagree, and wait for others to offer their
> opinions and experiences about this.

I don't see how the discussion was about to devolve into repetition
(though there appears to have been some miscommunication). While I agree
that others should give their input, I find it disappointing that you
ignored the rest of my email.

>> >> (insert (propertize "hello there!" 'face 'fixed-pitch))
>> >> 
>> >> This text increases along with the default face with "C-x +". Why does
>> >> the same not apply to the line-number face?
>> >
>> > I don't know.  Feel free to investigate and publish the findings.
>> 
>> Well, it definitely seems like a bug to me. text-scale-mode uses
>> face-remap-add-relative on 'default, but I don't see why this would
>> cause an issue. The manual states that any 'unspecified attributes mean
>> that the attributes are taken from the default face, so I don't see why
>> adding :inherit 'default should even make a difference.
>> 
>> Perhaps I should file a bug report?
>
> I don't think I understood what you consider a bug here.  Face
> attributes are calculated when the face is created, they are not
> updated whenever the default face changes.

Not in the case of 'unspecified, right? Evaluating (set-face-bold
'default t) changes every face with an 'unspecified bold attribute to
bold.

Just to be clear, this is what the manual states[1]:

   Apart from the values given below, each face attribute can have the
value ‘unspecified’.  This special value means that the face doesn’t
specify that attribute directly.  An ‘unspecified’ attribute tells Emacs
to refer instead to a parent face (see the description ‘:inherit’
attribute below); or, failing that, to an underlying face (*note
Displaying Faces::).  The ‘default’ face must specify all attributes.

So if I'm understanding this correctly, then you shouldn't have to
inherit from 'default since any unspecified attributes already fallback
to 'default. The behaviour in this case is different, therefore this is
a bug (something isn't handling face-remapping-alist correctly).

Footnotes: 
[1]  (info "(elisp) Face Attributes")

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: cloos@jhcloos.com,  emacs-devel@gnu.org
> Date: Wed, 12 Jul 2017 22:11:12 -0600
> 
> >> I don't see a single reason to make it different.
> >
> > It's for when the default face uses a variable-pitch font, as I
> > believe I already explained.
> 
> I wasn't referring to the fixed-pitch part, just the serif part. The
> addition of serifs is orthogonal to fixing the case of a
> variable-pitched default font. In your patch, you injected an unrelated
> property into the face that has nothing to do with fixing the
> variable-pitch issue.

I tried using fixed-pitch first, and it on my system it picked up a
bad font, that's why I switched to fixed-pitch-serif.  We should now
see if that gives satisfactory results, before using something more
elaborate, like different definitions for different platforms.

> Just to make myself extra clear: I am in favour of forcing line numbers
> to be fixed-pitch by default. However, I'm not in favour of using
> "Monospace Serif" instead of "Monospace" considering that the default is
> the latter.

But "Monospace" is not guaranteed to yield the same font as the
default face has in "emacs -Q".  It perhaps does that on your system,
but not on mine, and I don't expect it to do that for everyone.  The
"standard" font used by the default face in "emacs -Q" is determined
by the standard fontset, whose spec is window-system specific.
Moreover, "Monospace" etc. specifies a family, not a specific font;
what font will that produce depends on the OS (see comments to
face-font-family-alternatives) and on the fonts installed on the
particular user's system.

So if your expectation is that the line-number face by default uses
the same font as the one picked up via the standard fontset, it will
not be achieved by using fixed-pitch, except by sheer luck.

IOW, this is a complex issue, because the specific font used for the
face depends on factors some of which are OS-dependent and others
depend on the installed fonts.  At least some of these factors are
unpredictable, so choosing a good default that will work for everyone
is not easy.

The solution we seek needs IMO to satisfy the following requirements:

 . it should always use a fixed-pitch font, even if the default face
   or the standard fontset are customized to use a variable-pitch font
 . it should obey text-scale-adjust
 . it should be defined by defface, so it's easily customizable
 . it should yield the same visual appearance on all frames (unless
   the user explicitly customizes it differently for different frames)
 . it shouldn't change during an Emacs session, except if the user
   explicitly customizes it during that session

If you can come up with a way to satisfy these requirements while
producing the same font as the default face in "emacs -Q", please show
your solution.  I couldn't think of a way of doing that for all the
supported platforms, but I don't consider myself an expert on faces.
In any case, using fixed-pitch does NOT guarantee the same font as you
get by default, not on all platforms anyway, so doing that is not the
solution we seek.

It's possible that fixed-pitch-serif will produce unsatisfactory
results on some systems.  ("Unsatisfactory" here means that the
numbers look ugly or not easily legible, or we pick up a font of low
quality, like some raster font.  "Unsatisfactory" does NOT necessarily
mean identical to the default font in "emacs -Q".)  In that case, we
will have to look for better solutions.  But we are not there yet, IMO.

> I find it disappointing that you ignored the rest of my email.

I didn't ignore it, I just didn't see any constructive responses that
would advance the discussion.  All I see is a fundamental disagreement
where you repeatedly insist on getting the same font as in the default
case, which I think is a relatively weak requirement, and I don't know
how to achieve that universally without violating the stronger
requirements listed above.  Disagreements about defaults are
frequently like that, they can only be resolved by collecting enough
opinions and experiences; 2 opposite experiences are definitely not
enough.

> > I don't think I understood what you consider a bug here.  Face
> > attributes are calculated when the face is created, they are not
> > updated whenever the default face changes.
> 
> Not in the case of 'unspecified, right? Evaluating (set-face-bold
> 'default t) changes every face with an 'unspecified bold attribute to
> bold.

Yes, but face remapping is not implemented by setting attributes of
existing faces.  It (internally) produces new faces and redirects
existing faces to those new ones.  And I think there's a good reason
for that: we definitely do NOT want _all_ the faces to change their
sizes to follow the remapping of 'default'.  For starters, we only
want the faces to change in the current buffer, and we don't want
faces like 'tooltip' and 'mode-line' to change even for the current
buffer.

> So if I'm understanding this correctly, then you shouldn't have to
> inherit from 'default since any unspecified attributes already fallback
> to 'default. The behaviour in this case is different, therefore this is
> a bug (something isn't handling face-remapping-alist correctly).

face-remapping-alist is only handled where we decided to handle it,
and I think I at least partially understand why, see above.

Re: Native line numbers, final testing

[Alex]

Sorry for not replying sooner. Hopefully the fixed-pitch issue is behind
us now.

Eli Zaretskii <eliz@gnu.org> writes:

> Disagreements about defaults are
> frequently like that, they can only be resolved by collecting enough
> opinions and experiences; 2 opposite experiences are definitely not
> enough.

Fair enough.

>> > I don't think I understood what you consider a bug here.  Face
>> > attributes are calculated when the face is created, they are not
>> > updated whenever the default face changes.
>> 
>> Not in the case of 'unspecified, right? Evaluating (set-face-bold
>> 'default t) changes every face with an 'unspecified bold attribute to
>> bold.
>
> Yes, but face remapping is not implemented by setting attributes of
> existing faces.  It (internally) produces new faces and redirects
> existing faces to those new ones.  And I think there's a good reason
> for that: we definitely do NOT want _all_ the faces to change their
> sizes to follow the remapping of 'default'.  For starters, we only
> want the faces to change in the current buffer, and we don't want
> faces like 'tooltip' and 'mode-line' to change even for the current
> buffer.

That makes sense, but I still don't understand why explicitly inheriting
from default makes a difference for face remapping. Is it just a
hardcoded workaround?

>> So if I'm understanding this correctly, then you shouldn't have to
>> inherit from 'default since any unspecified attributes already fallback
>> to 'default. The behaviour in this case is different, therefore this is
>> a bug (something isn't handling face-remapping-alist correctly).
>
> face-remapping-alist is only handled where we decided to handle it,
> and I think I at least partially understand why, see above.

Would it make sense to handle it in the line-number area? 

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 25 Jul 2017 21:50:59 -0600
> 
> Hopefully the fixed-pitch issue is behind us now.

Seems so.

> > Yes, but face remapping is not implemented by setting attributes of
> > existing faces.  It (internally) produces new faces and redirects
> > existing faces to those new ones.  And I think there's a good reason
> > for that: we definitely do NOT want _all_ the faces to change their
> > sizes to follow the remapping of 'default'.  For starters, we only
> > want the faces to change in the current buffer, and we don't want
> > faces like 'tooltip' and 'mode-line' to change even for the current
> > buffer.
> 
> That makes sense, but I still don't understand why explicitly inheriting
> from default makes a difference for face remapping. Is it just a
> hardcoded workaround?

The inheriting face references its parent, so when the parent is
remapped, that affects the inheriting face through the attributes that
are inherited.

What is "hardcoded" here is that text-scale-adjust affects the
'default' face, so faces unrelated to it will not be affected.

> > face-remapping-alist is only handled where we decided to handle it,
> > and I think I at least partially understand why, see above.
> 
> Would it make sense to handle it in the line-number area? 

I don't know.  Why should it?  It didn't affect linum and nlinum
faces.  What we have now allows users to customize the face both if
they want it to be sensitive to scaling, and if they don't.  Why
should we take that freedom from them?

Re: Native line numbers, final testing

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 25 Jul 2017 21:50:59 -0600
>> 
>> > Yes, but face remapping is not implemented by setting attributes of
>> > existing faces.  It (internally) produces new faces and redirects
>> > existing faces to those new ones.  And I think there's a good reason
>> > for that: we definitely do NOT want _all_ the faces to change their
>> > sizes to follow the remapping of 'default'.  For starters, we only
>> > want the faces to change in the current buffer, and we don't want
>> > faces like 'tooltip' and 'mode-line' to change even for the current
>> > buffer.
>> 
>> That makes sense, but I still don't understand why explicitly inheriting
>> from default makes a difference for face remapping. Is it just a
>> hardcoded workaround?
>
> The inheriting face references its parent, so when the parent is
> remapped, that affects the inheriting face through the attributes that
> are inherited.
>
> What is "hardcoded" here is that text-scale-adjust affects the
> 'default' face, so faces unrelated to it will not be affected.

Right, but it's not clear to me why explicitly inheriting from default
is treated differently from an :inherit value of 'unspecified. From the
manual:

  An ‘unspecified’ attribute tells Emacs to refer instead to a parent
  face

>> > face-remapping-alist is only handled where we decided to handle it,
>> > and I think I at least partially understand why, see above.
>> 
>> Would it make sense to handle it in the line-number area? 
>
> I don't know.  Why should it?  It didn't affect linum and nlinum
> faces.  What we have now allows users to customize the face both if
> they want it to be sensitive to scaling, and if they don't.  Why
> should we take that freedom from them?

I don't think the ability to customize it should go away, but the system
in place that allows for the customization just seems odd to me. Instead
of using an ostensibly redundant :inherit value, why not make a
customizable list of faces that face remapping also affects?

Perhaps it's too much work for too little gain. In that case, then at
least documenting the current behaviour would be nice.

Re: Native line numbers, final testing

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 29 Jul 2017 00:12:42 -0600
> 
> >> That makes sense, but I still don't understand why explicitly inheriting
> >> from default makes a difference for face remapping. Is it just a
> >> hardcoded workaround?
> >
> > The inheriting face references its parent, so when the parent is
> > remapped, that affects the inheriting face through the attributes that
> > are inherited.
> >
> > What is "hardcoded" here is that text-scale-adjust affects the
> > 'default' face, so faces unrelated to it will not be affected.
> 
> Right, but it's not clear to me why explicitly inheriting from default
> is treated differently from an :inherit value of 'unspecified. From the
> manual:
> 
>   An ‘unspecified’ attribute tells Emacs to refer instead to a parent
>   face

That doesn't cover inheriting from 'unspecified'.  And it really
cannot cover it, because what is a "parent" face when the attribute is
':inherit'?

A comment in xfaces.c:merge_face_vectors says it all:

  static void
  merge_face_vectors (struct frame *f, Lisp_Object *from, Lisp_Object *to,
		      struct named_merge_point *named_merge_points)
  {
    int i;
    Lisp_Object font = Qnil;

    /* If FROM inherits from some other faces, merge their attributes into
       TO before merging FROM's direct attributes.  Note that an :inherit
       attribute of `unspecified' is the same as one of nil; we never
       merge :inherit attributes, so nil is more correct, but lots of
       other code uses `unspecified' as a generic value for face attributes. */

I've now added this caveat to the manual.

> I don't think the ability to customize it should go away, but the system
> in place that allows for the customization just seems odd to me.

IMO it isn't odd, it just didn't mean to affect any face but
'default' or its direct descendants.

> Instead of using an ostensibly redundant :inherit value, why not
> make a customizable list of faces that face remapping also affects?
> 
> Perhaps it's too much work for too little gain.

At this point, it certainly looks like that.