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