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

[Ioan-Adrian Ratiu]

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