[PATCH v5 2/7] emacs: make the refresh functions more consistent
Tomi Ollila
tomi.ollila at iki.fi
Mon Oct 10 11:17:11 PDT 2016
On Mon, Oct 10 2016, Mark Walters <markwalters1009 at gmail.com> wrote:
> 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:
>>
>>> 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
More information about the notmuch
mailing list