From: Michal Sojka <sojkam1@fel.cvut.cz>
To: 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: <877gs9msy8.fsf@nikula.org>
References: <cover.1346596156.git.jani@nikula.org>
	<87vcftbtmw.fsf@steelpick.2x.cz> <877gs9msy8.fsf@nikula.org>
User-Agent: Notmuch/0.14+3~g608c52f (http://notmuchmail.org) Emacs/24.1.1
	(x86_64-pc-linux-gnu)
Date: Wed, 05 Sep 2012 23:08:11 +0200
Message-ID: <87zk54yy84.fsf@steelpick.2x.cz>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

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 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.

> 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.

-Michal