[PATCH v5 0/7] Add refresh all buffers functionality

[PATCH v5 0/7] Add refresh all buffers functionality

[Mark Walters]

This is a simplified version of the series at
id:20161008210139.25322-1-adi@adirat.com

The main change is that it makes all the refresh functions refresh the
buffer without forcing the buffer to be displayed. 

In tree and show mode this was already the case; in search mode it is
a change but since the refresh function has to be called in the buffer
it is likely to already be displayed. In hello mode it a genuine
change, but notmuch-hello-update is a trivial wrapped of notmuch-hello
so anyone who wants to force display can just call notmuch-hello.

Once this change is made Ionel's changes become very clean.

Best wishes

Mark


Ioan-Adrian Ratiu (4):
  emacs: notmuch-search: add no-display functionality
  emacs: notmuch-search-refresh-view: reuse buffer
  emacs: notmuch-show: refresh all windows displaying buffer
  emacs: notmuch-lib: add refresh all buffers function

Mark Walters (3):
  emacs: tree: make refresh use generic binding
  emacs: make the refresh functions more consistent
  emacs: hello: stop update from forcing the buffer to be displayed

 emacs/notmuch-hello.el |  7 ++++---
 emacs/notmuch-lib.el   | 23 ++++++++++++++++++-----
 emacs/notmuch-show.el  | 19 +++++++++++++------
 emacs/notmuch-tree.el  |  1 -
 emacs/notmuch.el       | 16 +++++++++++-----
 5 files changed, 46 insertions(+), 20 deletions(-)

-- 
2.1.4

[PATCH v5 1/7] emacs: tree: make refresh use generic binding

[Mark Walters]

Not using the generic binding is an anomaly from when tree was in
contrib (as pick).
---
 emacs/notmuch-tree.el | 1 -
 1 file changed, 1 deletion(-)

diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index 1555812..d5587a9 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -271,7 +271,6 @@ FUNC."
     (define-key map "x" 'notmuch-tree-quit)
     (define-key map "A" 'notmuch-tree-archive-thread)
     (define-key map "a" 'notmuch-tree-archive-message-then-next)
-    (define-key map "=" 'notmuch-tree-refresh-view)
     (define-key map "z" 'notmuch-tree-to-tree)
     (define-key map "n" 'notmuch-tree-next-matching-message)
     (define-key map "p" 'notmuch-tree-prev-matching-message)
-- 
2.1.4

[PATCH v5 2/7] emacs: make the refresh functions more consistent

[Mark Walters]

The different refreshed functions were called differently: some were
called interactively and some were not. Make them all interactive.
---
 emacs/notmuch-hello.el | 1 +
 emacs/notmuch-lib.el   | 9 ++++-----
 emacs/notmuch.el       | 1 +
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index d582bff..089a19d 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -607,6 +607,7 @@ with `notmuch-hello-query-counts'."
 (defun notmuch-hello-update (&optional no-display)
   "Update the current notmuch view."
   ;; Lazy - rebuild everything.
+  (interactive)
   (notmuch-hello no-display))
 
 (defun notmuch-hello-window-configuration-change ()
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index b2cdace..8b55ca7 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -416,11 +416,10 @@ of its command symbol."
 (defun notmuch-refresh-this-buffer ()
   "Refresh the current buffer."
   (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))))
+  (when (and notmuch-buffer-refresh-function
+	     (commandp notmuch-buffer-refresh-function))
+    ;; Pass prefix argument, etc.
+    (call-interactively notmuch-buffer-refresh-function)))
 
 (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 6c36ad8..673811c 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -991,6 +991,7 @@ 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."
+  (interactive)
   (let ((target-line (line-number-at-pos))
 	(oldest-first notmuch-search-oldest-first)
 	(target-thread (notmuch-search-find-thread-id 'bare))
-- 
2.1.4

[PATCH v5 3/7] emacs: hello: stop update from forcing the buffer to be displayed

[Mark Walters]

Make the notmuch-hello refresh function (notmuch-hello-update) not
force the buffer to be displayed. All the callers call it when the
buffer is already displayed so it will only affect non-interactive
callers. Since it is just a trivial wrapper of notmuch-hello anyone
who wants to force the buffer to be displayed should just call
notmuch-hello.
---
 emacs/notmuch-hello.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 089a19d..c858a20 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -604,11 +604,11 @@ with `notmuch-hello-query-counts'."
 
 (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))
 
-(defun notmuch-hello-update (&optional no-display)
-  "Update the current notmuch view."
+(defun notmuch-hello-update ()
+  "Update the notmuch-hello buffer."
   ;; Lazy - rebuild everything.
   (interactive)
-  (notmuch-hello no-display))
+  (notmuch-hello t))
 
 (defun notmuch-hello-window-configuration-change ()
   "Hook function to update the hello buffer when it is switched to."
-- 
2.1.4

[PATCH v5 4/7] emacs: notmuch-search: add no-display functionality

[Mark Walters]

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

If no-display is non-nil when calling notmuch-search then do not force
the search buffer to be displayed.

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

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 673811c..8f0053c 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -926,7 +926,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.
@@ -937,6 +937,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."
@@ -950,7 +953,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)
-- 
2.1.4

[PATCH v5 5/7] emacs: notmuch-search-refresh-view: reuse buffer

[Mark Walters]

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

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 commits 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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 8f0053c..079a3d1 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -991,7 +991,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
@@ -1001,8 +1001,8 @@ same relative position within the new buffer."
 	(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)
-    (notmuch-search query oldest-first target-thread target-line)
+    ;; notmuch-search erases the current buffer.
+    (notmuch-search query oldest-first target-thread target-line t)
     (goto-char (point-min))))
 
 (defun notmuch-search-toggle-order ()
-- 
2.1.4

[PATCH v5 6/7] emacs: notmuch-show: refresh all windows displaying buffer

[Mark Walters]

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

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 message based on the (point) location. We store the state
of all displayed windows when refreshing a notmuch-show buffer and
re-apply the current shown message (point) for all windows.

Implementation note: Each window has it's own (point) value, besides
the buffer's (point) value. Sometimes these values are identical like
in the case where a single window displays a buffer. When multiple
windows display a buffer, (point) returns each window's specific value.
What we are storing in this changeset is the window values not the
buffer point values. The buffer's point is returned only if no window
is displaying the buffer, a case we do not care about here.

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 f2487ab..ac7eb77 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) nil 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.1.4

[PATCH v5 7/7] emacs: notmuch-lib: add refresh all buffers function

[Mark Walters]

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

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

Since the earlier changesets have stopped the refresh functions from
forcing the buffers to be redisplayed this can refresh buffers that
are not currently displayed without disturbing the user.  This is very
useful for silent async background updating the emacs display when new
mail is fetched.

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

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 8b55ca7..85c2237 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -427,6 +427,20 @@ of its command symbol."
   (notmuch-poll)
   (notmuch-refresh-this-buffer))
 
+(defun notmuch-refresh-all-buffers ()
+  "Invoke `notmuch-refresh-this-buffer' on all notmuch major-mode buffers.
+
+The buffers are silently refreshed, i.e. they are not forced to
+be displayed."
+  (dolist (buffer (buffer-list))
+    (let ((buffer-mode (buffer-local-value 'major-mode buffer)))
+      (when (memq buffer-mode '(notmuch-show-mode
+				notmuch-tree-mode
+				notmuch-search-mode
+				notmuch-hello-mode))
+	(with-current-buffer buffer
+	  (notmuch-refresh-this-buffer))))))
+
 (defun notmuch-prettify-subject (subject)
   ;; This function is used by `notmuch-search-process-filter' which
   ;; requires that we not disrupt its' matching state.
-- 
2.1.4

Re: [PATCH v5 2/7] emacs: make the refresh functions more consistent

[Tomi Ollila]

On Sun, Oct 09 2016, Mark Walters <markwalters1009@gmail.com> wrote:

> The different refreshed functions were called differently: some were
> called interactively and some were not. Make them all interactive.
> ---
>  emacs/notmuch-hello.el | 1 +
>  emacs/notmuch-lib.el   | 9 ++++-----
>  emacs/notmuch.el       | 1 +
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index d582bff..089a19d 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -607,6 +607,7 @@ with `notmuch-hello-query-counts'."
>  (defun notmuch-hello-update (&optional no-display)
>    "Update the current notmuch view."
>    ;; Lazy - rebuild everything.
> +  (interactive)
>    (notmuch-hello no-display))
>  
>  (defun notmuch-hello-window-configuration-change ()
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index b2cdace..8b55ca7 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -416,11 +416,10 @@ of its command symbol."
>  (defun notmuch-refresh-this-buffer ()
>    "Refresh the current buffer."
>    (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))))
> +  (when (and notmuch-buffer-refresh-function
> +	     (commandp notmuch-buffer-refresh-function))
> +    ;; Pass prefix argument, etc.
> +    (call-interactively notmuch-buffer-refresh-function)))

If there is going to be more rounds, IMO this (currently wrong, missing
second argument t) commandp check should be dropped. It is better to
signal programmer error than silently ignore non-nil variable that is
not referring to interactive function (the code is also simpler this way).

and a quesstion: the last patch in this series defines this
refresh all buffers function -- why is it not interactive ?

Otherwise series looks good to me.


Tomi


>  (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 6c36ad8..673811c 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -991,6 +991,7 @@ 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."
> +  (interactive)
>    (let ((target-line (line-number-at-pos))
>  	(oldest-first notmuch-search-oldest-first)
>  	(target-thread (notmuch-search-find-thread-id 'bare))
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v5 2/7] emacs: make the refresh functions more consistent

[Mark Walters]

On Sun, 09 Oct 2016, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, Oct 09 2016, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> The different refreshed functions were called differently: some were
>> called interactively and some were not. Make them all interactive.
>> ---
>>  emacs/notmuch-hello.el | 1 +
>>  emacs/notmuch-lib.el   | 9 ++++-----
>>  emacs/notmuch.el       | 1 +
>>  3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
>> index d582bff..089a19d 100644
>> --- a/emacs/notmuch-hello.el
>> +++ b/emacs/notmuch-hello.el
>> @@ -607,6 +607,7 @@ with `notmuch-hello-query-counts'."
>>  (defun notmuch-hello-update (&optional no-display)
>>    "Update the current notmuch view."
>>    ;; Lazy - rebuild everything.
>> +  (interactive)
>>    (notmuch-hello no-display))
>>  
>>  (defun notmuch-hello-window-configuration-change ()
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index b2cdace..8b55ca7 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -416,11 +416,10 @@ of its command symbol."
>>  (defun notmuch-refresh-this-buffer ()
>>    "Refresh the current buffer."
>>    (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))))
>> +  (when (and notmuch-buffer-refresh-function
>> +	     (commandp notmuch-buffer-refresh-function))
>> +    ;; Pass prefix argument, etc.
>> +    (call-interactively notmuch-buffer-refresh-function)))

Hi

Thanks for the review.

> If there is going to be more rounds, IMO this (currently wrong, missing
> second argument t) commandp check should be dropped. It is better to
> signal programmer error than silently ignore non-nil variable that is
> not referring to interactive function (the code is also simpler this way).

Yes I agree. Do you have any preference between just dropping the test,
and putting our own error instead?

> and a quesstion: the last patch in this series defines this
> refresh all buffers function -- why is it not interactive ?

Because I forgot to amend the last commit :-) The interactive is in my
tree, and was in what I tested  -- I agree it should be.

Best wishes

Mark


> Otherwise series looks good to me.


>
>
> Tomi
>
>
>>  (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 6c36ad8..673811c 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -991,6 +991,7 @@ 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."
>> +  (interactive)
>>    (let ((target-line (line-number-at-pos))
>>  	(oldest-first notmuch-search-oldest-first)
>>  	(target-thread (notmuch-search-find-thread-id 'bare))
>> -- 
>> 2.1.4
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v5 0/7] Add refresh all buffers functionality

[Ioan-Adrian Ratiu]

On Sun, 09 Oct 2016, Mark Walters <markwalters1009@gmail.com> wrote:
> This is a simplified version of the series at
> id:20161008210139.25322-1-adi@adirat.com
>
> The main change is that it makes all the refresh functions refresh the
> buffer without forcing the buffer to be displayed. 
>
> In tree and show mode this was already the case; in search mode it is
> a change but since the refresh function has to be called in the buffer
> it is likely to already be displayed. In hello mode it a genuine
> change, but notmuch-hello-update is a trivial wrapped of notmuch-hello
> so anyone who wants to force display can just call notmuch-hello.
>
> Once this change is made Ionel's changes become very clean.

LGTM. I had a few minutes to apply and test this series and all my
use-cases work perfectly. Thank you for helping with this.

>
> Best wishes
>
> Mark
>
>
> Ioan-Adrian Ratiu (4):
>   emacs: notmuch-search: add no-display functionality
>   emacs: notmuch-search-refresh-view: reuse buffer
>   emacs: notmuch-show: refresh all windows displaying buffer
>   emacs: notmuch-lib: add refresh all buffers function
>
> Mark Walters (3):
>   emacs: tree: make refresh use generic binding
>   emacs: make the refresh functions more consistent
>   emacs: hello: stop update from forcing the buffer to be displayed
>
>  emacs/notmuch-hello.el |  7 ++++---
>  emacs/notmuch-lib.el   | 23 ++++++++++++++++++-----
>  emacs/notmuch-show.el  | 19 +++++++++++++------
>  emacs/notmuch-tree.el  |  1 -
>  emacs/notmuch.el       | 16 +++++++++++-----
>  5 files changed, 46 insertions(+), 20 deletions(-)
>
> -- 
> 2.1.4

Re: [PATCH v5 2/7] emacs: make the refresh functions more consistent

[Tomi Ollila]

On Mon, Oct 10 2016, Mark Walters <markwalters1009@gmail.com> wrote:

> On Sun, 09 Oct 2016, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> On Sun, Oct 09 2016, Mark Walters <markwalters1009@gmail.com> wrote:
>>
>>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>>> index b2cdace..8b55ca7 100644
>>> --- a/emacs/notmuch-lib.el
>>> +++ b/emacs/notmuch-lib.el
>>> @@ -416,11 +416,10 @@ of its command symbol."
>>>  (defun notmuch-refresh-this-buffer ()
>>>    "Refresh the current buffer."
>>>    (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))))
>>> +  (when (and notmuch-buffer-refresh-function
>>> +	     (commandp notmuch-buffer-refresh-function))
>>> +    ;; Pass prefix argument, etc.
>>> +    (call-interactively notmuch-buffer-refresh-function)))
>
> Hi
>
> Thanks for the review.
>
>> If there is going to be more rounds, IMO this (currently wrong, missing
>> second argument t) commandp check should be dropped. It is better to
>> signal programmer error than silently ignore non-nil variable that is
>> not referring to interactive function (the code is also simpler this way).
>
> Yes I agree. Do you have any preference between just dropping the test,
> and putting our own error instead?

I'd just drop the test. e.g. 

(if/when notmuch-buffer-refresh-function
   (call-interactively notmuch-buffer-refresh-function))

Tomi

>
>> and a quesstion: the last patch in this series defines this
>> refresh all buffers function -- why is it not interactive ?
>
> Because I forgot to amend the last commit :-) The interactive is in my
> tree, and was in what I tested  -- I agree it should be.
>
> Best wishes
>
> Mark
>
>
>> Otherwise series looks good to me.
>
>
>>
>> Tomi