[PATCH 1/3] emacs: add no-display arg to notmuch-hello-refresh-hook

[PATCH 1/3] emacs: add no-display arg to notmuch-hello-refresh-hook

[Jani Nikula]

Add no-display arg to notmuch-hello-refresh-hook to allow each hook to
decide what is appropriate when no-display is t, which is typically
the case when called non-interactively. This is used by the following
patch.

This breaks existing hooks people might have, which will now need to
accept the argument.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 emacs/notmuch-hello.el |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 684bedc..bc43178 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -812,7 +812,7 @@ following:
       (unless (widget-at)
 	(when notmuch-hello-search-pos
 	  (goto-char notmuch-hello-search-pos)))))
-  (run-hooks 'notmuch-hello-refresh-hook)
+  (run-hook-with-args 'notmuch-hello-refresh-hook no-display)
   (setq notmuch-hello-first-run nil))
 
 (defun notmuch-folder ()
-- 
1.7.9.5

[PATCH 2/3] test: fix hook-counter to accept the new no-display param

[Jani Nikula]

notmuch-hello-refresh-hook is now called with no-display param. Accept
(and ignore) it in hook-counter.
---
 test/test-lib.el |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 6271da2..9f66b0e 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -74,7 +74,7 @@ running, quit if it terminated."
       (kill-emacs)
     (run-at-time "1 min" nil 'orphan-watchdog pid)))
 
-(defun hook-counter (hook)
+(defun hook-counter (hook &rest no-display)
   "Count how many times a hook is called.  Increments
 `hook'-counter variable value if it is bound, otherwise does
 nothing."
-- 
1.7.9.5

[PATCH 3/3] emacs: add notmuch hello refresh hook to display message count change

[Jani Nikula]

Add a notmuch hello refresh hook to display a message about change in
message count in the database since the notmuch-hello buffer was last
refreshed manually (no-display is nil).

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 emacs/notmuch-hello.el |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index bc43178..bcca044 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -148,6 +148,7 @@ International Bureau of Weights and Measures."
 (defcustom notmuch-hello-refresh-hook nil
   "Functions called after updating a `notmuch-hello' buffer."
   :type 'hook
+  :options '(notmuch-hello-refresh-status-message)
   :group 'notmuch-hello
   :group 'notmuch-hooks)
 
@@ -749,6 +750,32 @@ following:
     (let ((fill-column (- (window-width) notmuch-hello-indent)))
       (center-region start (point)))))
 
+(defvar notmuch-hello-refresh-count 0
+  "Number of messages in the database when `notmuch-hello' was last run.
+
+Used internally by `notmuch-hello-refresh-status-message'.")
+
+(defun notmuch-hello-refresh-status-message (no-display)
+  "Hook to display a status message when refreshing notmuch-hello buffer.
+
+Display a status message about the difference in message count in
+the database since the last call."
+  (unless no-display
+    (let* ((new-count
+	    (string-to-number (car (process-lines notmuch-command "count"))))
+	   (diff-count (- new-count notmuch-hello-refresh-count)))
+      (cond
+       ((= notmuch-hello-refresh-count 0)
+	(message "You have %s messages."
+		 (notmuch-hello-nice-number new-count)))
+       ((> diff-count 0)
+	(message "You have %s more messages since last refresh."
+		 (notmuch-hello-nice-number diff-count)))
+       ((< diff-count 0)
+	(message "You have %s fewer messages since last refresh."
+		 (notmuch-hello-nice-number (- diff-count)))))
+      (setq notmuch-hello-refresh-count new-count))))
+
 ;;;###autoload
 (defun notmuch-hello (&optional no-display)
   "Run notmuch and display saved searches, known tags, etc."
-- 
1.7.9.5

Re: [PATCH 3/3] emacs: add notmuch hello refresh hook to display message count change

[Mark Walters]

On Fri, 15 Jun 2012, Jani Nikula <jani@nikula.org> wrote:
> Add a notmuch hello refresh hook to display a message about change in
> message count in the database since the notmuch-hello buffer was last
> refreshed manually (no-display is nil).
>
> Signed-off-by: Jani Nikula <jani@nikula.org>

Hi

I just have a couple of comments/queries. First, since it breaks
existing hooks I think it needs a NEWS item. 

Secondly, I, personally, would much prefer the hook with the slight
tweak that it counts inbox messages rather than all messages. Is that
an option that could be put in the defcustom?

Best wishes

Mark

> ---
>  emacs/notmuch-hello.el |   27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index bc43178..bcca044 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -148,6 +148,7 @@ International Bureau of Weights and Measures."
>  (defcustom notmuch-hello-refresh-hook nil
>    "Functions called after updating a `notmuch-hello' buffer."
>    :type 'hook
> +  :options '(notmuch-hello-refresh-status-message)
>    :group 'notmuch-hello
>    :group 'notmuch-hooks)
>  
> @@ -749,6 +750,32 @@ following:
>      (let ((fill-column (- (window-width) notmuch-hello-indent)))
>        (center-region start (point)))))
>  
> +(defvar notmuch-hello-refresh-count 0
> +  "Number of messages in the database when `notmuch-hello' was last run.
> +
> +Used internally by `notmuch-hello-refresh-status-message'.")
> +
> +(defun notmuch-hello-refresh-status-message (no-display)
> +  "Hook to display a status message when refreshing notmuch-hello buffer.
> +
> +Display a status message about the difference in message count in
> +the database since the last call."
> +  (unless no-display
> +    (let* ((new-count
> +	    (string-to-number (car (process-lines notmuch-command "count"))))
> +	   (diff-count (- new-count notmuch-hello-refresh-count)))
> +      (cond
> +       ((= notmuch-hello-refresh-count 0)
> +	(message "You have %s messages."
> +		 (notmuch-hello-nice-number new-count)))
> +       ((> diff-count 0)
> +	(message "You have %s more messages since last refresh."
> +		 (notmuch-hello-nice-number diff-count)))
> +       ((< diff-count 0)
> +	(message "You have %s fewer messages since last refresh."
> +		 (notmuch-hello-nice-number (- diff-count)))))
> +      (setq notmuch-hello-refresh-count new-count))))
> +
>  ;;;###autoload
>  (defun notmuch-hello (&optional no-display)
>    "Run notmuch and display saved searches, known tags, etc."
> -- 
> 1.7.9.5
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/3] emacs: add notmuch hello refresh hook to display message count change

[Jani Nikula]

[-- Attachment #1: Type: text/plain, Size: 3146 bytes --]

On Jul 8, 2012 9:57 AM, "Mark Walters" <markwalters1009@gmail.com> wrote:
>
> On Fri, 15 Jun 2012, Jani Nikula <jani@nikula.org> wrote:
> > Add a notmuch hello refresh hook to display a message about change in
> > message count in the database since the notmuch-hello buffer was last
> > refreshed manually (no-display is nil).
> >
> > Signed-off-by: Jani Nikula <jani@nikula.org>
>
> Hi
>
> I just have a couple of comments/queries. First, since it breaks
> existing hooks I think it needs a NEWS item.

Sure.

> Secondly, I, personally, would much prefer the hook with the slight
> tweak that it counts inbox messages rather than all messages. Is that
> an option that could be put in the defcustom?

I suppose the query could be customizable. That's simple enough; for
anything non-trivial people can write their own hooks.

BR,
Jani.

>
> Best wishes
>
> Mark
>
> > ---
> >  emacs/notmuch-hello.el |   27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> > index bc43178..bcca044 100644
> > --- a/emacs/notmuch-hello.el
> > +++ b/emacs/notmuch-hello.el
> > @@ -148,6 +148,7 @@ International Bureau of Weights and Measures."
> >  (defcustom notmuch-hello-refresh-hook nil
> >    "Functions called after updating a `notmuch-hello' buffer."
> >    :type 'hook
> > +  :options '(notmuch-hello-refresh-status-message)
> >    :group 'notmuch-hello
> >    :group 'notmuch-hooks)
> >
> > @@ -749,6 +750,32 @@ following:
> >      (let ((fill-column (- (window-width) notmuch-hello-indent)))
> >        (center-region start (point)))))
> >
> > +(defvar notmuch-hello-refresh-count 0
> > +  "Number of messages in the database when `notmuch-hello' was last
run.
> > +
> > +Used internally by `notmuch-hello-refresh-status-message'.")
> > +
> > +(defun notmuch-hello-refresh-status-message (no-display)
> > +  "Hook to display a status message when refreshing notmuch-hello
buffer.
> > +
> > +Display a status message about the difference in message count in
> > +the database since the last call."
> > +  (unless no-display
> > +    (let* ((new-count
> > +         (string-to-number (car (process-lines notmuch-command
"count"))))
> > +        (diff-count (- new-count notmuch-hello-refresh-count)))
> > +      (cond
> > +       ((= notmuch-hello-refresh-count 0)
> > +     (message "You have %s messages."
> > +              (notmuch-hello-nice-number new-count)))
> > +       ((> diff-count 0)
> > +     (message "You have %s more messages since last refresh."
> > +              (notmuch-hello-nice-number diff-count)))
> > +       ((< diff-count 0)
> > +     (message "You have %s fewer messages since last refresh."
> > +              (notmuch-hello-nice-number (- diff-count)))))
> > +      (setq notmuch-hello-refresh-count new-count))))
> > +
> >  ;;;###autoload
> >  (defun notmuch-hello (&optional no-display)
> >    "Run notmuch and display saved searches, known tags, etc."
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

[-- Attachment #2: Type: text/html, Size: 4449 bytes --]

Re: [PATCH 1/3] emacs: add no-display arg to notmuch-hello-refresh-hook

[Austin Clements]

Quoth Jani Nikula on Jun 15 at  6:53 pm:
> Add no-display arg to notmuch-hello-refresh-hook to allow each hook to
> decide what is appropriate when no-display is t, which is typically
> the case when called non-interactively. This is used by the following
> patch.
> 
> This breaks existing hooks people might have, which will now need to
> accept the argument.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>

This seems like an overloaded use of no-display.  If I'm reading the
code right, no-display indicates whether or not the notmuch-hello
buffer should be switched to and seems like a workaround for some
particular corner-case (I'm not even sure what).  This seems like a
strange condition to predicate a hook on (but maybe I just don't
understand).  What condition, abstractly speaking, is
notmuch-hello-refresh-status-message trying to run under?

Re: [PATCH 1/3] emacs: add no-display arg to notmuch-hello-refresh-hook

[Jani Nikula]

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

On Jul 10, 2012 4:49 AM, "Austin Clements" <amdragon@mit.edu> wrote:
>
> Quoth Jani Nikula on Jun 15 at  6:53 pm:
> > Add no-display arg to notmuch-hello-refresh-hook to allow each hook to
> > decide what is appropriate when no-display is t, which is typically
> > the case when called non-interactively. This is used by the following
> > patch.
> >
> > This breaks existing hooks people might have, which will now need to
> > accept the argument.
> >
> > Signed-off-by: Jani Nikula <jani@nikula.org>
>
> This seems like an overloaded use of no-display.  If I'm reading the
> code right, no-display indicates whether or not the notmuch-hello
> buffer should be switched to and seems like a workaround for some
> particular corner-case (I'm not even sure what).  This seems like a
> strange condition to predicate a hook on (but maybe I just don't
> understand).  What condition, abstractly speaking, is
> notmuch-hello-refresh-status-message trying to run under?

IIUC, no-display is useful for calling refresh from outside of emacs, e.g.
from post-new hook in an automated fashion, so you can have an up-to-date
buffer when you switch to it. There's no point in displaying the refresh
message when you don't also switch to the buffer, is there? And this way
you'll get the diff between the manual (through user interaction) refreshes
of the buffer, not between two cron jobs.

J.

[-- Attachment #2: Type: text/html, Size: 1715 bytes --]

Re: [PATCH 1/3] emacs: add no-display arg to notmuch-hello-refresh-hook

[Austin Clements]

Quoth Jani Nikula on Jul 10 at  8:10 am:
>    On Jul 10, 2012 4:49 AM, "Austin Clements" <[1]amdragon@mit.edu> wrote:
>    >
>    > Quoth Jani Nikula on Jun 15 at  6:53 pm:
>    > > Add no-display arg to notmuch-hello-refresh-hook to allow each hook to
>    > > decide what is appropriate when no-display is t, which is typically
>    > > the case when called non-interactively. This is used by the following
>    > > patch.
>    > >
>    > > This breaks existing hooks people might have, which will now need to
>    > > accept the argument.
>    > >
>    > > Signed-off-by: Jani Nikula <[2]jani@nikula.org>
>    >
>    > This seems like an overloaded use of no-display.  If I'm reading the
>    > code right, no-display indicates whether or not the notmuch-hello
>    > buffer should be switched to and seems like a workaround for some
>    > particular corner-case (I'm not even sure what).  This seems like a
>    > strange condition to predicate a hook on (but maybe I just don't
>    > understand).  What condition, abstractly speaking, is
>    > notmuch-hello-refresh-status-message trying to run under?
> 
>    IIUC, no-display is useful for calling refresh from outside of emacs, e.g.
>    from post-new hook in an automated fashion, so you can have an up-to-date
>    buffer when you switch to it. There's no point in displaying the refresh
>    message when you don't also switch to the buffer, is there? And this way
>    you'll get the diff between the manual (through user interaction)
>    refreshes of the buffer, not between two cron jobs.

Oh, I see.  This makes more sense.  The only use of no-display in the
notmuch code I can find appears to be to refresh the hello buffer when
you exit a search buffer started from hello, when it might actually
make sense to run the difference hook in your patch (I assume it wants
to avoid switching to the hello buffer in this case because the buffer
order may have changed?  Of course, there are other ways to get to the
hello buffer other than exiting a search buffer started from it...  I
found all of this code very confusing).

Maybe this just needs better documentation?