From: Tomi Ollila Date: Thu, 6 Sep 2012 15:28:46 +0000 (+0300) Subject: Re: [PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=751d16df8342bcc6f0dae99632d95f97bc2eb121;p=notmuch-archives.git Re: [PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc --- diff --git a/73/1394b13ee36d8d7b99dfe3d79a17f01ee4d049 b/73/1394b13ee36d8d7b99dfe3d79a17f01ee4d049 new file mode 100644 index 000000000..426cf6352 --- /dev/null +++ b/73/1394b13ee36d8d7b99dfe3d79a17f01ee4d049 @@ -0,0 +1,141 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id BEBB4431FAF + for ; Thu, 6 Sep 2012 08:28:40 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0 +X-Spam-Level: +X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] + autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id I3PgQTSxQFa6 for ; + Thu, 6 Sep 2012 08:28:39 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by olra.theworths.org (Postfix) with ESMTP id 6E585431FAE + for ; Thu, 6 Sep 2012 08:28:39 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id 0C6151000E5; + Thu, 6 Sep 2012 18:28:47 +0300 (EEST) +From: Tomi Ollila +To: Michal Sojka , Jani Nikula , + 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: + <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) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Thu, 06 Sep 2012 15:28:40 -0000 + +On Thu, Sep 06 2012, Michal Sojka wrote: + +> On Tue, Sep 04 2012, Jani Nikula wrote: +>> On Tue, 04 Sep 2012, Michal Sojka 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