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

[Stefan Monnier <monnier@iro.umontreal.ca>]

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

I don't think the overlay_arrows_changed_p machinery was adapted
correspondingly, tho.  Actually, looking at diffs around 2005, I don't
see many related changes at all.  I haven't found that discussion yet,
but I wouldn't be surprised if that buffer-local approach already worked
(with the same caveat as we're discussing here), or that the change was
mostly to set_buffer before calling those functions, but that very
little, if any, changes were made to overlay_arrows_changed_p code.

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

The only commits I see which touch this part of the code around that date are:

  commit 025568d9fcdf993a33c53e30a94ae50be1d972a7
  Author: Richard M. Stallman <rms@gnu.org>
  Date:   Thu Sep 15 13:16:53 2005 +0000

    (overlay_arrow_at_row): Add HAVE_WINDOW_SYSTEM conditional.
    (display_mode_element): Instead of `lisp_string' and `this',
    record `offset' and increment that.
    `last_offset' replaces `last'.

  commit 25bcb3535cee413d750730e4894e7a39dbdc839a
  Author: Kim F. Storm <storm@cua.dk>
  Date:   Mon Apr 18 14:10:09 2005 +0000

    (overlay_arrow_string_or_property): Remove PBITMAP arg.
    Calls changed.  Don't check for overlay-arrow-bitmap property here.
    (overlay_arrow_at_row): Remove PBITMAP arg.  Instead, if left
    fringe is present, return Lisp integer for bitmap (or -1 for default).
    Fix value of overlay-arrow-bitmap property to be a symbol, use
    lookup_fringe_bitmap to parse it.
    (display_line): Change call to overlay_arrow_at_row.  Store integer
    return value as overlay bitmap in row rather than window.
    Only show overlay arrow if row displays text, or if no other overlay
    arrow is seen in window (if overlay marker is at point-max).

  commit c514efbbf12b7468322df6badaf88b365a4f52ff
  Author: Kim F. Storm <storm@cua.dk>
  Date:   Sun Oct 17 13:17:00 2004 +0000

    (overlay_arrow_at_row): Return overlay string rather
    than bitmap if there is not left fringe.
    (get_overlay_arrow_glyph_row): Also used on windows system.
    (display_line): Display overlay string if no left fringe.

None of them touches overlay_arrows_changed_p, really.

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

Yes, I was thinking of an API that looks like:

    (register-overlay-arrow-marker MARKER &optional STRING 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.?

I don't understand the question.  Which "these variables" are you
talking about?  Is this comment related to the above suggestion to use
overlays instead, or is it unrelated?  How do you plan on detecting
those changes in order to then record them?


        Stefan