Re: [Emacs-diffs] master fd8f724: * src/xdisp.c (overlay_arrows_changed_p): Fix last change.

[Eli Zaretskii]

> From: Stefan Monnier <address@hidden>
> Cc: address@hidden
> Date: Thu, 02 Mar 2017 00:27:30 -0500
> 
> > I think it assumes that at most one buffer changes its overlay-arrows
> > between two redisplay cycles.
> 
> Given that it was used to:
> 
> - cause all windows to be considered for refresh when any overlay-arrow moves
> - disable the try_window_id optimisation, i.e. force every window's
>   display matrices to be re-rendered.
> 
> My impression is that it was specifically designed to handle the case
> where an overlay arrow is moved in another window than the currently
> selected window.  And I think it does that more or less correctly as
> long as none of the vars on the list are made buffer-local (I guess it
> would still fail in to refresh things in the corner case where one of
> the markers is moved from one buffer to another while keeping the same
> numeric position).

I don't think this is all there is to it, because this stuff was
specifically designed to support buffer-local values of overlay-arrow
related variables.  You will find a long discussion in Mar 2005 which
led to this; that discussion ended by all the interested parties
declaring a victory, i.e. that it was then possible to have more than
one overlay-arrow by using several distinct buffer-local values.
(Months later the ELisp manual was updated to say that it's okay to
use buffer-local values for these variables.)

> > To fix this thoroughly, we probably need to add
> > overlay-arrow-position, overlay-arrow-string, and overlay-arrow-bitmap
> > to the list of variables at the end of frame.el which trigger
> > redisplay of their respective buffers.
> 
> But that won't catch the case where those vars aren't modified and
> instead, the marker's position is simply modified by `move-marker`.

Then the markers will have to be watched as well.

> The intent of my patch was to refine this so only those windows which
> have an overlay-arrow that changed gets re-rendered.  I think it does
> work correctly *if* none of the vars were made buffer-local.

But being able to make them buffer-local was an explicit goal of the
Mar 2005 changes...

> Maybe instead of getting redisplay to pay closer attention to
> overlay-arrow-variable-list so as to catch all the cases, we can replace
> this with another mechanism which make the redisplay's job easier.
> I can see two such options:
> - add some kind of function to register a new marker as being an
>   overlay-arrow.  I.e. instead of putting the marker into one of the
>   variables in overlay-arrow-variable-list, you'd call that function
>   (call it `register-overlay-arrow-marker`).  Then redisplay can keep an
>   internal list of "overlay-arrow markers" and overlay_arrows_changed_p
>   can loop through that list to see if one of them changed, without
>   having to worry about buffer-local variables.

We need to react not only to the markers, but also to
overlay-arrow-string and overlay-arrow-bitmap.

> - use overlays instead of markers for overlay-arrows: all
>   overlay modification functions already trigger the needed redisplay,
>   so we could get rid of overlay_arrows_changed_p altogether.
>   The downside is that overlay_arrow_at_row would have to look through
>   all the overlays looking for one with the magic property, but assuming
>   we keep a constraint like "overlay-arrow overlays must start at the
>   beginning of the line where the arrow will be displayed", it should be
>   cheap enough (i.e. a single call to something like overlays-at).

Isn't it easier to keep the record of changes in these variables in
the buffer object, like we do with BUF_MODIFF etc.?