Re: [PATCH v2 0/5] emacs: notmuch-hello status message refresh hook, etc

[Tomi Ollila <tomi.ollila@iki.fi>]

On Sun, Sep 02 2012, Jani Nikula <jani@nikula.org> 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@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@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch