From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 468E56DE0AAB for ; Sun, 25 Sep 2016 05:28:23 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -0.166 X-Spam-Level: X-Spam-Status: No, score=-0.166 tagged_above=-999 required=5 tests=[AWL=-0.145, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tvmBo10iIm1Z for ; Sun, 25 Sep 2016 05:28:22 -0700 (PDT) Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by arlo.cworth.org (Postfix) with ESMTPS id CC91E6DE0AAA for ; Sun, 25 Sep 2016 05:28:21 -0700 (PDT) Received: by mail-wm0-f68.google.com with SMTP id w84so9872113wmg.0 for ; Sun, 25 Sep 2016 05:28:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adirat-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:in-reply-to:references:date:message-id:mime-version; bh=KVe98ea6KA3wVhXjgAy6VwWMpfygmIPmGxhoELAl478=; b=vre6RQBCsbMOoPH1pnuzkdJtpfLSoZq5UkiKx34WsJjefNb2kQczkc4fiOJ/y2dTv0 sJoqw2pFPnLQ9OE+1WBY0q8TrmNnJ0F3MwqZGD1MeWtJMmKsRhAcHhaI2ioY/KmrGMsm RJ+lgJknC87XQ7y533oxfpGfqLcd11iNs0Ru2h2ZjiH+uU7BHbJFgoqArrmf6pUADS3j jTuPZ1qRh5xVKKQ1Pa6fDWZjbjt5hNhBjzY72ZprhDo/r3rc2UVBqKyW7BY0EFPQK55C MQnMN39by0XwnOt7uNmJyAkuEVGC7bdnp/wpn9nlP86CmsPhLZlT0+zMY64mgYBDadRs 6/Ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=KVe98ea6KA3wVhXjgAy6VwWMpfygmIPmGxhoELAl478=; b=dfA9YYHpVMHJsUBImg797qsSTOLMjZe1/85E1Pcc3Eq41oCJwRTdecLgbNeD/UyKNE lhUIvPvchwwEFBObhK5uRQeQ7UT37eP0vKdLEJhiIbiuhczbfSszYVuRuMg+L7Ybi9ok eVnZkBafmRXZr37FIcixq8vQSOKXZ06XSHAho9jphOIVdgvPnqnaq4avdO7Lak6GFEa7 XouuDQvreVcquXazFtj/1K/6hLDSUD+uDksdDc5FPp0FqtS+LiVgvLF3JczZgUZdVA2n rMVY88HNPCAvZwQ24ECDGZpACWSUw05EPtxBNRByVTS0Sv1++3ROhJUj5mN1IA8zmx6G d4bw== X-Gm-Message-State: AA6/9RlwcbbIKHHd/BWHc1gqglwrkU6cOco02wESmWdsld2Pt0w58Owc5f/yzCax6g7d9w== X-Received: by 10.28.135.206 with SMTP id j197mr9620496wmd.31.1474806499910; Sun, 25 Sep 2016 05:28:19 -0700 (PDT) Received: from adiPC ([188.24.78.211]) by smtp.gmail.com with ESMTPSA id u124sm5762809wmu.10.2016.09.25.05.28.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 25 Sep 2016 05:28:19 -0700 (PDT) From: Ioan-Adrian Ratiu To: Mark Walters , notmuch@notmuchmail.org Subject: Re: [PATCH v2 2/4] emacs: notmuch-show: refresh all windows showing a buffer In-Reply-To: <8737kowakz.fsf@qmul.ac.uk> References: <20160924200735.25425-1-adi@adirat.com> <20160924200735.25425-3-adi@adirat.com> <8737kowakz.fsf@qmul.ac.uk> Date: Sun, 25 Sep 2016 15:28:14 +0300 Message-ID: <87wpi0cgf5.fsf@adiPC.i-did-not-set--mail-host-address--so-tickle-me> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 25 Sep 2016 12:28:23 -0000 Hi Mark and thank you again for the great feedback. On Sun, 25 Sep 2016, Mark Walters wrote: > On Sat, 24 Sep 2016, Ioan-Adrian Ratiu wrote: >> This updates all windows displaying a notmuch-show buffer when the >> buffer refresh function is called. >> >> Each window displaying a notmuch-show buffer has its own currently >> displayed messaged based on the (point) location. Store the state >> of all displayed windows when refreshing a notmuch-show buffer and >> re-apply the current shown message for all windows. >> >> Signed-off-by: Ioan-Adrian Ratiu >> --- >> emacs/notmuch-show.el | 19 +++++++++++++------ >> 1 file changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el >> index 641398d..c39065f 100644 >> --- a/emacs/notmuch-show.el >> +++ b/emacs/notmuch-show.el >> @@ -1317,8 +1317,13 @@ If no messages match the query return NIL." >> >> This includes: >> - the list of open messages, >> - - the current message." >> - (list (notmuch-show-get-message-id) (notmuch-show-get-message-ids-for-open-messages))) >> + - the combination of current message id with/for each visible window." >> + (let* ((win-list (get-buffer-window-list (current-buffer) t)) > > Should this be (get-buffer-window-list (current-buffer) nil t)) ? I am > assuming you don't care about the minibuffer, but do want all frames? Yes, exactly. I've forgotten that nil arg. Great catch. > >> + (win-id-combo (mapcar (lambda (win) >> + (with-selected-window win >> + (list win (notmuch-show-get-message-id)))) >> + win-list))) >> + (list win-id-combo (notmuch-show-get-message-ids-for-open-messages)))) > > Before I make a comment here I should stay I rather unsure about how > emacs deals with point when there are multiple windows. I think each > window has a value for point for each buffer regardless of whether that > buffer is currently displayed in that window. Based on all the documentation I could find and code/testing I've done: 1. Each emacs buffer is displayed in a window or not displayed at all. 2. Each window has only one point value which it always displays if the window is visible. 3. Each buffer has a point value which is used only when the buffer is not displayed in any window (used as storage for restoring windows). 4. A window's point value is restored from the buffer point storage value only when the first window switches to a previously undisplayed buffer (so buffer point overwrites window point) 5. A buffer's point value is written with the window point value when the last window displaying said buffer switches to another buffer (so window point overwrites buffer point) 6. When a single window displays a buffer, the window's point and the buffer point are identical (they are kept in sync by the same mechanism above at 5.) I hope I explained this inteligibely :) Based on these rules my code works (of course it can always contain bugs, gotta squash them all). > > If I understand the code correctly this only resets point for the > windows currently displaying buffer. > > I note that this is better than the current refresh-single-buffer code: > however, if you actually want it running on a timer in the background, > rather then you may require better behaviour. As it is improvement on > what we currently do I leave this to you to decide. The problem we have to solve here is that all point values for all windows displaying current-buffer are lost the moment we call erase-buffer because when each window displays an empty buffer after erase, the point is reset, so we need to store them for all windows before erasing the underlying buffer (if we want to restore all windows). The current code in origin/master does not bother with this logic because it only restores one window, so it needs only one current message id (based on point) in the state. What I do is add to state all current messages (points) for every window so we can restore them when applying the state after erase-buffer. We only need to do this for each (window current-message) combination in the state, the other list stored in the state, the open/closed messages list per buffer and identical to all windows. If you have any suggestions on how to modify the commit message to make all of this clearer, they are very welcome :) I usually spend hours figuring out all this logic and it's very hard for me to put it in simple, understandable and concise wording. > > Best wishes > > Mark > > >> (defun notmuch-show-get-query () >> "Return the current query in this show buffer" >> @@ -1345,8 +1350,8 @@ This includes: >> This includes: >> - opening the messages previously opened, >> - closing all other messages, >> - - moving to the correct current message." >> - (let ((current (car state)) >> + - moving to the correct current message in every displayed window." >> + (let ((win-msg-alist (car state)) >> (open (cadr state))) >> >> ;; Open those that were open. >> @@ -1355,8 +1360,10 @@ This includes: >> (member (notmuch-show-get-message-id) open)) >> until (not (notmuch-show-goto-message-next))) >> >> - ;; Go to the previously open message. >> - (notmuch-show-goto-message current))) >> + (dolist (win-msg-pair win-msg-alist) >> + (with-selected-window (car win-msg-pair) >> + ;; Go to the previously open message in this window >> + (notmuch-show-goto-message (cadr win-msg-pair)))))) >> >> (defun notmuch-show-refresh-view (&optional reset-state) >> "Refresh the current view. >> -- >> 2.10.0