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

[Mark Walters]

On Sun, 09 Oct 2016, Tomi Ollila <tomi.ollila at iki.fi> wrote:
> On Sun, Oct 09 2016, Mark Walters <markwalters1009 at 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 at notmuchmail.org
>> https://notmuchmail.org/mailman/listinfo/notmuch