From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel 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 Message-ID: <83mvd3mzus.fsf@gnu.org> References: <83y3wqnvol.fsf@gnu.org> <83tw7enq58.fsf@gnu.org> <83r32inna5.fsf@gnu.org> <83bmtlnd4l.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1488470613 26538 195.159.176.226 (2 Mar 2017 16:03:33 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 2 Mar 2017 16:03:33 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Mar 02 17:03:29 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cjTCN-00066x-Ge for ged-emacs-devel@m.gmane.org; Thu, 02 Mar 2017 17:03:23 +0100 Original-Received: from localhost ([::1]:53088 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjTCT-00021r-GB for ged-emacs-devel@m.gmane.org; Thu, 02 Mar 2017 11:03:29 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:45276) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjTBR-00020u-NT for emacs-devel@gnu.org; Thu, 02 Mar 2017 11:02:28 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cjTBO-0004G5-Fz for emacs-devel@gnu.org; Thu, 02 Mar 2017 11:02:25 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:57979) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjTBO-0004G0-DZ; Thu, 02 Mar 2017 11:02:22 -0500 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:1847 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1cjTBN-00065e-P8; Thu, 02 Mar 2017 11:02:22 -0500 In-reply-to: (message from Stefan Monnier on Thu, 02 Mar 2017 00:27:30 -0500) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:212712 Archived-At: > From: Stefan Monnier > 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.?