[PATCH v2 2/4] emacs: notmuch-show: refresh all windows showing a buffer

[Ioan-Adrian Ratiu]

Hi Mark and thank you again for the great feedback.

On Sun, 25 Sep 2016, Mark Walters <markwalters1009 at gmail.com> wrote:
> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi at adirat.com> 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 <adi at adirat.com>
>> ---
>>  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