[PATCH v2 0/4] Add refresh all buffers functionality

[PATCH v2 0/4] Add refresh all buffers functionality

[Ioan-Adrian Ratiu]

Changes since v1 (thank you Mark for your feedback):
    * Major rewrite/simplification of the notmuch-show refresh patch to
    support simultaneous refreshing multiple windows displaying a buffer
    * Removed the notmuch-show-message-adjust () patch because it's not
    needed after the above rewrite
    * Rewrote notmuch-refresh-all-buffers () to use dolist instead of
    while loop
    * Minor commit message/metadata improvements

This patch series adds a function to refresh all buffers, including an
option to silently refresh them in the background i.e. to not show the
newly refreshed buffer in any window.

This is very useful for asynchronously updating all buffers when new
mail arrives, using logic similar to the following (it's what I use):

(setq process-connection-type nil)

(defun done-index-sentinel (process event)
  (notmuch-refresh-all-buffers t)
  (message "Mail sync complete"))

(defun done-sync-sentinel (process event)
  (message "Indexing mail using notmuch")
  (set-process-sentinel (start-process "notmuch" nil "notmuch" "new")
			'done-index-sentinel))

(defun run-mail-sync ()
  (message "Syncing mail in background")
  (set-process-sentinel (start-process "mbsync" nil "mbsync" "gmail")
			  'done-sync-sentinel))

(run-with-idle-timer 600 nil 'run-mail-sync)

Ioan-Adrian Ratiu (4):
  emacs: reuse buffer when refreshing searches
  emacs: notmuch-show: refresh all windows showing a buffer
  emacs: add refresh buffer optional no-display arg
  emacs: notmuch-lib: add refresh all buffers function

 emacs/notmuch-lib.el  | 22 +++++++++++++++++++---
 emacs/notmuch-show.el | 19 +++++++++++++------
 emacs/notmuch.el      | 24 ++++++++++++++++--------
 3 files changed, 48 insertions(+), 17 deletions(-)

-- 
2.10.0

[PATCH v2 1/4] emacs: reuse buffer when refreshing searches

[Ioan-Adrian Ratiu]

There's no reason to completely kill a buffer while refreshing its
search results because the buffer name is constant between refreshes
(based on the search query), only its contents may change and notmuch
search kills all local variables, so it's safe to reuse.

Reusing the same buffer also makes it possible to do things like
refreshing a buffer which is not focused or even not shown in any
window - this will be used in the next patches to add auto-refresh
capabilities to all existing notmuch buffers + a function to call
after syncing mail to refresh everything.

Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
---
 emacs/notmuch.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 8e14692..05687b7 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -984,7 +984,7 @@ the configured default sort order."
 (defun notmuch-search-refresh-view ()
   "Refresh the current view.
 
-Kills the current buffer and runs a new search with the same
+Erases the current buffer and runs a new search with the same
 query string as the current search. If the current thread is in
 the new search results, then point will be placed on the same
 thread. Otherwise, point will be moved to attempt to be in the
@@ -992,8 +992,9 @@ same relative position within the new buffer."
   (let ((target-line (line-number-at-pos))
 	(oldest-first notmuch-search-oldest-first)
 	(target-thread (notmuch-search-find-thread-id 'bare))
-	(query notmuch-search-query-string))
-    (notmuch-bury-or-kill-this-buffer)
+	(query notmuch-search-query-string)
+	(inhibit-read-only t))
+    (erase-buffer)
     (notmuch-search query oldest-first target-thread target-line)
     (goto-char (point-min))))
 
-- 
2.10.0

Re: [PATCH v2 1/4] emacs: reuse buffer when refreshing searches

[Mark Walters]

On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
> There's no reason to completely kill a buffer while refreshing its
> search results because the buffer name is constant between refreshes
> (based on the search query), only its contents may change and notmuch
> search kills all local variables, so it's safe to reuse.
>
> Reusing the same buffer also makes it possible to do things like
> refreshing a buffer which is not focused or even not shown in any
> window - this will be used in the next patches to add auto-refresh
> capabilities to all existing notmuch buffers + a function to call
> after syncing mail to refresh everything.
>
> Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
> ---
>  emacs/notmuch.el | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 8e14692..05687b7 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -984,7 +984,7 @@ the configured default sort order."
>  (defun notmuch-search-refresh-view ()
>    "Refresh the current view.
>  
> -Kills the current buffer and runs a new search with the same
> +Erases the current buffer and runs a new search with the same
>  query string as the current search. If the current thread is in
>  the new search results, then point will be placed on the same
>  thread. Otherwise, point will be moved to attempt to be in the
> @@ -992,8 +992,9 @@ same relative position within the new buffer."
>    (let ((target-line (line-number-at-pos))
>  	(oldest-first notmuch-search-oldest-first)
>  	(target-thread (notmuch-search-find-thread-id 'bare))
> -	(query notmuch-search-query-string))
> -    (notmuch-bury-or-kill-this-buffer)
> +	(query notmuch-search-query-string)
> +	(inhibit-read-only t))
> +    (erase-buffer)

Hi

Something I missed previously: I think erase-buffer does not remove
overlays (see the comment in notmuch-show-refresh-view). Thus it is
probably worth removing them manually. I don't think there will be any
visual artefacts in this case, but I think emacs's speed is quadratic in
the number of overlays.

Anyway I suggest adding a (remove-overlays) before or after the
erase-buffer.

Best wishes

Mark


>      (notmuch-search query oldest-first target-thread target-line)
>      (goto-char (point-min))))
>  
> -- 
> 2.10.0

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

[Ioan-Adrian Ratiu]

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@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))
+	 (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))))
 
 (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

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

[Mark Walters]

On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@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@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?

> +	 (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.

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.

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

Re: [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@gmail.com> wrote:
> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@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@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

[PATCH v2 3/4] emacs: add refresh buffer optional no-display arg

[Ioan-Adrian Ratiu]

Add an optional no-display arg to the generic buffer refresh function,
notmuch-refresh-this-buffer, which works the same way as notmuch-hello
mode's notmuch-hello-update no-display arg.

The idea is for the generic notmuch-refresh-this-buffer to pass down
this arg to the "mode-specific" refresh functions to be able to update
buffers without bringing them to the foreground (if they are already
foregrounded, i.e. displayed in a window, this has no effect).

When updating a search buffer, notmuch currently always brings results
in a window to the foreground. Perhaps counter-intuitively, we do not
want this behaviour necessarily, because we want to auto-refresh any
kind of search buffers, even those backgrounded (not displayed in any
window/frame) from previous searches. This is why we add a no-display
arg to notmuch-search.

We do this to show which mails have appeard or dissapeared since the
last search refresh and have this information updated in real time
even when switching buffers. The ultimate goal of this is to have all
notmuch buffers auto-refresh when the email client syncs (this function
is added in the next patch).

Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
---
 emacs/notmuch-lib.el | 10 +++++++---
 emacs/notmuch.el     | 17 ++++++++++++-----
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 2f015b0..6618365 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -409,14 +409,18 @@ of its command symbol."
   "Function to call to refresh the current buffer.")
 (make-variable-buffer-local 'notmuch-buffer-refresh-function)
 
-(defun notmuch-refresh-this-buffer ()
-  "Refresh the current buffer."
+(defun notmuch-refresh-this-buffer (&optional no-display)
+  "Refresh the current buffer.
+
+If no-display is non-nil do not try to bring the buffer to the
+foreground. If the buffer is already foregrounded i.e. displayed
+in a window on screen, no-display has no effect."
   (interactive)
   (when notmuch-buffer-refresh-function
     (if (commandp notmuch-buffer-refresh-function)
 	;; Pass prefix argument, etc.
 	(call-interactively notmuch-buffer-refresh-function)
-      (funcall notmuch-buffer-refresh-function))))
+      (funcall notmuch-buffer-refresh-function no-display))))
 
 (defun notmuch-poll-and-refresh-this-buffer ()
   "Invoke `notmuch-poll' to import mail, then refresh the current buffer."
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 05687b7..ec7a242 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -924,7 +924,7 @@ PROMPT is the string to prompt with."
 
 ;;;###autoload
 (put 'notmuch-search 'notmuch-doc "Search for messages.")
-(defun notmuch-search (&optional query oldest-first target-thread target-line)
+(defun notmuch-search (&optional query oldest-first target-thread target-line no-display)
   "Display threads matching QUERY in a notmuch-search buffer.
 
 If QUERY is nil, it is read interactively from the minibuffer.
@@ -935,6 +935,9 @@ Other optional parameters are used as follows:
                  current if it appears in the search results.
   TARGET-LINE: The line number to move to if the target thread does not
                appear in the search results.
+  NO-DISPLAY: Do not try to foreground the search results buffer. If it is
+              already foregrounded i.e. displayed in a window, this has no
+              effect, meaning the buffer will remain visible.
 
 When called interactively, this will prompt for a query and use
 the configured default sort order."
@@ -948,7 +951,9 @@ the configured default sort order."
 
   (let* ((query (or query (notmuch-read-query "Notmuch search: ")))
 	 (buffer (get-buffer-create (notmuch-search-buffer-title query))))
-    (switch-to-buffer buffer)
+    (if no-display
+	(set-buffer buffer)
+      (switch-to-buffer buffer))
     (notmuch-search-mode)
     ;; Don't track undo information for this buffer
     (set 'buffer-undo-list t)
@@ -981,21 +986,23 @@ the configured default sort order."
 	  (set-process-query-on-exit-flag proc nil))))
     (run-hooks 'notmuch-search-hook)))
 
-(defun notmuch-search-refresh-view ()
+(defun notmuch-search-refresh-view (&optional no-display)
   "Refresh the current view.
 
 Erases the current buffer and runs a new search with the same
 query string as the current search. If the current thread is in
 the new search results, then point will be placed on the same
 thread. Otherwise, point will be moved to attempt to be in the
-same relative position within the new buffer."
+same relative position within the new buffer. If no-display is
+non-nil, the search results buffer will not be foregrounded, if
+it already is displayed in a window, then no-display has no effect."
   (let ((target-line (line-number-at-pos))
 	(oldest-first notmuch-search-oldest-first)
 	(target-thread (notmuch-search-find-thread-id 'bare))
 	(query notmuch-search-query-string)
 	(inhibit-read-only t))
     (erase-buffer)
-    (notmuch-search query oldest-first target-thread target-line)
+    (notmuch-search query oldest-first target-thread target-line no-display)
     (goto-char (point-min))))
 
 (defun notmuch-search-toggle-order ()
-- 
2.10.0

Re: [PATCH v2 3/4] emacs: add refresh buffer optional no-display arg

[Mark Walters]

On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
> Add an optional no-display arg to the generic buffer refresh function,
> notmuch-refresh-this-buffer, which works the same way as notmuch-hello
> mode's notmuch-hello-update no-display arg.
>
> The idea is for the generic notmuch-refresh-this-buffer to pass down
> this arg to the "mode-specific" refresh functions to be able to update
> buffers without bringing them to the foreground (if they are already
> foregrounded, i.e. displayed in a window, this has no effect).
>
> When updating a search buffer, notmuch currently always brings results
> in a window to the foreground. Perhaps counter-intuitively, we do not
> want this behaviour necessarily, because we want to auto-refresh any
> kind of search buffers, even those backgrounded (not displayed in any
> window/frame) from previous searches. This is why we add a no-display
> arg to notmuch-search.
>
> We do this to show which mails have appeard or dissapeared since the
> last search refresh and have this information updated in real time
> even when switching buffers. The ultimate goal of this is to have all
> notmuch buffers auto-refresh when the email client syncs (this function
> is added in the next patch).
>
> Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
> ---
>  emacs/notmuch-lib.el | 10 +++++++---
>  emacs/notmuch.el     | 17 ++++++++++++-----
>  2 files changed, 19 insertions(+), 8 deletions(-)

I feel I am missing something here: why do you not need to change
notmuch-show-refresh-view and notmuch-tree-refresh-view as well? Note
notmuch-show-refresh-view already has an optional argument.

Best wishes

Mark

>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 2f015b0..6618365 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -409,14 +409,18 @@ of its command symbol."
>    "Function to call to refresh the current buffer.")
>  (make-variable-buffer-local 'notmuch-buffer-refresh-function)
>  
> -(defun notmuch-refresh-this-buffer ()
> -  "Refresh the current buffer."
> +(defun notmuch-refresh-this-buffer (&optional no-display)
> +  "Refresh the current buffer.
> +
> +If no-display is non-nil do not try to bring the buffer to the
> +foreground. If the buffer is already foregrounded i.e. displayed
> +in a window on screen, no-display has no effect."
>    (interactive)
>    (when notmuch-buffer-refresh-function
>      (if (commandp notmuch-buffer-refresh-function)
>  	;; Pass prefix argument, etc.
>  	(call-interactively notmuch-buffer-refresh-function)
> -      (funcall notmuch-buffer-refresh-function))))
> +      (funcall notmuch-buffer-refresh-function no-display))))
>  
>  (defun notmuch-poll-and-refresh-this-buffer ()
>    "Invoke `notmuch-poll' to import mail, then refresh the current buffer."
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 05687b7..ec7a242 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -924,7 +924,7 @@ PROMPT is the string to prompt with."
>  
>  ;;;###autoload
>  (put 'notmuch-search 'notmuch-doc "Search for messages.")
> -(defun notmuch-search (&optional query oldest-first target-thread target-line)
> +(defun notmuch-search (&optional query oldest-first target-thread target-line no-display)
>    "Display threads matching QUERY in a notmuch-search buffer.
>  
>  If QUERY is nil, it is read interactively from the minibuffer.
> @@ -935,6 +935,9 @@ Other optional parameters are used as follows:
>                   current if it appears in the search results.
>    TARGET-LINE: The line number to move to if the target thread does not
>                 appear in the search results.
> +  NO-DISPLAY: Do not try to foreground the search results buffer. If it is
> +              already foregrounded i.e. displayed in a window, this has no
> +              effect, meaning the buffer will remain visible.
>  
>  When called interactively, this will prompt for a query and use
>  the configured default sort order."
> @@ -948,7 +951,9 @@ the configured default sort order."
>  
>    (let* ((query (or query (notmuch-read-query "Notmuch search: ")))
>  	 (buffer (get-buffer-create (notmuch-search-buffer-title query))))
> -    (switch-to-buffer buffer)
> +    (if no-display
> +	(set-buffer buffer)
> +      (switch-to-buffer buffer))
>      (notmuch-search-mode)
>      ;; Don't track undo information for this buffer
>      (set 'buffer-undo-list t)
> @@ -981,21 +986,23 @@ the configured default sort order."
>  	  (set-process-query-on-exit-flag proc nil))))
>      (run-hooks 'notmuch-search-hook)))
>  
> -(defun notmuch-search-refresh-view ()
> +(defun notmuch-search-refresh-view (&optional no-display)
>    "Refresh the current view.
>  
>  Erases the current buffer and runs a new search with the same
>  query string as the current search. If the current thread is in
>  the new search results, then point will be placed on the same
>  thread. Otherwise, point will be moved to attempt to be in the
> -same relative position within the new buffer."
> +same relative position within the new buffer. If no-display is
> +non-nil, the search results buffer will not be foregrounded, if
> +it already is displayed in a window, then no-display has no effect."
>    (let ((target-line (line-number-at-pos))
>  	(oldest-first notmuch-search-oldest-first)
>  	(target-thread (notmuch-search-find-thread-id 'bare))
>  	(query notmuch-search-query-string)
>  	(inhibit-read-only t))
>      (erase-buffer)
> -    (notmuch-search query oldest-first target-thread target-line)
> +    (notmuch-search query oldest-first target-thread target-line no-display)
>      (goto-char (point-min))))
>  
>  (defun notmuch-search-toggle-order ()
> -- 
> 2.10.0

Re: [PATCH v2 3/4] emacs: add refresh buffer optional no-display arg

[Ioan-Adrian Ratiu]

On Sun, 25 Sep 2016, Mark Walters <markwalters1009@gmail.com> wrote:
> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
>> Add an optional no-display arg to the generic buffer refresh function,
>> notmuch-refresh-this-buffer, which works the same way as notmuch-hello
>> mode's notmuch-hello-update no-display arg.
>>
>> The idea is for the generic notmuch-refresh-this-buffer to pass down
>> this arg to the "mode-specific" refresh functions to be able to update
>> buffers without bringing them to the foreground (if they are already
>> foregrounded, i.e. displayed in a window, this has no effect).
>>
>> When updating a search buffer, notmuch currently always brings results
>> in a window to the foreground. Perhaps counter-intuitively, we do not
>> want this behaviour necessarily, because we want to auto-refresh any
>> kind of search buffers, even those backgrounded (not displayed in any
>> window/frame) from previous searches. This is why we add a no-display
>> arg to notmuch-search.
>>
>> We do this to show which mails have appeard or dissapeared since the
>> last search refresh and have this information updated in real time
>> even when switching buffers. The ultimate goal of this is to have all
>> notmuch buffers auto-refresh when the email client syncs (this function
>> is added in the next patch).
>>
>> Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
>> ---
>>  emacs/notmuch-lib.el | 10 +++++++---
>>  emacs/notmuch.el     | 17 ++++++++++++-----
>>  2 files changed, 19 insertions(+), 8 deletions(-)
>
> I feel I am missing something here: why do you not need to change
> notmuch-show-refresh-view and notmuch-tree-refresh-view as well? Note
> notmuch-show-refresh-view already has an optional argument.

Sorry I've missed this mail on my previous reading.

Only when refreshing notmuch-search (switch-to-buffer) is called which
brings a buffer up in a window, so adding the optional argument to
notmuch show or tree doesn't make sense because the buffer is not
forced to be visible like in notmuch-search's refresh view case.

Of course instead of adding the no-display arg to notmuch-search's
refresh function we could remove the switch-to-buffer call and make it
behave like notmuch-show or tree, but this changes the user-visible
behaviour and I guess people expect the notmuch-search buffer to be made
visible by default. Also notmuch-search creates new buffers based on the
serach queries so it doesn't make sense to reuse them and that's why
we're forcing them to be displayed by default, right?

What do you think about this? Do you with me to change the patch?

Ionel

>
> Best wishes
>
> Mark
>
>>
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index 2f015b0..6618365 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -409,14 +409,18 @@ of its command symbol."
>>    "Function to call to refresh the current buffer.")
>>  (make-variable-buffer-local 'notmuch-buffer-refresh-function)
>>  
>> -(defun notmuch-refresh-this-buffer ()
>> -  "Refresh the current buffer."
>> +(defun notmuch-refresh-this-buffer (&optional no-display)
>> +  "Refresh the current buffer.
>> +
>> +If no-display is non-nil do not try to bring the buffer to the
>> +foreground. If the buffer is already foregrounded i.e. displayed
>> +in a window on screen, no-display has no effect."
>>    (interactive)
>>    (when notmuch-buffer-refresh-function
>>      (if (commandp notmuch-buffer-refresh-function)
>>  	;; Pass prefix argument, etc.
>>  	(call-interactively notmuch-buffer-refresh-function)
>> -      (funcall notmuch-buffer-refresh-function))))
>> +      (funcall notmuch-buffer-refresh-function no-display))))
>>  
>>  (defun notmuch-poll-and-refresh-this-buffer ()
>>    "Invoke `notmuch-poll' to import mail, then refresh the current buffer."
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index 05687b7..ec7a242 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -924,7 +924,7 @@ PROMPT is the string to prompt with."
>>  
>>  ;;;###autoload
>>  (put 'notmuch-search 'notmuch-doc "Search for messages.")
>> -(defun notmuch-search (&optional query oldest-first target-thread target-line)
>> +(defun notmuch-search (&optional query oldest-first target-thread target-line no-display)
>>    "Display threads matching QUERY in a notmuch-search buffer.
>>  
>>  If QUERY is nil, it is read interactively from the minibuffer.
>> @@ -935,6 +935,9 @@ Other optional parameters are used as follows:
>>                   current if it appears in the search results.
>>    TARGET-LINE: The line number to move to if the target thread does not
>>                 appear in the search results.
>> +  NO-DISPLAY: Do not try to foreground the search results buffer. If it is
>> +              already foregrounded i.e. displayed in a window, this has no
>> +              effect, meaning the buffer will remain visible.
>>  
>>  When called interactively, this will prompt for a query and use
>>  the configured default sort order."
>> @@ -948,7 +951,9 @@ the configured default sort order."
>>  
>>    (let* ((query (or query (notmuch-read-query "Notmuch search: ")))
>>  	 (buffer (get-buffer-create (notmuch-search-buffer-title query))))
>> -    (switch-to-buffer buffer)
>> +    (if no-display
>> +	(set-buffer buffer)
>> +      (switch-to-buffer buffer))
>>      (notmuch-search-mode)
>>      ;; Don't track undo information for this buffer
>>      (set 'buffer-undo-list t)
>> @@ -981,21 +986,23 @@ the configured default sort order."
>>  	  (set-process-query-on-exit-flag proc nil))))
>>      (run-hooks 'notmuch-search-hook)))
>>  
>> -(defun notmuch-search-refresh-view ()
>> +(defun notmuch-search-refresh-view (&optional no-display)
>>    "Refresh the current view.
>>  
>>  Erases the current buffer and runs a new search with the same
>>  query string as the current search. If the current thread is in
>>  the new search results, then point will be placed on the same
>>  thread. Otherwise, point will be moved to attempt to be in the
>> -same relative position within the new buffer."
>> +same relative position within the new buffer. If no-display is
>> +non-nil, the search results buffer will not be foregrounded, if
>> +it already is displayed in a window, then no-display has no effect."
>>    (let ((target-line (line-number-at-pos))
>>  	(oldest-first notmuch-search-oldest-first)
>>  	(target-thread (notmuch-search-find-thread-id 'bare))
>>  	(query notmuch-search-query-string)
>>  	(inhibit-read-only t))
>>      (erase-buffer)
>> -    (notmuch-search query oldest-first target-thread target-line)
>> +    (notmuch-search query oldest-first target-thread target-line no-display)
>>      (goto-char (point-min))))
>>  
>>  (defun notmuch-search-toggle-order ()
>> -- 
>> 2.10.0

[PATCH v2 4/4] emacs: notmuch-lib: add refresh all buffers function

[Ioan-Adrian Ratiu]

notmuch-refresh-all-buffers calls each buffer's major mode specific
refresh function using the generic notmuch-refresh-this-buffer function.

It is very useful because by passing a non-nil arg to the buffer specific
refresh functions it refreshes all notmuch buffers in the background and
this again is very useful when doing periodic timer-based mail syncing.

Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
---
 emacs/notmuch-lib.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 6618365..72fee4d 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -428,6 +428,18 @@ in a window on screen, no-display has no effect."
   (notmuch-poll)
   (notmuch-refresh-this-buffer))
 
+(defun notmuch-refresh-all-buffers (&optional no-display)
+  "Invoke `notmuch-refresh-this-buffer' on all notmuch major-mode buffers.
+
+If no-display is non-nil all buffers are silently refreshed, i.e. they are
+not foregrounded even if not displayed in any window. If no-display is nil
+then each buffer's mode-specific refresh function uses its default behaviour."
+  (dolist (buffer (buffer-list))
+    (let ((buffer-mode (buffer-local-value 'major-mode buffer)))
+      (when (string-prefix-p "notmuch" (format "%s" buffer-mode))
+	(with-current-buffer buffer
+	  (notmuch-refresh-this-buffer no-display))))))
+
 (defun notmuch-prettify-subject (subject)
   ;; This function is used by `notmuch-search-process-filter' which
   ;; requires that we not disrupt its' matching state.
-- 
2.10.0

Re: [PATCH v2 4/4] emacs: notmuch-lib: add refresh all buffers function

[Mark Walters]

On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
> notmuch-refresh-all-buffers calls each buffer's major mode specific
> refresh function using the generic notmuch-refresh-this-buffer function.
>
> It is very useful because by passing a non-nil arg to the buffer specific
> refresh functions it refreshes all notmuch buffers in the background and
> this again is very useful when doing periodic timer-based mail syncing.
>
> Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
> ---
>  emacs/notmuch-lib.el | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 6618365..72fee4d 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -428,6 +428,18 @@ in a window on screen, no-display has no effect."
>    (notmuch-poll)
>    (notmuch-refresh-this-buffer))
>  
> +(defun notmuch-refresh-all-buffers (&optional no-display)
> +  "Invoke `notmuch-refresh-this-buffer' on all notmuch major-mode buffers.
> +
> +If no-display is non-nil all buffers are silently refreshed, i.e. they are
> +not foregrounded even if not displayed in any window. If no-display is nil
> +then each buffer's mode-specific refresh function uses its default behaviour."
> +  (dolist (buffer (buffer-list))
> +    (let ((buffer-mode (buffer-local-value 'major-mode buffer)))
> +      (when (string-prefix-p "notmuch" (format "%s" buffer-mode))

I think I would prefer an explicit list of possible matches e.g.,

  (when (memq buffer-mode '(notmuch-show-mode
                            notmuch-tree-mode
                            notmuch-hello-mode
                            notmuch-search-mode)

This makes it easier to see when it will be called, and makes sure we
don't do anything weird if we have notmuch-compose-mode for example.

Best wishes

Mark


> +	(with-current-buffer buffer
> +	  (notmuch-refresh-this-buffer no-display))))))
> +
>  (defun notmuch-prettify-subject (subject)
>    ;; This function is used by `notmuch-search-process-filter' which
>    ;; requires that we not disrupt its' matching state.
> -- 
> 2.10.0

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Ioan-Adrian Ratiu]

Argh, so right after I posted this I found a bug: for every new window
in which you open the same notmuch-show buffer it creates a new buffer.

For example if from notmuch-search you open a thread "hello" in multiple
windows, each window will show a different "hello<1>" "hello<2>" etc
buffer instead of showing a single "hello" buffer for all windows.

I'm aware of this issue and I'll fix it in v3, however please if you
have time & feedback for v2 I'd greatly appreciate it.

Best wishes,
Ionel

On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
> Changes since v1 (thank you Mark for your feedback):
>     * Major rewrite/simplification of the notmuch-show refresh patch to
>     support simultaneous refreshing multiple windows displaying a buffer
>     * Removed the notmuch-show-message-adjust () patch because it's not
>     needed after the above rewrite
>     * Rewrote notmuch-refresh-all-buffers () to use dolist instead of
>     while loop
>     * Minor commit message/metadata improvements
>
> This patch series adds a function to refresh all buffers, including an
> option to silently refresh them in the background i.e. to not show the
> newly refreshed buffer in any window.
>
> This is very useful for asynchronously updating all buffers when new
> mail arrives, using logic similar to the following (it's what I use):
>
> (setq process-connection-type nil)
>
> (defun done-index-sentinel (process event)
>   (notmuch-refresh-all-buffers t)
>   (message "Mail sync complete"))
>
> (defun done-sync-sentinel (process event)
>   (message "Indexing mail using notmuch")
>   (set-process-sentinel (start-process "notmuch" nil "notmuch" "new")
> 			'done-index-sentinel))
>
> (defun run-mail-sync ()
>   (message "Syncing mail in background")
>   (set-process-sentinel (start-process "mbsync" nil "mbsync" "gmail")
> 			  'done-sync-sentinel))
>
> (run-with-idle-timer 600 nil 'run-mail-sync)
>
> Ioan-Adrian Ratiu (4):
>   emacs: reuse buffer when refreshing searches
>   emacs: notmuch-show: refresh all windows showing a buffer
>   emacs: add refresh buffer optional no-display arg
>   emacs: notmuch-lib: add refresh all buffers function
>
>  emacs/notmuch-lib.el  | 22 +++++++++++++++++++---
>  emacs/notmuch-show.el | 19 +++++++++++++------
>  emacs/notmuch.el      | 24 ++++++++++++++++--------
>  3 files changed, 48 insertions(+), 17 deletions(-)
>
> -- 
> 2.10.0

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Ioan-Adrian Ratiu]

On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
> Argh, so right after I posted this I found a bug: for every new window
> in which you open the same notmuch-show buffer it creates a new buffer.
>
> For example if from notmuch-search you open a thread "hello" in multiple
> windows, each window will show a different "hello<1>" "hello<2>" etc
> buffer instead of showing a single "hello" buffer for all windows.

This is really weird. I'm experiencing this bug even without my patches
so it's not a fault in my code. I've tried with both emacs 25.1 and the
latest emacs git rev, does anyone else experience this behaviour?

Am I missing something, is this an expected behaviour and not a bug?

>
> I'm aware of this issue and I'll fix it in v3, however please if you
> have time & feedback for v2 I'd greatly appreciate it.
>
> Best wishes,
> Ionel
>
> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
>> Changes since v1 (thank you Mark for your feedback):
>>     * Major rewrite/simplification of the notmuch-show refresh patch to
>>     support simultaneous refreshing multiple windows displaying a buffer
>>     * Removed the notmuch-show-message-adjust () patch because it's not
>>     needed after the above rewrite
>>     * Rewrote notmuch-refresh-all-buffers () to use dolist instead of
>>     while loop
>>     * Minor commit message/metadata improvements
>>
>> This patch series adds a function to refresh all buffers, including an
>> option to silently refresh them in the background i.e. to not show the
>> newly refreshed buffer in any window.
>>
>> This is very useful for asynchronously updating all buffers when new
>> mail arrives, using logic similar to the following (it's what I use):
>>
>> (setq process-connection-type nil)
>>
>> (defun done-index-sentinel (process event)
>>   (notmuch-refresh-all-buffers t)
>>   (message "Mail sync complete"))
>>
>> (defun done-sync-sentinel (process event)
>>   (message "Indexing mail using notmuch")
>>   (set-process-sentinel (start-process "notmuch" nil "notmuch" "new")
>> 			'done-index-sentinel))
>>
>> (defun run-mail-sync ()
>>   (message "Syncing mail in background")
>>   (set-process-sentinel (start-process "mbsync" nil "mbsync" "gmail")
>> 			  'done-sync-sentinel))
>>
>> (run-with-idle-timer 600 nil 'run-mail-sync)
>>
>> Ioan-Adrian Ratiu (4):
>>   emacs: reuse buffer when refreshing searches
>>   emacs: notmuch-show: refresh all windows showing a buffer
>>   emacs: add refresh buffer optional no-display arg
>>   emacs: notmuch-lib: add refresh all buffers function
>>
>>  emacs/notmuch-lib.el  | 22 +++++++++++++++++++---
>>  emacs/notmuch-show.el | 19 +++++++++++++------
>>  emacs/notmuch.el      | 24 ++++++++++++++++--------
>>  3 files changed, 48 insertions(+), 17 deletions(-)
>>
>> -- 
>> 2.10.0

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[David Bremner]

Ioan-Adrian Ratiu <adi@adirat.com> writes:

> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
>> Argh, so right after I posted this I found a bug: for every new window
>> in which you open the same notmuch-show buffer it creates a new buffer.
>>
>> For example if from notmuch-search you open a thread "hello" in multiple
>> windows, each window will show a different "hello<1>" "hello<2>" etc
>> buffer instead of showing a single "hello" buffer for all windows.
>
> This is really weird. I'm experiencing this bug even without my patches
> so it's not a fault in my code. I've tried with both emacs 25.1 and the
> latest emacs git rev, does anyone else experience this behaviour?
>
> Am I missing something, is this an expected behaviour and not a bug?

I don't (yet) have an opinion on whether this is a bug, but I can
confirm the behaviour exists, e.g. using devel/try-emacs-mua -Q in emacs 24.5.1

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Ioan-Adrian Ratiu]

On Sat, 24 Sep 2016, David Bremner <david@tethera.net> wrote:
> Ioan-Adrian Ratiu <adi@adirat.com> writes:
>
>> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
>>> Argh, so right after I posted this I found a bug: for every new window
>>> in which you open the same notmuch-show buffer it creates a new buffer.
>>>
>>> For example if from notmuch-search you open a thread "hello" in multiple
>>> windows, each window will show a different "hello<1>" "hello<2>" etc
>>> buffer instead of showing a single "hello" buffer for all windows.
>>
>> This is really weird. I'm experiencing this bug even without my patches
>> so it's not a fault in my code. I've tried with both emacs 25.1 and the
>> latest emacs git rev, does anyone else experience this behaviour?
>>
>> Am I missing something, is this an expected behaviour and not a bug?
>
> I don't (yet) have an opinion on whether this is a bug, but I can
> confirm the behaviour exists, e.g. using devel/try-emacs-mua -Q in emacs 24.5.1

It's caused by the generate-new-buffer-name call in notmuch-show(), it's
been there since cca 2010 by 9bee20aed (notmuch.el: Make notmuch-show
buffer name first subject...)

I don't quite understand why generate-new-buffer-name is called at all
there. What's wrong with the existing buffer names and why do we want
to create others? :)

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Mark Walters]

On Sun, 25 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
> On Sat, 24 Sep 2016, David Bremner <david@tethera.net> wrote:
>> Ioan-Adrian Ratiu <adi@adirat.com> writes:
>>
>>> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
>>>> Argh, so right after I posted this I found a bug: for every new window
>>>> in which you open the same notmuch-show buffer it creates a new buffer.
>>>>
>>>> For example if from notmuch-search you open a thread "hello" in multiple
>>>> windows, each window will show a different "hello<1>" "hello<2>" etc
>>>> buffer instead of showing a single "hello" buffer for all windows.
>>>
>>> This is really weird. I'm experiencing this bug even without my patches
>>> so it's not a fault in my code. I've tried with both emacs 25.1 and the
>>> latest emacs git rev, does anyone else experience this behaviour?
>>>
>>> Am I missing something, is this an expected behaviour and not a bug?
>>
>> I don't (yet) have an opinion on whether this is a bug, but I can
>> confirm the behaviour exists, e.g. using devel/try-emacs-mua -Q in emacs 24.5.1
>
> It's caused by the generate-new-buffer-name call in notmuch-show(), it's
> been there since cca 2010 by 9bee20aed (notmuch.el: Make notmuch-show
> buffer name first subject...)
>
> I don't quite understand why generate-new-buffer-name is called at all
> there. What's wrong with the existing buffer names and why do we want
> to create others? :)

Hi

I think this behaviour is expected in the sense that this is what the
code has always done. It could be that the current behaviour made more
sense before dme's commit 30f1c43e which made q only kill the current
buffer if it was the only copy.

If I understand this code correctly the buffer gets the name the subject
of the thread. Obviously we can't reuse (i.e. overwrite/kill) a buffer
just because it has the same subject as the new thread so some
care would be needed.

Plausibly we could reuse the existing buffer it was for the same
thread. However, there are some things to consider before doing this
since a notmuch-show buffer does have a fair amount of internal state,
some of which could be lost if it is refreshed, and some of which could
linger into the new buffer and be confusing.

For example:

1) tags deleted or added since the last refresh are shown in the show
buffer (red strikethough, green underline by default) and these visual
cues would be lost (the tag changes have already happened so that is
fine)

2) Which messages are collapsed or expanded. Notmuch normally attempts
to keep this the same when the buffer is refreshed (see
notmuch-show-capture-state and notmuch-show-apply-state). Keeping the
old state would be confusing as some matching messages might not be
expanded, but removing the old state could lose information.

I am definitely not saying that we don't want to change to just allowing
one show buffer for a particular search -- indeed the above two are
probably pretty trivial -- but I think they, and other similar
things, should be thought about first.

Best wishes

Mark

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Tomi Ollila]

On Sun, Sep 25 2016, Mark Walters <markwalters1009@gmail.com> wrote:

> On Sun, 25 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
>> On Sat, 24 Sep 2016, David Bremner <david@tethera.net> wrote:
>>> Ioan-Adrian Ratiu <adi@adirat.com> writes:
>>>
>>>> On Sat, 24 Sep 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
>>>>> Argh, so right after I posted this I found a bug: for every new window
>>>>> in which you open the same notmuch-show buffer it creates a new buffer.
>>>>>
>>>>> For example if from notmuch-search you open a thread "hello" in multiple
>>>>> windows, each window will show a different "hello<1>" "hello<2>" etc
>>>>> buffer instead of showing a single "hello" buffer for all windows.
>>>>
>>>> This is really weird. I'm experiencing this bug even without my patches
>>>> so it's not a fault in my code. I've tried with both emacs 25.1 and the
>>>> latest emacs git rev, does anyone else experience this behaviour?
>>>>
>>>> Am I missing something, is this an expected behaviour and not a bug?
>>>
>>> I don't (yet) have an opinion on whether this is a bug, but I can
>>> confirm the behaviour exists, e.g. using devel/try-emacs-mua -Q in emacs 24.5.1
>>
>> It's caused by the generate-new-buffer-name call in notmuch-show(), it's
>> been there since cca 2010 by 9bee20aed (notmuch.el: Make notmuch-show
>> buffer name first subject...)
>>
>> I don't quite understand why generate-new-buffer-name is called at all
>> there. What's wrong with the existing buffer names and why do we want
>> to create others? :)
>
> Hi
>
> I think this behaviour is expected in the sense that this is what the
> code has always done. It could be that the current behaviour made more
> sense before dme's commit 30f1c43e which made q only kill the current
> buffer if it was the only copy.
>
> If I understand this code correctly the buffer gets the name the subject
> of the thread. Obviously we can't reuse (i.e. overwrite/kill) a buffer
> just because it has the same subject as the new thread so some
> care would be needed.

I have 2 quick (drive-by-;) comments...

1) could we include thread in in the generated subject and how much would
that helo

2) then, minor commit message related comment: if there is going to be v3,
in id:20160924200735.25425-2-adi@adirat.com adi mentioned 'next patches'
-- those are not patches (anymore) when commits are made, so it would be
better to reword that sentence. If anythine else doesn't come up, simplest
thing is to change the word to 'commits'. As said, this is minor thing,
and we have worse things in commit messages; if there is no need to send
v3, or the message change is forgotten then it may go in as it is now...


Tomi


>
> Plausibly we could reuse the existing buffer it was for the same
> thread. However, there are some things to consider before doing this
> since a notmuch-show buffer does have a fair amount of internal state,
> some of which could be lost if it is refreshed, and some of which could
> linger into the new buffer and be confusing.
>
> For example:
>
> 1) tags deleted or added since the last refresh are shown in the show
> buffer (red strikethough, green underline by default) and these visual
> cues would be lost (the tag changes have already happened so that is
> fine)
>
> 2) Which messages are collapsed or expanded. Notmuch normally attempts
> to keep this the same when the buffer is refreshed (see
> notmuch-show-capture-state and notmuch-show-apply-state). Keeping the
> old state would be confusing as some matching messages might not be
> expanded, but removing the old state could lose information.
>
> I am definitely not saying that we don't want to change to just allowing
> one show buffer for a particular search -- indeed the above two are
> probably pretty trivial -- but I think they, and other similar
> things, should be thought about first.
>
> Best wishes
>
> Mark
>
>
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

<silly vocab nitpicking>

On Sun 2016-09-25 03:32:08 -0400, Tomi Ollila wrote:
> 2) then, minor commit message related comment: if there is going to be v3,
> in id:20160924200735.25425-2-adi@adirat.com adi mentioned 'next patches'
> -- those are not patches (anymore) when commits are made, so it would be
> better to reword that sentence. If anythine else doesn't come up, simplest
> thing is to change the word to 'commits'. As said, this is minor thing,
> and we have worse things in commit messages; if there is no need to send
> v3, or the message change is forgotten then it may go in as it is now...

Before these things are accepted into whatever you consider the
canonical git repo to be, while they're still patches floating around in
our various mailboxes, they aren't really "commits" either.  I'd use
"changesets" as the generic term.

</silly vocab nitpicking>

        --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 930 bytes --]

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Tomi Ollila]

On Thu, Oct 06 2016, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> <silly vocab nitpicking>
>
> On Sun 2016-09-25 03:32:08 -0400, Tomi Ollila wrote:
>> 2) then, minor commit message related comment: if there is going to be v3,
>> in id:20160924200735.25425-2-adi@adirat.com adi mentioned 'next patches'
>> -- those are not patches (anymore) when commits are made, so it would be
>> better to reword that sentence. If anythine else doesn't come up, simplest
>> thing is to change the word to 'commits'. As said, this is minor thing,
>> and we have worse things in commit messages; if there is no need to send
>> v3, or the message change is forgotten then it may go in as it is now...
>
> Before these things are accepted into whatever you consider the
> canonical git repo to be, while they're still patches floating around in
> our various mailboxes, they aren't really "commits" either.  I'd use
> "changesets" as the generic term.

That's even betten reason to reword the message so none of the words are
used >;)


>
> </silly vocab nitpicking>
>
>         --dkg
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 0/4] Add refresh all buffers functionality

[Ioan-Adrian Ratiu]

On Thu, 06 Oct 2016, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> <silly vocab nitpicking>
>
> On Sun 2016-09-25 03:32:08 -0400, Tomi Ollila wrote:
>> 2) then, minor commit message related comment: if there is going to be v3,
>> in id:20160924200735.25425-2-adi@adirat.com adi mentioned 'next patches'
>> -- those are not patches (anymore) when commits are made, so it would be
>> better to reword that sentence. If anythine else doesn't come up, simplest
>> thing is to change the word to 'commits'. As said, this is minor thing,
>> and we have worse things in commit messages; if there is no need to send
>> v3, or the message change is forgotten then it may go in as it is now...
>
> Before these things are accepted into whatever you consider the
> canonical git repo to be, while they're still patches floating around in
> our various mailboxes, they aren't really "commits" either.  I'd use
> "changesets" as the generic term.
>
> </silly vocab nitpicking>

I have v3 ready to send (will send it later today when I get home).
I've reworded to use the word "commits".

IMO this is just bikeshedding. It doesn't matter how we call these,
either patches, commits, changesets are ok for me, just pleaso don't
make me reword these too many times.

>
>         --dkg