From: Tomi Ollila <tomi.ollila@iki.fi>
To: Michal Sojka <sojkam1@fel.cvut.cz>, Jani Nikula <jani@nikula.org>,
	notmuch@notmuchmail.org
Subject: Re: [PATCH v2 0/5] emacs: notmuch-hello status message refresh hook,
	etc
In-Reply-To: <87zk54yy84.fsf@steelpick.2x.cz>
References: <cover.1346596156.git.jani@nikula.org>
	<87vcftbtmw.fsf@steelpick.2x.cz> <877gs9msy8.fsf@nikula.org>
	<87zk54yy84.fsf@steelpick.2x.cz>
User-Agent: Notmuch/0.14+11~gd9bf007 (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-unknown-linux-gnu)
Date: Thu, 06 Sep 2012 18:28:46 +0300
Message-ID: <m2k3w7xj9t.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Thu, Sep 06 2012, Michal Sojka wrote:

> On Tue, Sep 04 2012, Jani Nikula wrote:
>> On Tue, 04 Sep 2012, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>>> On Sun, Sep 02 2012, Jani Nikula wrote:
>>>> Hi all, v2 of [1] addressing review comments: added NEWS, improved
>>>> documentation, made the query customizable.
>>>>
>>>> [1]
>>>>     id:"37899e28dbf67e4620a53279a869be3174c02d6f.1339775602.git.jani@nikula.org"
>>>
>>> Hello Jani,
>>>
>>> if I understand correctly, the goal of this patchset is to display the
>>> message with the difference in the number of messages before and after
>>> refresh of notmuch-hello.
>>
>> Difference between two refreshes of notmuch-hello to be specific.
>>
>>> I think the current implementation is unnecessarily complicated. It
>>> would be sufficient to implement this directly in `notmuch-hello'
>>> without any hook. If `notmuch-hello-refresh-status-query' is nil
>>> (default) no message would be shown. If it is configured to something
>>> else, the message would be shown provided that no-display is nil. This
>>> way you don't have to break existing user's hooks and achieve the same
>>> behavior, don't you?
>>
>> I think it would be useful to pass no-display to hooks anyway,
>> regardless of the use here.
>
> I'm strongly against breaking existing user's setups without a good
> reason. The meaning of no-display parameter, as I understand it, is only
> to tells whether the refresh should happen in background or foreground.
> Isn't there any other method for hooks to check for this conditions? For
> example something like checking whether notmuch-hello buffer is shown in
> a window of the active frame? Or a simple (perhaps buffer-local)
> variable can contain this information.

I mentioned using no-display from dynamic scope -- which isn't very
good solution in a long run. Lately I've been leaning to Jani's suggestion
by just calling refresh-hook with no-display arg.... But if there is
well-established way to provide this information to hooks without
adding this argument in the hook call then that could be used..

> I don't know what people use notmuch-hello-refresh-hook for. I suppose
> that your use case (displaying a message outside of hello buffer) is
> rather an exception than a typical use.

In any case, I think the refresh-hook needs to know this no-display
information, somehow. If there is no other good way than giving it
as an argument to the hook then that should be done (by pushing patches
1-3).

>> I don't see that as a big reason to do this one way or the other.
>> Having this implemented directly in notmuch-hello does not make this
>> less complicated either.
>
> That's true, but it would be simpler for users to set it up. With your
> patches, users would have to configure the variable (query) as well as
> the hook. I propose to have only one place to configure this feature.

I've been thinking that Jani's way to provide the information to minibuffer
is a bit better than provide it in hello buffer -- as I think there is a
small "imperfection" that may irritate users: The default '*' is in use.
The hello buffer is refreshed (via emacsclient or something?) with
no-display t -- message count changes but the reference count
(notmuch-hello-refresh-count) keeps the same. Next time the hello buffer is
visible, showing current message count -- and user refreshes it.  In most
cases message count change in hello buffer is different that what will be
shown in minibuffer.

Now, If this "imperfection" reason enough to have this feature available
from wiki/contrib. 

If not, then the decision between hook/part of notmuch-hello is to be done:

1) If hook, then one less if's to be used -- and customization useful for
those who don't (add-hook...) more hooks there.

2) If somewhere in notmuch-hello processing then this "query-variable"
(named appropriately) can be used to check whether this feature is in 
use; if nil then not, otherwise it contains the query variable to be used
in (notmuch-command "count" ...) call.

> -Michal

Tomi