From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Stefan Monnier 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 00:27:30 -0500 Message-ID: References: <83y3wqnvol.fsf@gnu.org> <83tw7enq58.fsf@gnu.org> <83r32inna5.fsf@gnu.org> <83bmtlnd4l.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1488432517 19260 195.159.176.226 (2 Mar 2017 05:28:37 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 2 Mar 2017 05:28:37 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Mar 02 06:28:31 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 1cjJHq-0003Lu-QG for ged-emacs-devel@m.gmane.org; Thu, 02 Mar 2017 06:28:24 +0100 Original-Received: from localhost ([::1]:50471 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjJHv-0000LG-5T for ged-emacs-devel@m.gmane.org; Thu, 02 Mar 2017 00:28:27 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:53737) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjJHM-0000L9-4x for emacs-devel@gnu.org; Thu, 02 Mar 2017 00:27:53 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cjJHL-0007ur-2n for emacs-devel@gnu.org; Thu, 02 Mar 2017 00:27:52 -0500 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:33652) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cjJHH-0007rx-4Q; Thu, 02 Mar 2017 00:27:47 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0AoCQD5rLdY/zPujBhdHAEBBAEBCgEBgycpQYQ4i0uQcykBkDiEU4INhhwEAgKCOEEXAQIBAQEBAQEBYihCDgGEIAEEAVYjBQsLNBIUGA0kigUIs26LGAEBAQcCASWLO4o5BYZmiW6LVI0/jzGGYEiSbiEBNYEBIRQILD6ETx0ZgWYigXeIPAEBAQ X-IPAS-Result: A0AoCQD5rLdY/zPujBhdHAEBBAEBCgEBgycpQYQ4i0uQcykBkDiEU4INhhwEAgKCOEEXAQIBAQEBAQEBYihCDgGEIAEEAVYjBQsLNBIUGA0kigUIs26LGAEBAQcCASWLO4o5BYZmiW6LVI0/jzGGYEiSbiEBNYEBIRQILD6ETx0ZgWYigXeIPAEBAQ X-IronPort-AV: E=Sophos;i="5.35,228,1484024400"; d="scan'208";a="294238553" Original-Received: from 24-140-238-51.cpe.teksavvy.com (HELO pastel.home) ([24.140.238.51]) by smtp.teksavvy.com with ESMTP; 02 Mar 2017 00:27:31 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id 2C6986565E; Thu, 2 Mar 2017 00:27:30 -0500 (EST) In-Reply-To: <83bmtlnd4l.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 01 Mar 2017 19:03:06 +0200") X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.181 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:212690 Archived-At: > In any case, if you expect a buffer shown in a non-selected window to > be redisplayed just because its overlay-arrow was modified, this never > worked in Emacs. Hmm.... I see. Then I guess I really understand even less about this code than I thought. > 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). > 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`. Another option is to change overlay_arrows_changed_p so that it checks every var to see if it's been made local to a buffer, and if so, loop though all buffers to check if it changed in any of them (and save the last-arrow-position per buffer). > Changes in overlay-arrows always triggered a thorough redisplay. That's true for changes to overlay-arrows stored in non-buffer-local variables, yes. And it was so thorough that it caused re-rendering of every single window, regardless if it has/had an overlay-arrow in it. 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. 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. - 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). Stefan