unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: emacs-devel@gnu.org
Subject: Re: [Emacs-diffs] master fd8f724: * src/xdisp.c (overlay_arrows_changed_p): Fix last change.
Date: Thu, 02 Mar 2017 18:02:03 +0200	[thread overview]
Message-ID: <83mvd3mzus.fsf@gnu.org> (raw)
In-Reply-To: <jwvy3woxqis.fsf-monnier+emacs@gnu.org> (message from Stefan Monnier on Thu, 02 Mar 2017 00:27:30 -0500)

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> 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.?



  reply	other threads:[~2017-03-02 16:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-28 16:10 [Emacs-diffs] master fd8f724: * src/xdisp.c (overlay_arrows_changed_p): Fix last change Eli Zaretskii
2017-02-28 17:47 ` Stefan Monnier
2017-02-28 18:09   ` Eli Zaretskii
2017-02-28 18:58     ` Stefan Monnier
2017-02-28 19:11       ` Eli Zaretskii
2017-02-28 20:02         ` Stefan Monnier
2017-03-01 17:03           ` Eli Zaretskii
2017-03-02  5:27             ` Stefan Monnier
2017-03-02 16:02               ` Eli Zaretskii [this message]
2017-03-02 18:29                 ` Stefan Monnier
2017-03-02 20:18                   ` Eli Zaretskii
2017-03-02 22:32                     ` Stefan Monnier
2017-03-03  8:11                       ` Eli Zaretskii
2017-03-06  2:53                         ` Stefan Monnier
2017-03-03  8:13                   ` Eli Zaretskii
2017-03-06  4:17                     ` Stefan Monnier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83mvd3mzus.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).