Re: Native line numbers landed on master

[Eli Zaretskii]

> From: Juanma Barranquero <address@hidden>
> Date: Tue, 1 Oct 2019 07:44:47 +0200
> Cc: address@hidden, Emacs developers <address@hidden>
> 
> > Patches welcome.
> 
> To quote Barney Stinson, "Challenge accepted!"

Thanks.

> > I don't think it should be too hard to write that.
> 
> Didn't know you were an optimist.

I am not.

> Ok. The following extremely simple patch (with no documentation, etc, and
> horrible, *horrible* face names because I couldn't think of anything
> better) seems to work in the various tests I've done in absolute, visual
> and relative modes.

Ideas for better face names are welcome.  I can suggest:

  line-number-tenth-line
  line-number-fifth-line

(the latter is inaccurate, but a doc string can explain the subtlety).

> Now, I'm not sure about code (try_cursor_movement and try_window_id) that
> compare the line-number and line-number-current-line faces for
> optimization. Performance seems to be fine, and I haven't seen any obvious
> visual glitch.
> 
> Also, this in maybe_produce_line_number:
> 
>   /* Compute point's line number if needed.  */
>   if ((EQ (Vdisplay_line_numbers, Qrelative)
>        || EQ (Vdisplay_line_numbers, Qvisual)
>        || lnum_face_id != current_lnum_face_id)
>       && !it->pt_lnum)
> 
> should perhaps compare curremt_lnum_face_id also with the face_ids of the
> two new faces. ?

I don't think so.  Both the issue with disabling try_cursor_movement
and try_window_id, and the issue with the above 'if' clause are
because the line-number-current-line face is used for the _current_
line, whose position is not fixed wrt to the rest of the lines, but
changes due to moving point.  For that reason, any point movement
means the new and the old "current" lines need to be redrawn, even
though neither their text nor their line numbers have changed.  By
contrast, the 5th and 10th lines' positions wrt to the rest of the
lines are fixed, so point movement doesn't require to redraw these
lines.

The above is the theory, but I understand you didn't see any redisplay
problems without those changes?  If so, the theory is correct.

I suggest to wait for a week or so, to let people try the patch and
suggest better names for the faces, then push this.