unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v3 0/4] Add refresh all buffers functionality
@ 2016-10-06 13:42 Ioan-Adrian Ratiu
  2016-10-06 13:42 ` [PATCH v3 1/4] emacs: reuse buffer when refreshing searches Ioan-Adrian Ratiu
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-10-06 13:42 UTC (permalink / raw)
  To: notmuch

Changes since v2 (based on Mark & Tomi's feedback)
 * fixed a missing parameter in the get-buffer-window-list call in notmuch-show-capture-state
 * added a (remove-overlays) call before (erase-buffer) in notmuch-search-refresh-view
 * replaced the (string-prefix-p "notmuch") in notmuch-refresh-all-buffers with explicit list
 * reworded the commit messages to replace the word patches with commits

What I intentionally did not touch in v3 is the notmuch-show call to
generate-new-buffer-name which creates now buffers when opening threads
from notmuch-search; that fix should go in another patch series.

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  | 25 ++++++++++++++++++++++---
 emacs/notmuch-show.el | 19 +++++++++++++------
 emacs/notmuch.el      | 25 +++++++++++++++++--------
 3 files changed, 52 insertions(+), 17 deletions(-)

-- 
2.10.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/4] emacs: reuse buffer when refreshing searches
  2016-10-06 13:42 [PATCH v3 0/4] Add refresh all buffers functionality Ioan-Adrian Ratiu
@ 2016-10-06 13:42 ` Ioan-Adrian Ratiu
  2016-10-06 13:42 ` [PATCH v3 2/4] emacs: notmuch-show: refresh all windows showing a buffer Ioan-Adrian Ratiu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-10-06 13:42 UTC (permalink / raw)
  To: notmuch

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

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 888672b..586c84e 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -985,7 +985,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
@@ -993,8 +993,10 @@ 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))
+    (remove-overlays)
+    (erase-buffer)
     (notmuch-search query oldest-first target-thread target-line)
     (goto-char (point-min))))
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/4] emacs: notmuch-show: refresh all windows showing a buffer
  2016-10-06 13:42 [PATCH v3 0/4] Add refresh all buffers functionality Ioan-Adrian Ratiu
  2016-10-06 13:42 ` [PATCH v3 1/4] emacs: reuse buffer when refreshing searches Ioan-Adrian Ratiu
@ 2016-10-06 13:42 ` Ioan-Adrian Ratiu
  2016-10-06 13:42 ` [PATCH v3 3/4] emacs: add refresh buffer optional no-display arg Ioan-Adrian Ratiu
  2016-10-06 13:42 ` [PATCH v3 4/4] emacs: notmuch-lib: add refresh all buffers function Ioan-Adrian Ratiu
  3 siblings, 0 replies; 9+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-10-06 13:42 UTC (permalink / raw)
  To: notmuch

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 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.10.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/4] emacs: add refresh buffer optional no-display arg
  2016-10-06 13:42 [PATCH v3 0/4] Add refresh all buffers functionality Ioan-Adrian Ratiu
  2016-10-06 13:42 ` [PATCH v3 1/4] emacs: reuse buffer when refreshing searches Ioan-Adrian Ratiu
  2016-10-06 13:42 ` [PATCH v3 2/4] emacs: notmuch-show: refresh all windows showing a buffer Ioan-Adrian Ratiu
@ 2016-10-06 13:42 ` Ioan-Adrian Ratiu
  2016-10-06 17:24   ` Mark Walters
  2016-10-06 13:42 ` [PATCH v3 4/4] emacs: notmuch-lib: add refresh all buffers function Ioan-Adrian Ratiu
  3 siblings, 1 reply; 9+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-10-06 13:42 UTC (permalink / raw)
  To: notmuch

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 commit).

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 b2cdace..af6a8f4 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -413,14 +413,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 586c84e..f3912d4 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -925,7 +925,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.
@@ -936,6 +936,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."
@@ -949,7 +952,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)
@@ -982,14 +987,16 @@ 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))
@@ -997,7 +1004,7 @@ same relative position within the new buffer."
 	(inhibit-read-only t))
     (remove-overlays)
     (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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 4/4] emacs: notmuch-lib: add refresh all buffers function
  2016-10-06 13:42 [PATCH v3 0/4] Add refresh all buffers functionality Ioan-Adrian Ratiu
                   ` (2 preceding siblings ...)
  2016-10-06 13:42 ` [PATCH v3 3/4] emacs: add refresh buffer optional no-display arg Ioan-Adrian Ratiu
@ 2016-10-06 13:42 ` Ioan-Adrian Ratiu
  3 siblings, 0 replies; 9+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-10-06 13:42 UTC (permalink / raw)
  To: notmuch

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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index af6a8f4..01733a2 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -432,6 +432,21 @@ 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 (memq buffer-mode '(notmuch-show-mode
+				notmuch-tree-mode
+				notmuch-search-mode
+				notmuch-hello-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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/4] emacs: add refresh buffer optional no-display arg
  2016-10-06 13:42 ` [PATCH v3 3/4] emacs: add refresh buffer optional no-display arg Ioan-Adrian Ratiu
@ 2016-10-06 17:24   ` Mark Walters
  2016-10-06 20:59     ` Ioan-Adrian Ratiu
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2016-10-06 17:24 UTC (permalink / raw)
  To: Ioan-Adrian Ratiu, notmuch

On Thu, 06 Oct 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 commit).
>
> 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 b2cdace..af6a8f4 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -413,14 +413,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))))

Hi

I think this is very fragile -- it relies on the fact that the refresh
functions in show and tree mode are interactive, so get called but the
call-interactive line (which doesn't have the no-display argument)
whereas the refresh functions in hello and search mode are not
interactive so get called by the funcall line and so do get the
no-display argument.

[In fact the notmuch-tree seems to bind "=" to notmuch-tree-refresh-view
whereas it could use the generic framework, which would mean it would
plausibly lose the interactive.]

However, I am not sure what the correct solution is.

Best wishes

Mark



>  
>  (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 586c84e..f3912d4 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -925,7 +925,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.
> @@ -936,6 +936,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."
> @@ -949,7 +952,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)
> @@ -982,14 +987,16 @@ 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))
> @@ -997,7 +1004,7 @@ same relative position within the new buffer."
>  	(inhibit-read-only t))
>      (remove-overlays)
>      (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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/4] emacs: add refresh buffer optional no-display arg
  2016-10-06 17:24   ` Mark Walters
@ 2016-10-06 20:59     ` Ioan-Adrian Ratiu
  2016-10-06 21:42       ` [PATCH] emacs: make the refresh code more consistent Mark Walters
  0 siblings, 1 reply; 9+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-10-06 20:59 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Thu, 06 Oct 2016, Mark Walters <markwalters1009@gmail.com> wrote:
> On Thu, 06 Oct 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 commit).
>>
>> 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 b2cdace..af6a8f4 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -413,14 +413,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))))
>
> Hi
>
> I think this is very fragile -- it relies on the fact that the refresh
> functions in show and tree mode are interactive, so get called but the
> call-interactive line (which doesn't have the no-display argument)
> whereas the refresh functions in hello and search mode are not
> interactive so get called by the funcall line and so do get the
> no-display argument.
>
> [In fact the notmuch-tree seems to bind "=" to notmuch-tree-refresh-view
> whereas it could use the generic framework, which would mean it would
> plausibly lose the interactive.]
>
> However, I am not sure what the correct solution is.

I agree it's messy, but I don't know if adding the no-display arg to
notmuch-show and notmuch-tree makes any sense because they don't force
the buffer to become visible (like how notmuch-search/hello do by using
switch-to-buffer). notmuch-show and tree assume the current buffer and
don't change it's visibility. This is how to current code works.

If it does make sense to add to them the no-display arg then we either
have to make all refresh functions interactive or non-interactive. By
doing this we can call all functions using a single code path and pass
the no-display arg to all in one call.

For notmuch-hello-update the situation looks pretty simple, it calls
notmuch-hello which is interactive. It already has the no-display arg,
we make it interactive and we're done. With notmuch-search-refresh-view
the situation is similar. Then call-interactively all and pass no-display.

Making them all non-interactive seems much harder and I think will
likely break stuff. So I prefer 1. using the code as is or 2. making
all interactive.

Any other ideas, everyone? I'm open to any kind of sugestions on this.

Ionel

>
> Best wishes
>
> Mark
>
>
>
>>  
>>  (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 586c84e..f3912d4 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -925,7 +925,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.
>> @@ -936,6 +936,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."
>> @@ -949,7 +952,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)
>> @@ -982,14 +987,16 @@ 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))
>> @@ -997,7 +1004,7 @@ same relative position within the new buffer."
>>  	(inhibit-read-only t))
>>      (remove-overlays)
>>      (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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] emacs: make the refresh code more consistent
  2016-10-06 20:59     ` Ioan-Adrian Ratiu
@ 2016-10-06 21:42       ` Mark Walters
  2016-10-07 14:49         ` Ioan-Adrian Ratiu
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2016-10-06 21:42 UTC (permalink / raw)
  To: notmuch

The current refresh code is a little haphazard with some of the
refresh functions called interactively, and some not. Some of the
refresh functions take arguments and they aren't consistent.

This makes all the functions have the same form.
---

This might be a sensible change to make before the series
id:20161006134227.17194-1-adi@adirat.com (or merge into that series).

I think the refresh functions should all be called non-interactively
as that will make it easier to pass arguments, and they should also
take the same arguments (though they can feel free to ignore them).

Best wishes

Mark


emacs/notmuch-hello.el |  2 +-
 emacs/notmuch-lib.el   | 22 ++++++++++++----------
 emacs/notmuch-show.el  |  2 +-
 emacs/notmuch-tree.el  |  5 ++---
 emacs/notmuch.el       |  2 +-
 5 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index d582bff..97280ca 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -604,7 +604,7 @@ with `notmuch-hello-query-counts'."
 
 (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))
 
-(defun notmuch-hello-update (&optional no-display)
+(defun notmuch-hello-update (&optional ignore no-display)
   "Update the current notmuch view."
   ;; Lazy - rebuild everything.
   (notmuch-hello no-display))
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index b2cdace..2d27e56 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -410,23 +410,25 @@ of its command symbol."
       (pop-to-buffer (help-buffer)))))
 
 (defvar notmuch-buffer-refresh-function nil
-  "Function to call to refresh the current buffer.")
+  "Function to call to refresh the current buffer.
+
+It will be called with two arguments: the first is the prefix
+argument when notmuch-refresh-this-buffer is called
+interactively, the second requests that the refresh call not
+display the buffer.")
 (make-variable-buffer-local 'notmuch-buffer-refresh-function)
 
-(defun notmuch-refresh-this-buffer ()
+(defun notmuch-refresh-this-buffer (prefix)
   "Refresh the current buffer."
-  (interactive)
+  (interactive "P")
   (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 prefix)))
 
-(defun notmuch-poll-and-refresh-this-buffer ()
+(defun notmuch-poll-and-refresh-this-buffer (prefix)
   "Invoke `notmuch-poll' to import mail, then refresh the current buffer."
-  (interactive)
+  (interactive "P")
   (notmuch-poll)
-  (notmuch-refresh-this-buffer))
+  (notmuch-refresh-this-buffer prefix))
 
 (defun notmuch-prettify-subject (subject)
   ;; This function is used by `notmuch-search-process-filter' which
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f2487ab..1772d10 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1358,7 +1358,7 @@ This includes:
     ;; Go to the previously open message.
     (notmuch-show-goto-message current)))
 
-(defun notmuch-show-refresh-view (&optional reset-state)
+(defun notmuch-show-refresh-view (&optional reset-state ignore)
   "Refresh the current view.
 
 Refreshes the current view, observing changes in display
diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index 1555812..c347712 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)
@@ -571,9 +570,9 @@ message will be \"unarchived\", i.e. the tag changes in
   (when (window-live-p notmuch-tree-message-window)
     (notmuch-tree-show-message-in)))
 
-(defun notmuch-tree-refresh-view ()
+(defun notmuch-tree-refresh-view (&rest ignore)
   "Refresh view."
-  (interactive)
+  (interactive "P")
   (let ((inhibit-read-only t)
 	(basic-query notmuch-tree-basic-query)
 	(query-context notmuch-tree-query-context)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 888672b..ee1bb54 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -982,7 +982,7 @@ 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 (&rest ignore)
   "Refresh the current view.
 
 Kills the current buffer and runs a new search with the same
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] emacs: make the refresh code more consistent
  2016-10-06 21:42       ` [PATCH] emacs: make the refresh code more consistent Mark Walters
@ 2016-10-07 14:49         ` Ioan-Adrian Ratiu
  0 siblings, 0 replies; 9+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-10-07 14:49 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Thu, 06 Oct 2016, Mark Walters <markwalters1009@gmail.com> wrote:
> The current refresh code is a little haphazard with some of the
> refresh functions called interactively, and some not. Some of the
> refresh functions take arguments and they aren't consistent.
>
> This makes all the functions have the same form.
> ---
>
> This might be a sensible change to make before the series
> id:20161006134227.17194-1-adi@adirat.com (or merge into that series).
>
> I think the refresh functions should all be called non-interactively
> as that will make it easier to pass arguments, and they should also
> take the same arguments (though they can feel free to ignore them).

Thank you for this, I'll pull it as is in my patch series and rebase
my patches on top of it and send v4 tonight.

Regards,
Ionel

>
> Best wishes
>
> Mark
>
>
> emacs/notmuch-hello.el |  2 +-
>  emacs/notmuch-lib.el   | 22 ++++++++++++----------
>  emacs/notmuch-show.el  |  2 +-
>  emacs/notmuch-tree.el  |  5 ++---
>  emacs/notmuch.el       |  2 +-
>  5 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index d582bff..97280ca 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -604,7 +604,7 @@ with `notmuch-hello-query-counts'."
>  
>  (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))
>  
> -(defun notmuch-hello-update (&optional no-display)
> +(defun notmuch-hello-update (&optional ignore no-display)
>    "Update the current notmuch view."
>    ;; Lazy - rebuild everything.
>    (notmuch-hello no-display))
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index b2cdace..2d27e56 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -410,23 +410,25 @@ of its command symbol."
>        (pop-to-buffer (help-buffer)))))
>  
>  (defvar notmuch-buffer-refresh-function nil
> -  "Function to call to refresh the current buffer.")
> +  "Function to call to refresh the current buffer.
> +
> +It will be called with two arguments: the first is the prefix
> +argument when notmuch-refresh-this-buffer is called
> +interactively, the second requests that the refresh call not
> +display the buffer.")
>  (make-variable-buffer-local 'notmuch-buffer-refresh-function)
>  
> -(defun notmuch-refresh-this-buffer ()
> +(defun notmuch-refresh-this-buffer (prefix)
>    "Refresh the current buffer."
> -  (interactive)
> +  (interactive "P")
>    (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 prefix)))
>  
> -(defun notmuch-poll-and-refresh-this-buffer ()
> +(defun notmuch-poll-and-refresh-this-buffer (prefix)
>    "Invoke `notmuch-poll' to import mail, then refresh the current buffer."
> -  (interactive)
> +  (interactive "P")
>    (notmuch-poll)
> -  (notmuch-refresh-this-buffer))
> +  (notmuch-refresh-this-buffer prefix))
>  
>  (defun notmuch-prettify-subject (subject)
>    ;; This function is used by `notmuch-search-process-filter' which
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f2487ab..1772d10 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1358,7 +1358,7 @@ This includes:
>      ;; Go to the previously open message.
>      (notmuch-show-goto-message current)))
>  
> -(defun notmuch-show-refresh-view (&optional reset-state)
> +(defun notmuch-show-refresh-view (&optional reset-state ignore)
>    "Refresh the current view.
>  
>  Refreshes the current view, observing changes in display
> diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
> index 1555812..c347712 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)
> @@ -571,9 +570,9 @@ message will be \"unarchived\", i.e. the tag changes in
>    (when (window-live-p notmuch-tree-message-window)
>      (notmuch-tree-show-message-in)))
>  
> -(defun notmuch-tree-refresh-view ()
> +(defun notmuch-tree-refresh-view (&rest ignore)
>    "Refresh view."
> -  (interactive)
> +  (interactive "P")
>    (let ((inhibit-read-only t)
>  	(basic-query notmuch-tree-basic-query)
>  	(query-context notmuch-tree-query-context)
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 888672b..ee1bb54 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -982,7 +982,7 @@ 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 (&rest ignore)
>    "Refresh the current view.
>  
>  Kills the current buffer and runs a new search with the same
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-10-07 14:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-06 13:42 [PATCH v3 0/4] Add refresh all buffers functionality Ioan-Adrian Ratiu
2016-10-06 13:42 ` [PATCH v3 1/4] emacs: reuse buffer when refreshing searches Ioan-Adrian Ratiu
2016-10-06 13:42 ` [PATCH v3 2/4] emacs: notmuch-show: refresh all windows showing a buffer Ioan-Adrian Ratiu
2016-10-06 13:42 ` [PATCH v3 3/4] emacs: add refresh buffer optional no-display arg Ioan-Adrian Ratiu
2016-10-06 17:24   ` Mark Walters
2016-10-06 20:59     ` Ioan-Adrian Ratiu
2016-10-06 21:42       ` [PATCH] emacs: make the refresh code more consistent Mark Walters
2016-10-07 14:49         ` Ioan-Adrian Ratiu
2016-10-06 13:42 ` [PATCH v3 4/4] emacs: notmuch-lib: add refresh all buffers function Ioan-Adrian Ratiu

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).