[PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc
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 at 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
Re: [PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc
On Thu, Sep 06 2012, Michal Sojka wrote: On Tue, Sep 04 2012, Jani Nikula wrote: On Tue, 04 Sep 2012, Michal Sojka sojk...@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.j...@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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc
On Tue, Sep 04 2012, Jani Nikula wrote: On Tue, 04 Sep 2012, Michal Sojka sojk...@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.j...@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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc
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 at > 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. 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? -Michal
[PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc
On Sun, Sep 02 2012, Jani Nikula wrote: > Hi all, v2 of [1] addressing review comments: added NEWS, improved > documentation, made the query customizable. Hi Jani Just today I was looking these older patches below and was about to comment those when these arrived. I think the idea is great, but there is just something in the integration... > [1] id:"37899e28dbf67e4620a53279a869be3174c02d6f.1339775602.git.jani at > nikula.org" I see hooks a great way to extend some functionality outside of that package instead of touching it. Also, maybe I've been using emacs too much as these hooks as defcustom seems little weird. Yet as I grepped 'defcustom.*hook' through emacs-24.2/lisp/*.el and I got many hits -- and some of those even added some ":options" to those (but most those I looked did not specify anything to choose from). That said, I am not _personally_ very thilled about the idea of defining functions inside notmuch and then that work via a hook... Therefore I'd like to propose an alternative way to do this: Integrate this change status information to the notmuch-hello-insert-header () function, disabled by default -- and add customization variable which can be used to enable it. like: Welcome to notmuch. You have 22 195 messages. Welcome to notmuch. You have 22 185 messages (-10). Welcome to notmuch. You have 22 205 messages (+20). or second line for the status info. I would enable this feature if implemented this way. Also I would enable this implemented like this patch series if this is to be applied (I just give +-0). But this could also be implemented as a hook outside of notmuch relying that variable no-display is available in dynamic scope -- but without promising the variable will be there. > BR, > Jani. Finally, I hope my comment does not silence the audience but that this sparks more comment and good solution is used for this useful feature. Tomi > > Jani Nikula (5): > emacs: document the notmuch-hello no-display argument > emacs: add no-display arg to notmuch-hello-refresh-hook > test: fix hook-counter to accept the new no-display param > emacs: add notmuch hello refresh hook to display message count change > NEWS: notmuch-hello refresh hook changes > > NEWS | 18 ++ > emacs/notmuch-hello.el | 46 -- > test/test-lib.el |2 +- > 3 files changed, 63 insertions(+), 3 deletions(-) > > -- > 1.7.9.5 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc
Hi all, v2 of [1] addressing review comments: added NEWS, improved documentation, made the query customizable. [1] id:"37899e28dbf67e4620a53279a869be3174c02d6f.1339775602.git.jani at nikula.org" BR, Jani. Jani Nikula (5): emacs: document the notmuch-hello no-display argument emacs: add no-display arg to notmuch-hello-refresh-hook test: fix hook-counter to accept the new no-display param emacs: add notmuch hello refresh hook to display message count change NEWS: notmuch-hello refresh hook changes NEWS | 18 ++ emacs/notmuch-hello.el | 46 -- test/test-lib.el |2 +- 3 files changed, 63 insertions(+), 3 deletions(-) -- 1.7.9.5
