bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Dmitry Gutov]

There is a feature request for diff-hl: to indicate the VCS status of
different lines by using the colors of the line numbers, instead of
fringe or margin indicators.

(https://github.com/dgutov/diff-hl/issues/124)

I think this is not possible yet, but if there was a hook to choose how
a number is displayed, I'd be able to use it.

In GNU Emacs 27.0.50 (build 70, x86_64-pc-linux-gnu, GTK+ Version 
3.22.30, cairo version 1.15.10)
  of 2019-06-19 built on zappa
Repository revision: 922121e7ddbc107da14ea9c1280d15c828e85063
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12001000
System Description: Ubuntu 18.04.2 L

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 2 Jul 2019 14:14:15 +0300
> 
> There is a feature request for diff-hl: to indicate the VCS status of
> different lines by using the colors of the line numbers, instead of
> fringe or margin indicators.
> 
> (https://github.com/dgutov/diff-hl/issues/124)
> 
> I think this is not possible yet, but if there was a hook to choose how
> a number is displayed, I'd be able to use it.

I wouldn't hold my breath, because this will mean any display routine
that calculates layout will have to call some non-trivial Lisp,
possibly even running a subprocess.  Not a very good design, IMO.

Why don't you show these indicators on the fringes or in the display
margins?  That's what those features are there for, and native line
numbers keep their claws off the margins for that very reason.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Dmitry Gutov]

On 02.07.2019 17:30, Eli Zaretskii wrote:

 > I wouldn't hold my breath, because this will mean any display routine
that calculates layout will have to call some non-trivial Lisp,
possibly even running a subprocess.

Not really. In my use case it would only look up overlays on the lines 
corresponding to the numbers.

> Why don't you show these indicators on the fringes or in the display
> margins?

Already do. That's what diff-hl does by default.

I'm relaying a feature request here.

But speaking of fringes and margins, we still don't have a good solution 
for multiple packages to shows indicators on the same line.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Eli Zaretskii]

> Cc: 36472@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 2 Jul 2019 18:09:01 +0300
> 
> On 02.07.2019 17:30, Eli Zaretskii wrote:
> 
>  > I wouldn't hold my breath, because this will mean any display routine
> that calculates layout will have to call some non-trivial Lisp,
> possibly even running a subprocess.
> 
> Not really. In my use case it would only look up overlays on the lines 
> corresponding to the numbers.

I don't understand: who would need to look up overlays?  And how will
"it" know that it needs to look up overlays?

IOW, I don't think I understand the API you had in mind.  You
originally said "a hook", which implies the display engine would call
a hook variable, but now it sounds like you had something else in
mind.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Dmitry Gutov]

On 02.07.2019 18:38, Eli Zaretskii wrote:

> I don't understand: who would need to look up overlays?  And how will
> "it" know that it needs to look up overlays?

The function that diff-hl would add to the said hook. So it will know.

> IOW, I don't think I understand the API you had in mind.  You
> originally said "a hook", which implies the display engine would call
> a hook variable, but now it sounds like you had something else in
> mind.

Something like:

(defvar display-line-number-renderers-functions nil
  "The line number (a string) is mapped through all of the functions in 
this list, in turn. Each receives it as an argument, and then the return 
value is used. The functions are called in the buffer for which the line 
numbers are displayed, at the beginning of a line which corresponds to 
the given number.")

(add-hook 'display-line-number-renderers 'diff-hl-line-number-renderer)

(defun diff-hl-line-number-renderer (line-number-string)
   (cl-case (get-text-property (point) 'diff-hl-indicator-type)
     ...)

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Eli Zaretskii]

> Cc: 36472@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 2 Jul 2019 18:49:38 +0300
> 
> (defvar display-line-number-renderers-functions nil
>   "The line number (a string) is mapped through all of the functions in 
> this list, in turn. Each receives it as an argument, and then the return 
> value is used. The functions are called in the buffer for which the line 
> numbers are displayed, at the beginning of a line which corresponds to 
> the given number.")
> 
> (add-hook 'display-line-number-renderers 'diff-hl-line-number-renderer)
> 
> (defun diff-hl-line-number-renderer (line-number-string)
>    (cl-case (get-text-property (point) 'diff-hl-indicator-type)
>      ...)

I still have some questions:

  . The argument is a line-number string.  You expect the absolute
    line number there?  When the line-number display style is
    'relative' or 'visual', the absolute line number might not be
    available.
  . What kind of object is the return value, and how should the
    display engine use it?
  . You seem to assume the hook will be called at point, but that is
    not true: it will be called where the display engine is scanning
    the buffer.  So you need that position (the beginning of line or
    something) in the interface, or else you will need to calculate
    the position from the line number, not a nice prospect.
  . The display engine is sometimes called to scan buffer positions
    outside of the window, so you should either detect that or be sure
    to place your properties/overlays not only in the window.  Is that
    a problem?
  . What are the triggers for changing these properties/overlays?  Are
    they determined once and for all, or can they change after the
    buffer has been created and populated with the text?  If some
    changes in the buffer affect visual appearance of screen lines
    that are otherwise unaffected by the changes, it would mean
    disabling redisplay optimizations when this feature is used.  For
    example, with 'relative' style, moving point to another line
    requires to redraw all the lines in the window.

In general, calling Lisp from the display engine means complications
and all kinds of silly precautions, to protect ourselves from crazy
Lisp, so I'm still not very fond if the idea, sorry.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Dmitry Gutov]

On 02.07.2019 19:27, Eli Zaretskii wrote:

> I still have some questions:
> 
>    . The argument is a line-number string.  You expect the absolute
>      line number there?  When the line-number display style is
>      'relative' or 'visual', the absolute line number might not be
>      available.

I'm not sure about the best design overall, but for the use case in 
question the line number should already be styled as necessary according 
to the display style. Maybe the styling function should be the first in 
this hook.

>    . What kind of object is the return value, and how should the
>      display engine use it?

A propertized string. With a 'face' or 'font-lock-face' property?

Although if we just make a hook that would return a face to use, that 
would work just as well for me.

>    . You seem to assume the hook will be called at point, but that is
>      not true: it will be called where the display engine is scanning
>      the buffer.  So you need that position (the beginning of line or
>      something) in the interface, or else you will need to calculate
>      the position from the line number, not a nice prospect.

Either the calling code would temporarily change point or, yes, the 
position would be passed to the hook functions. Either is fine by me.

>    . The display engine is sometimes called to scan buffer positions
>      outside of the window, so you should either detect that or be sure
>      to place your properties/overlays not only in the window.  Is that
>      a problem?

No, the overlays should be present everywhere by that point.

>    . What are the triggers for changing these properties/overlays?  Are
>      they determined once and for all, or can they change after the
>      buffer has been created and populated with the text?

Normally they are changed in after-save-hook. But there is an option 
that makes that happen on a timer.

> If some
>      changes in the buffer affect visual appearance of screen lines
>      that are otherwise unaffected by the changes, it would mean
>      disabling redisplay optimizations when this feature is used.  For
>      example, with 'relative' style, moving point to another line
>      requires to redraw all the lines in the window.

Well, I'm not a pro on the display engine, but shouldn't redisplay 
happen anyway when those overlays are added/modified?

Because right now they set fringe bitmaps using the before-string 
overlay property. And that should cause redisplay anyway.

> In general, calling Lisp from the display engine means complications
> and all kinds of silly precautions, to protect ourselves from crazy
> Lisp, so I'm still not very fond if the idea, sorry.

It's okay, I'm not too invested to the idea personally. But if the 
option were available, I'd whip up integration pretty quickly.

But on that subject, maybe it'd be fine to just document what the 
functions on the new hook are allowed and not allowed to do. And then 
see if we really have to add actual restrictions to force third-party 
code to behave.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Eli Zaretskii]

> Cc: 36472@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 3 Jul 2019 17:19:09 +0300
> 
> On 02.07.2019 19:27, Eli Zaretskii wrote:
> 
> > I still have some questions:
> > 
> >    . The argument is a line-number string.  You expect the absolute
> >      line number there?  When the line-number display style is
> >      'relative' or 'visual', the absolute line number might not be
> >      available.
> 
> I'm not sure about the best design overall, but for the use case in 
> question the line number should already be styled as necessary according 
> to the display style. Maybe the styling function should be the first in 
> this hook.

There's some misunderstanding here, perhaps mine.  What I wanted to
say is that you may get a relative line-number string such as "-2",
which will probably tell you nothing about the position of that line.
IOW, a line number is not a good API design in this case, because the
display engine doesn't always know the absolute line number of each
line, whereas your function must have an unambiguous descriptor of the
line's location.

> >    . What kind of object is the return value, and how should the
> >      display engine use it?
> 
> A propertized string. With a 'face' or 'font-lock-face' property?
> 
> Although if we just make a hook that would return a face to use, that 
> would work just as well for me.

A face would be much easier to use from the display engine, I think.
I assume the face attributes can only specify colors?

> >    . You seem to assume the hook will be called at point, but that is
> >      not true: it will be called where the display engine is scanning
> >      the buffer.  So you need that position (the beginning of line or
> >      something) in the interface, or else you will need to calculate
> >      the position from the line number, not a nice prospect.
> 
> Either the calling code would temporarily change point

That's an absolute no-no for the display engine, because such changes
sooner or later leak to userland and cause adverse effects.

> >    . What are the triggers for changing these properties/overlays?  Are
> >      they determined once and for all, or can they change after the
> >      buffer has been created and populated with the text?
> 
> Normally they are changed in after-save-hook. But there is an option 
> that makes that happen on a timer.

Ouch!  Another performance killer.

> > If some
> >      changes in the buffer affect visual appearance of screen lines
> >      that are otherwise unaffected by the changes, it would mean
> >      disabling redisplay optimizations when this feature is used.  For
> >      example, with 'relative' style, moving point to another line
> >      requires to redraw all the lines in the window.
> 
> Well, I'm not a pro on the display engine, but shouldn't redisplay 
> happen anyway when those overlays are added/modified?
> 
> Because right now they set fringe bitmaps using the before-string 
> overlay property. And that should cause redisplay anyway.

Redisplay should and does happen, but it tries very hard to determine
which portions of the window actually need to be redrawn.  For some
features, the answer is "the entire window", and that makes redisplay
slower.  My question was how local are the changes caused by this
feature, i.e. could it happen that changes in some place in a buffer
cause changes on display in remote places?

> But on that subject, maybe it'd be fine to just document what the 
> functions on the new hook are allowed and not allowed to do. And then 
> see if we really have to add actual restrictions to force third-party 
> code to behave.

I don't know about "allowed".  Would it be reasonable to say don't
switch buffers and/or don't select another window?  There are also
things you cannot really disallow, because the caller doesn't know
enough about what happens under the hood and doesn't control that.
For example, if the Lisp function calls vertical-motion or
posn-at-point, that invokes display routines, so the code in question
could be re-entered; but how can we tell Lisp programmers "don't call
anything that could call vertical-motion"?  And where to put such
limitations for them to be visible and discoverable enough in the
first place?

So I think imposing such limitations is impractical, and the only
reasonable thing to do whenever we call Lisp is take all the
precautions that assume the worst.  We had quite a few bugs recently
that were caused by not taking all the precautions.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Dmitry Gutov]

On 03.07.2019 19:14, Eli Zaretskii wrote:

> There's some misunderstanding here, perhaps mine.  What I wanted to
> say is that you may get a relative line-number string such as "-2",
> which will probably tell you nothing about the position of that line.

Like I wrote in the example docstring, the hook functions would be 
called with point temporary changed to the corresponding line's 
beginning. You didn't like that, so I suggested the position would be 
passed as a second parameter to the functions. Or it can be a dynamic 
variable. Whichever option looks best to you.

> IOW, a line number is not a good API design in this case, because the
> display engine doesn't always know the absolute line number of each
> line, whereas your function must have an unambiguous descriptor of the
> line's location.

The display engine doesn't, but the display-line-numbers feature clearly 
does. Knowing line numbers is in its job description.

Or are you hinting at some optimization where, when the style is 
`relative', it doesn't bother to compute the absolute numbers?

Anyway, it doesn't matter for this particular use case: I only need 
line-beginning-position, not its absolute number.

>> Although if we just make a hook that would return a face to use, that
>> would work just as well for me.
> 
> A face would be much easier to use from the display engine, I think.
> I assume the face attributes can only specify colors?

Sure. Although some other users of this hook could also want to specify 
other properties. Maybe we'll want to combine the return values?

>> Either the calling code would temporarily change point
> 
> That's an absolute no-no for the display engine, because such changes
> sooner or later leak to userland and cause adverse effects.

OK.

>> Normally they are changed in after-save-hook. But there is an option
>> that makes that happen on a timer.
> 
> Ouch!  Another performance killer.

That feature already exists, you know. And it updates fringe or margin 
indicators. People seem to like it. In my limited testing, I haven't 
observed any major slowdowns.

> Redisplay should and does happen, but it tries very hard to determine
> which portions of the window actually need to be redrawn.  For some
> features, the answer is "the entire window", and that makes redisplay
> slower.

So is there an optimization that looks at new overlays, checks that it 
only has a before-string with a fringe spec, and then only updates the 
fringe?

> My question was how local are the changes caused by this
> feature, i.e. could it happen that changes in some place in a buffer
> cause changes on display in remote places?

In my case, the hook function would just look up a property on overlays 
at bol. Thus no far-reaching changes. But it's hard to guarantee, API-wise.

>> But on that subject, maybe it'd be fine to just document what the
>> functions on the new hook are allowed and not allowed to do. And then
>> see if we really have to add actual restrictions to force third-party
>> code to behave.
> 
> I don't know about "allowed".  Would it be reasonable to say don't
> switch buffers and/or don't select another window?

Sure, I guess. Though I would like to know why a temporary changes in 
the current buffer or the selected window would be so bad.

> There are also
> things you cannot really disallow, because the caller doesn't know
> enough about what happens under the hood and doesn't control that.
> For example, if the Lisp function calls vertical-motion or
> posn-at-point, that invokes display routines, so the code in question
> could be re-entered; but how can we tell Lisp programmers "don't call
> anything that could call vertical-motion"?  And where to put such
> limitations for them to be visible and discoverable enough in the
> first place?

I suppose the display code could check for re-entrance (e.g. by setting 
a variable at the beginning of the redisplay routine) and abort any such 
attempts with a Lisp-level error. Thus the limitation would be enforced 
at runtime, which is not perfect, but if the error is intelligible, it 
shouldn't be hard for a programmer to understand the reasons and change 
their code.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Eli Zaretskii]

> Cc: 36472@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 8 Jul 2019 02:46:50 +0300
> 
> > IOW, a line number is not a good API design in this case, because the
> > display engine doesn't always know the absolute line number of each
> > line, whereas your function must have an unambiguous descriptor of the
> > line's location.
> 
> The display engine doesn't, but the display-line-numbers feature clearly 
> does. Knowing line numbers is in its job description.
> 
> Or are you hinting at some optimization where, when the style is 
> `relative', it doesn't bother to compute the absolute numbers?

Yes.  In particular, in the 'visual' style.

> Anyway, it doesn't matter for this particular use case: I only need 
> line-beginning-position, not its absolute number.

Right.

> > I assume the face attributes can only specify colors?
> 
> Sure. Although some other users of this hook could also want to specify 
> other properties. Maybe we'll want to combine the return values?

If the face attributes make characters wider or narrower than the
faces of other numbers, you will have unpleasant horizontal movement
of the line's text.  Colors can never cause this.

> >> Normally they are changed in after-save-hook. But there is an option
> >> that makes that happen on a timer.
> > 
> > Ouch!  Another performance killer.
> 
> That feature already exists, you know. And it updates fringe or margin 
> indicators. People seem to like it. In my limited testing, I haven't 
> observed any major slowdowns.

I meant it will be a slow-down for redisplay.

> > Redisplay should and does happen, but it tries very hard to determine
> > which portions of the window actually need to be redrawn.  For some
> > features, the answer is "the entire window", and that makes redisplay
> > slower.
> 
> So is there an optimization that looks at new overlays, checks that it 
> only has a before-string with a fringe spec, and then only updates the 
> fringe?

Some of that.  We know whether the overlays changed, and we have tests
for whether the fringe needs to be updated on a per-line basis.

> > My question was how local are the changes caused by this
> > feature, i.e. could it happen that changes in some place in a buffer
> > cause changes on display in remote places?
> 
> In my case, the hook function would just look up a property on overlays 
> at bol. Thus no far-reaching changes.

What events trigger changes in that property?

> But it's hard to guarantee, API-wise.

Yes, this is always a concern with such hooks.

> > I don't know about "allowed".  Would it be reasonable to say don't
> > switch buffers and/or don't select another window?
> 
> Sure, I guess. Though I would like to know why a temporary changes in 
> the current buffer or the selected window would be so bad.

Because when the display engine works on a window, it always makes
that window's buffer the current buffer.  If some Lisp we call changes
that, redisplay will be confused.  So we need to save and restore the
current buffer (and sometimes also the selected frame), to avoid such
problems.

> > There are also
> > things you cannot really disallow, because the caller doesn't know
> > enough about what happens under the hood and doesn't control that.
> > For example, if the Lisp function calls vertical-motion or
> > posn-at-point, that invokes display routines, so the code in question
> > could be re-entered; but how can we tell Lisp programmers "don't call
> > anything that could call vertical-motion"?  And where to put such
> > limitations for them to be visible and discoverable enough in the
> > first place?
> 
> I suppose the display code could check for re-entrance (e.g. by setting 
> a variable at the beginning of the redisplay routine) and abort any such 
> attempts with a Lisp-level error. Thus the limitation would be enforced 
> at runtime, which is not perfect, but if the error is intelligible, it 
> shouldn't be hard for a programmer to understand the reasons and change 
> their code.

Errors signaled during redisplay are caught and only logged in
*Messages*, because displaying them would re-enter redisplay and cause
the same error again.  So such errors are not very visible, and could
go undetected for a long time.  IOW, it is better to (tediously)
protect ourselves than signal errors in such situations.

bug#36472: 27.0.50; Convey information by showing line numbers using different colors?

[Dmitry Gutov]

On 08.07.2019 15:23, Eli Zaretskii wrote:

> If the face attributes make characters wider or narrower than the
> faces of other numbers, you will have unpleasant horizontal movement
> of the line's text.  Colors can never cause this.

Makes sense.

>>>> Normally they are changed in after-save-hook. But there is an option
>>>> that makes that happen on a timer.
>>>
>>> Ouch!  Another performance killer.
>>
>> That feature already exists, you know. And it updates fringe or margin
>> indicators. People seem to like it. In my limited testing, I haven't
>> observed any major slowdowns.
> 
> I meant it will be a slow-down for redisplay.

Not sure I understand you here. Redisplay would be slowed down by 
waiting for timers?

>> So is there an optimization that looks at new overlays, checks that it
>> only has a before-string with a fringe spec, and then only updates the
>> fringe?
> 
> Some of that.  We know whether the overlays changed, and we have tests
> for whether the fringe needs to be updated on a per-line basis.

If it's really making a noticeable change, maybe we could standardize on 
a text property which would indicate which colors to use for the 
display-line-numbers area. That would circumvent the whole issue of 
running Lisp in redisplay.

>>> My question was how local are the changes caused by this
>>> feature, i.e. could it happen that changes in some place in a buffer
>>> cause changes on display in remote places?
>>
>> In my case, the hook function would just look up a property on overlays
>> at bol. Thus no far-reaching changes.
> 
> What events trigger changes in that property?

Like I said, saving a buffer, or a timer firing (depending on whether a 
certain modification in enabled). Also the user can make an overlay 
outdated by typing, so the highlighting is removed right away in such 
cases (only for the closest hunk).

> Because when the display engine works on a window, it always makes
> that window's buffer the current buffer.  If some Lisp we call changes
> that, redisplay will be confused.  So we need to save and restore the
> current buffer (and sometimes also the selected frame), to avoid such
> problems.

But if any such change is temporary (e.g. using with-current-buffer), 
it's all well, right? In that case, such limitation sounds very 
reasonable to me.

> Errors signaled during redisplay are caught and only logged in
> *Messages*, because displaying them would re-enter redisplay and cause
> the same error again.  So such errors are not very visible, and could
> go undetected for a long time.  IOW, it is better to (tediously)
> protect ourselves than signal errors in such situations.

I don't know, I still think it's better for the programmer to see the 
error to be able to fix their code. Even if it's only in *Messages*. The 
accompanying simplification of error handling sounds beneficial to me as 
well.