unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
@ 2011-12-11 21:48 Jani Nikula
  2011-12-11 22:00 ` Dmitry Kurochkin
                   ` (2 more replies)
  0 siblings, 3 replies; 54+ messages in thread
From: Jani Nikula @ 2011-12-11 21:48 UTC (permalink / raw)
  To: notmuch

Let notmuch-poll-script be a function as well as a string. Make default
value nil instead of an empty string, but allow "" for backwards
compatibility. Add a notmuch poll function to call "notmuch new" using the
configured notmuch-command.

This allows taking better advantage of the "notmuch new" hooks from emacs
without intermediate scripts.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 emacs/notmuch.el |   44 ++++++++++++++++++++++++++++++++------------
 1 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 8936149..6c7e800 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -965,28 +965,48 @@ same relative position within the new buffer."
     (notmuch-search query oldest-first target-thread target-line continuation)
     (goto-char (point-min))))
 
-(defcustom notmuch-poll-script ""
-  "An external script to incorporate new mail into the notmuch database.
+(defcustom notmuch-poll-script nil
+  "A script or a function to incorporate new mail into the notmuch database.
 
-If this variable is non empty, then it should name a script to be
-invoked by `notmuch-search-poll-and-refresh-view' and
-`notmuch-hello-poll-and-update' (each have a default keybinding
-of 'G'). The script could do any of the following depending on
+This variable can be set to a function or the name of an external
+script to be invoked by `notmuch-search-poll-and-refresh-view'
+and `notmuch-hello-poll-and-update' (each have a default
+keybinding of 'G'). Set to nil to do nothing.
+
+The function or script could do any of the following depending on
 the user's needs:
 
 1. Invoke a program to transfer mail to the local mail store
 2. Invoke \"notmuch new\" to incorporate the new mail
-3. Invoke one or more \"notmuch tag\" commands to classify the mail"
-  :type 'string
+3. Invoke one or more \"notmuch tag\" commands to classify the mail
+
+You can also choose to use \"notmuch new\" through the provided
+`notmuch-poll-script-notmuch-new' function, and have the
+\"notmuch new\" hooks perform the actions above."
+  :type '(choice (const :tag "Not set" nil)
+		 (const :tag "Notmuch new" notmuch-poll-script-notmuch-new)
+		 (function :tag "Custom function"
+			   :value notmuch-poll-script-notmuch-new)
+		 (string :tag "Custom script"))
   :group 'notmuch)
 
+(defun notmuch-poll-script-notmuch-new ()
+  "Run \"notmuch new\"."
+  (call-process notmuch-command nil nil nil "new"))
+
 (defun notmuch-poll ()
-  "Run external script to import mail.
+  "Run external script or call a function to import mail.
 
-Invokes `notmuch-poll-script' if it is not set to an empty string."
+Invokes `notmuch-poll-script', which can be a function or the
+name of an external script. Does nothing if `notmuch-poll-script'
+is nil or an empty string."
   (interactive)
-  (if (not (string= notmuch-poll-script ""))
-      (call-process notmuch-poll-script nil nil)))
+  (cond
+   ((stringp notmuch-poll-script)
+    (if (not (string= notmuch-poll-script "")) ;; for backwards compatibility
+	(call-process notmuch-poll-script nil nil)))
+   ((functionp notmuch-poll-script)
+    (funcall notmuch-poll-script))))
 
 (defun notmuch-search-poll-and-refresh-view ()
   "Invoke `notmuch-poll' to import mail, then refresh the current view."
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 21:48 [PATCH] emacs: support "notmuch new" as a notmuch-poll-script Jani Nikula
@ 2011-12-11 22:00 ` Dmitry Kurochkin
  2011-12-11 22:19   ` Jani Nikula
  2011-12-11 22:58   ` Austin Clements
  2011-12-12 19:57 ` [PATCH v2] " Jani Nikula
  2011-12-12 20:50 ` [PATCH v3] " Jani Nikula
  2 siblings, 2 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2011-12-11 22:00 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Hi Jani.

On Sun, 11 Dec 2011 23:48:20 +0200, Jani Nikula <jani@nikula.org> wrote:
> Let notmuch-poll-script be a function as well as a string. Make default
> value nil instead of an empty string, but allow "" for backwards
> compatibility. Add a notmuch poll function to call "notmuch new" using the
> configured notmuch-command.
> 
> This allows taking better advantage of the "notmuch new" hooks from emacs
> without intermediate scripts.
> 

I was just thinking about working on this myself :)

I think a better solution would be to allow running a command with
arguments.  Creating a elisp function just to run a command with some
parameters feels wrong.  This way we would have to add another function
each time we want to add another argument.

Function support for notmuch-poll-script seems like a useful feature on
it's own.

Regards,
  Dmitry

> Signed-off-by: Jani Nikula <jani@nikula.org>
> ---
>  emacs/notmuch.el |   44 ++++++++++++++++++++++++++++++++------------
>  1 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 8936149..6c7e800 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -965,28 +965,48 @@ same relative position within the new buffer."
>      (notmuch-search query oldest-first target-thread target-line continuation)
>      (goto-char (point-min))))
>  
> -(defcustom notmuch-poll-script ""
> -  "An external script to incorporate new mail into the notmuch database.
> +(defcustom notmuch-poll-script nil
> +  "A script or a function to incorporate new mail into the notmuch database.
>  
> -If this variable is non empty, then it should name a script to be
> -invoked by `notmuch-search-poll-and-refresh-view' and
> -`notmuch-hello-poll-and-update' (each have a default keybinding
> -of 'G'). The script could do any of the following depending on
> +This variable can be set to a function or the name of an external
> +script to be invoked by `notmuch-search-poll-and-refresh-view'
> +and `notmuch-hello-poll-and-update' (each have a default
> +keybinding of 'G'). Set to nil to do nothing.
> +
> +The function or script could do any of the following depending on
>  the user's needs:
>  
>  1. Invoke a program to transfer mail to the local mail store
>  2. Invoke \"notmuch new\" to incorporate the new mail
> -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> -  :type 'string
> +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> +
> +You can also choose to use \"notmuch new\" through the provided
> +`notmuch-poll-script-notmuch-new' function, and have the
> +\"notmuch new\" hooks perform the actions above."
> +  :type '(choice (const :tag "Not set" nil)
> +		 (const :tag "Notmuch new" notmuch-poll-script-notmuch-new)
> +		 (function :tag "Custom function"
> +			   :value notmuch-poll-script-notmuch-new)
> +		 (string :tag "Custom script"))
>    :group 'notmuch)
>  
> +(defun notmuch-poll-script-notmuch-new ()
> +  "Run \"notmuch new\"."
> +  (call-process notmuch-command nil nil nil "new"))
> +
>  (defun notmuch-poll ()
> -  "Run external script to import mail.
> +  "Run external script or call a function to import mail.
>  
> -Invokes `notmuch-poll-script' if it is not set to an empty string."
> +Invokes `notmuch-poll-script', which can be a function or the
> +name of an external script. Does nothing if `notmuch-poll-script'
> +is nil or an empty string."
>    (interactive)
> -  (if (not (string= notmuch-poll-script ""))
> -      (call-process notmuch-poll-script nil nil)))
> +  (cond
> +   ((stringp notmuch-poll-script)
> +    (if (not (string= notmuch-poll-script "")) ;; for backwards compatibility
> +	(call-process notmuch-poll-script nil nil)))
> +   ((functionp notmuch-poll-script)
> +    (funcall notmuch-poll-script))))
>  
>  (defun notmuch-search-poll-and-refresh-view ()
>    "Invoke `notmuch-poll' to import mail, then refresh the current view."
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 22:00 ` Dmitry Kurochkin
@ 2011-12-11 22:19   ` Jani Nikula
  2011-12-11 22:39     ` Dmitry Kurochkin
  2011-12-11 22:58   ` Austin Clements
  1 sibling, 1 reply; 54+ messages in thread
From: Jani Nikula @ 2011-12-11 22:19 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch


Hi Dmitry -

On Mon, 12 Dec 2011 02:00:45 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Sun, 11 Dec 2011 23:48:20 +0200, Jani Nikula <jani@nikula.org> wrote:
> > Let notmuch-poll-script be a function as well as a string. Make default
> > value nil instead of an empty string, but allow "" for backwards
> > compatibility. Add a notmuch poll function to call "notmuch new" using the
> > configured notmuch-command.
> > 
> > This allows taking better advantage of the "notmuch new" hooks from emacs
> > without intermediate scripts.
> > 
> 
> I was just thinking about working on this myself :)

:)

> I think a better solution would be to allow running a command with
> arguments.  Creating a elisp function just to run a command with some
> parameters feels wrong.  This way we would have to add another function
> each time we want to add another argument.

One thing to take into account is running "notmuch new" using
notmuch-command, and make that happen with one click in custom.

> Function support for notmuch-poll-script seems like a useful feature on
> it's own.

I just did this quickly to scratch my own itches, so to speak. My elisp
is not that great, so I really wouldn't mind if you wanted to continue
from here and make it your own. It'll be a while before I have the time
to (figure out how to) do this properly anyway. But up to you, really.

BR,
Jani.


> 
> Regards,
>   Dmitry
> 
> > Signed-off-by: Jani Nikula <jani@nikula.org>
> > ---
> >  emacs/notmuch.el |   44 ++++++++++++++++++++++++++++++++------------
> >  1 files changed, 32 insertions(+), 12 deletions(-)
> > 
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index 8936149..6c7e800 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -965,28 +965,48 @@ same relative position within the new buffer."
> >      (notmuch-search query oldest-first target-thread target-line continuation)
> >      (goto-char (point-min))))
> >  
> > -(defcustom notmuch-poll-script ""
> > -  "An external script to incorporate new mail into the notmuch database.
> > +(defcustom notmuch-poll-script nil
> > +  "A script or a function to incorporate new mail into the notmuch database.
> >  
> > -If this variable is non empty, then it should name a script to be
> > -invoked by `notmuch-search-poll-and-refresh-view' and
> > -`notmuch-hello-poll-and-update' (each have a default keybinding
> > -of 'G'). The script could do any of the following depending on
> > +This variable can be set to a function or the name of an external
> > +script to be invoked by `notmuch-search-poll-and-refresh-view'
> > +and `notmuch-hello-poll-and-update' (each have a default
> > +keybinding of 'G'). Set to nil to do nothing.
> > +
> > +The function or script could do any of the following depending on
> >  the user's needs:
> >  
> >  1. Invoke a program to transfer mail to the local mail store
> >  2. Invoke \"notmuch new\" to incorporate the new mail
> > -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> > -  :type 'string
> > +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> > +
> > +You can also choose to use \"notmuch new\" through the provided
> > +`notmuch-poll-script-notmuch-new' function, and have the
> > +\"notmuch new\" hooks perform the actions above."
> > +  :type '(choice (const :tag "Not set" nil)
> > +		 (const :tag "Notmuch new" notmuch-poll-script-notmuch-new)
> > +		 (function :tag "Custom function"
> > +			   :value notmuch-poll-script-notmuch-new)
> > +		 (string :tag "Custom script"))
> >    :group 'notmuch)
> >  
> > +(defun notmuch-poll-script-notmuch-new ()
> > +  "Run \"notmuch new\"."
> > +  (call-process notmuch-command nil nil nil "new"))
> > +
> >  (defun notmuch-poll ()
> > -  "Run external script to import mail.
> > +  "Run external script or call a function to import mail.
> >  
> > -Invokes `notmuch-poll-script' if it is not set to an empty string."
> > +Invokes `notmuch-poll-script', which can be a function or the
> > +name of an external script. Does nothing if `notmuch-poll-script'
> > +is nil or an empty string."
> >    (interactive)
> > -  (if (not (string= notmuch-poll-script ""))
> > -      (call-process notmuch-poll-script nil nil)))
> > +  (cond
> > +   ((stringp notmuch-poll-script)
> > +    (if (not (string= notmuch-poll-script "")) ;; for backwards compatibility
> > +	(call-process notmuch-poll-script nil nil)))
> > +   ((functionp notmuch-poll-script)
> > +    (funcall notmuch-poll-script))))
> >  
> >  (defun notmuch-search-poll-and-refresh-view ()
> >    "Invoke `notmuch-poll' to import mail, then refresh the current view."
> > -- 
> > 1.7.5.4
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 22:19   ` Jani Nikula
@ 2011-12-11 22:39     ` Dmitry Kurochkin
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2011-12-11 22:39 UTC (permalink / raw)
  To: Jani Nikula, notmuch

On Mon, 12 Dec 2011 00:19:36 +0200, Jani Nikula <jani@nikula.org> wrote:
> 
> Hi Dmitry -
> 
> On Mon, 12 Dec 2011 02:00:45 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Sun, 11 Dec 2011 23:48:20 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > Let notmuch-poll-script be a function as well as a string. Make default
> > > value nil instead of an empty string, but allow "" for backwards
> > > compatibility. Add a notmuch poll function to call "notmuch new" using the
> > > configured notmuch-command.
> > > 
> > > This allows taking better advantage of the "notmuch new" hooks from emacs
> > > without intermediate scripts.
> > > 
> > 
> > I was just thinking about working on this myself :)
> 
> :)
> 
> > I think a better solution would be to allow running a command with
> > arguments.  Creating a elisp function just to run a command with some
> > parameters feels wrong.  This way we would have to add another function
> > each time we want to add another argument.
> 
> One thing to take into account is running "notmuch new" using
> notmuch-command, and make that happen with one click in custom.
> 

Indeed.  One click in custom should be easy, but using notmuch-command
may not be so.  So now I like your solution :) Though others may think
of something better.

The only comment I have:

+		 (function :tag "Custom function"
+			   :value notmuch-poll-script-notmuch-new)

Should we set :value to notmuch-poll-script-notmuch-new here?  There is
another option for "notmuch new", so perhaps there should be no value
for custom function as for custom script?

Regards,
  Dmitry

> > Function support for notmuch-poll-script seems like a useful feature on
> > it's own.
> 
> I just did this quickly to scratch my own itches, so to speak. My elisp
> is not that great, so I really wouldn't mind if you wanted to continue
> from here and make it your own. It'll be a while before I have the time
> to (figure out how to) do this properly anyway. But up to you, really.
> 
> BR,
> Jani.
> 
> 
> > 
> > Regards,
> >   Dmitry
> > 
> > > Signed-off-by: Jani Nikula <jani@nikula.org>
> > > ---
> > >  emacs/notmuch.el |   44 ++++++++++++++++++++++++++++++++------------
> > >  1 files changed, 32 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > > index 8936149..6c7e800 100644
> > > --- a/emacs/notmuch.el
> > > +++ b/emacs/notmuch.el
> > > @@ -965,28 +965,48 @@ same relative position within the new buffer."
> > >      (notmuch-search query oldest-first target-thread target-line continuation)
> > >      (goto-char (point-min))))
> > >  
> > > -(defcustom notmuch-poll-script ""
> > > -  "An external script to incorporate new mail into the notmuch database.
> > > +(defcustom notmuch-poll-script nil
> > > +  "A script or a function to incorporate new mail into the notmuch database.
> > >  
> > > -If this variable is non empty, then it should name a script to be
> > > -invoked by `notmuch-search-poll-and-refresh-view' and
> > > -`notmuch-hello-poll-and-update' (each have a default keybinding
> > > -of 'G'). The script could do any of the following depending on
> > > +This variable can be set to a function or the name of an external
> > > +script to be invoked by `notmuch-search-poll-and-refresh-view'
> > > +and `notmuch-hello-poll-and-update' (each have a default
> > > +keybinding of 'G'). Set to nil to do nothing.
> > > +
> > > +The function or script could do any of the following depending on
> > >  the user's needs:
> > >  
> > >  1. Invoke a program to transfer mail to the local mail store
> > >  2. Invoke \"notmuch new\" to incorporate the new mail
> > > -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> > > -  :type 'string
> > > +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> > > +
> > > +You can also choose to use \"notmuch new\" through the provided
> > > +`notmuch-poll-script-notmuch-new' function, and have the
> > > +\"notmuch new\" hooks perform the actions above."
> > > +  :type '(choice (const :tag "Not set" nil)
> > > +		 (const :tag "Notmuch new" notmuch-poll-script-notmuch-new)
> > > +		 (function :tag "Custom function"
> > > +			   :value notmuch-poll-script-notmuch-new)
> > > +		 (string :tag "Custom script"))
> > >    :group 'notmuch)
> > >  
> > > +(defun notmuch-poll-script-notmuch-new ()
> > > +  "Run \"notmuch new\"."
> > > +  (call-process notmuch-command nil nil nil "new"))
> > > +
> > >  (defun notmuch-poll ()
> > > -  "Run external script to import mail.
> > > +  "Run external script or call a function to import mail.
> > >  
> > > -Invokes `notmuch-poll-script' if it is not set to an empty string."
> > > +Invokes `notmuch-poll-script', which can be a function or the
> > > +name of an external script. Does nothing if `notmuch-poll-script'
> > > +is nil or an empty string."
> > >    (interactive)
> > > -  (if (not (string= notmuch-poll-script ""))
> > > -      (call-process notmuch-poll-script nil nil)))
> > > +  (cond
> > > +   ((stringp notmuch-poll-script)
> > > +    (if (not (string= notmuch-poll-script "")) ;; for backwards compatibility
> > > +	(call-process notmuch-poll-script nil nil)))
> > > +   ((functionp notmuch-poll-script)
> > > +    (funcall notmuch-poll-script))))
> > >  
> > >  (defun notmuch-search-poll-and-refresh-view ()
> > >    "Invoke `notmuch-poll' to import mail, then refresh the current view."
> > > -- 
> > > 1.7.5.4
> > > 
> > > _______________________________________________
> > > notmuch mailing list
> > > notmuch@notmuchmail.org
> > > http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 22:00 ` Dmitry Kurochkin
  2011-12-11 22:19   ` Jani Nikula
@ 2011-12-11 22:58   ` Austin Clements
  2011-12-11 23:10     ` Jani Nikula
  1 sibling, 1 reply; 54+ messages in thread
From: Austin Clements @ 2011-12-11 22:58 UTC (permalink / raw)
  To: Jani Nikula, Dmitry Kurochkin; +Cc: notmuch

Quoth Dmitry Kurochkin on Dec 12 at  2:00 am:
> Hi Jani.
> 
> On Sun, 11 Dec 2011 23:48:20 +0200, Jani Nikula <jani@nikula.org> wrote:
> > Let notmuch-poll-script be a function as well as a string. Make default
> > value nil instead of an empty string, but allow "" for backwards
> > compatibility. Add a notmuch poll function to call "notmuch new" using the
> > configured notmuch-command.
> > 
> > This allows taking better advantage of the "notmuch new" hooks from emacs
> > without intermediate scripts.
> > 
> 
> I was just thinking about working on this myself :)
> 
> I think a better solution would be to allow running a command with
> arguments.  Creating a elisp function just to run a command with some
> parameters feels wrong.  This way we would have to add another function
> each time we want to add another argument.

This seems a little awkward to me, too, though perhaps it's the best
way.  Other approaches to consider include accepting a list for
notmuch-poll-script (e.g., ("notmuch" "new")) or leaving it as a
string but treating it as a shell command so "notmuch new" would Just
Work.  Personally, I think the latter is the most intuitive, but it
would be worth looking at how other customizable external commands are
done in Emacs.

A function seems powerful, but also like overkill.  Can you give a use
case for a function that wouldn't be more easily solved by one of the
above approaches?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 22:58   ` Austin Clements
@ 2011-12-11 23:10     ` Jani Nikula
  2011-12-12  0:31       ` Austin Clements
  0 siblings, 1 reply; 54+ messages in thread
From: Jani Nikula @ 2011-12-11 23:10 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

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

On Dec 12, 2011 12:56 AM, "Austin Clements" <amdragon@mit.edu> wrote:
>
> Quoth Dmitry Kurochkin on Dec 12 at  2:00 am:
> > Hi Jani.
> >
> > On Sun, 11 Dec 2011 23:48:20 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > Let notmuch-poll-script be a function as well as a string. Make
default
> > > value nil instead of an empty string, but allow "" for backwards
> > > compatibility. Add a notmuch poll function to call "notmuch new"
using the
> > > configured notmuch-command.
> > >
> > > This allows taking better advantage of the "notmuch new" hooks from
emacs
> > > without intermediate scripts.
> > >
> >
> > I was just thinking about working on this myself :)
> >
> > I think a better solution would be to allow running a command with
> > arguments.  Creating a elisp function just to run a command with some
> > parameters feels wrong.  This way we would have to add another function
> > each time we want to add another argument.
>
> This seems a little awkward to me, too, though perhaps it's the best
> way.  Other approaches to consider include accepting a list for
> notmuch-poll-script (e.g., ("notmuch" "new")) or leaving it as a
> string but treating it as a shell command so "notmuch new" would Just
> Work.  Personally, I think the latter is the most intuitive, but it
> would be worth looking at how other customizable external commands are
> done in Emacs.
>
> A function seems powerful, but also like overkill.  Can you give a use
> case for a function that wouldn't be more easily solved by one of the
> above approaches?

The only reason I had for using a function was running notmuch using
notmuch-command. Any ideas how to do that with the Just Works approach?

J.

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 23:10     ` Jani Nikula
@ 2011-12-12  0:31       ` Austin Clements
  2011-12-12  0:39         ` Dmitry Kurochkin
  2011-12-12 10:15         ` Tomi Ollila
  0 siblings, 2 replies; 54+ messages in thread
From: Austin Clements @ 2011-12-12  0:31 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch

Quoth Jani Nikula on Dec 12 at  1:10 am:
>    On Dec 12, 2011 12:56 AM, "Austin Clements" <[1]amdragon@mit.edu> wrote:
>    >
>    > Quoth Dmitry Kurochkin on Dec 12 at  2:00 am:
>    > > Hi Jani.
>    > >
>    > > On Sun, 11 Dec 2011 23:48:20 +0200, Jani Nikula <[2]jani@nikula.org>
>    wrote:
>    > > > Let notmuch-poll-script be a function as well as a string. Make
>    default
>    > > > value nil instead of an empty string, but allow "" for backwards
>    > > > compatibility. Add a notmuch poll function to call "notmuch new"
>    using the
>    > > > configured notmuch-command.
>    > > >
>    > > > This allows taking better advantage of the "notmuch new" hooks from
>    emacs
>    > > > without intermediate scripts.
>    > > >
>    > >
>    > > I was just thinking about working on this myself :)
>    > >
>    > > I think a better solution would be to allow running a command with
>    > > arguments.  Creating a elisp function just to run a command with some
>    > > parameters feels wrong.  This way we would have to add another
>    function
>    > > each time we want to add another argument.
>    >
>    > This seems a little awkward to me, too, though perhaps it's the best
>    > way.  Other approaches to consider include accepting a list for
>    > notmuch-poll-script (e.g., ("notmuch" "new")) or leaving it as a
>    > string but treating it as a shell command so "notmuch new" would Just
>    > Work.  Personally, I think the latter is the most intuitive, but it
>    > would be worth looking at how other customizable external commands are
>    > done in Emacs.
>    >
>    > A function seems powerful, but also like overkill.  Can you give a use
>    > case for a function that wouldn't be more easily solved by one of the
>    > above approaches?
> 
>    The only reason I had for using a function was running notmuch using
>    notmuch-command. Any ideas how to do that with the Just Works approach?

Oh, I see.  I'd missed that.

So here's another idea, prefaced with a rant.

It's bothered me for a long time that notmuch-emacs didn't just know
by default how to check for new mail.  What MUA doesn't know how to
check for new mail?  Why does a new user of notmuch have to tell it
how to check for new mail?  Of course, this *had* to be configured
before because everyone had their own way of checking for new mail.
Hooks eliminate this unnecessary flexibility and make "notmuch new"
the one true way to check for new mail---as it ought to be---and in
turn make the notmuch-poll-script variable obsolete.

So, what about changing the default "" setting of notmuch-poll-script
from meaning "do nothing and be useless" to meaning "run notmuch new
(using notmuch-command)"?  It will then automatically do the right
thing for new users, while still being backward-compatible and
allowing an escape hatch for bizarre situations.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12  0:31       ` Austin Clements
@ 2011-12-12  0:39         ` Dmitry Kurochkin
  2011-12-12 10:15         ` Tomi Ollila
  1 sibling, 0 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2011-12-12  0:39 UTC (permalink / raw)
  To: Austin Clements, Jani Nikula; +Cc: notmuch

On Sun, 11 Dec 2011 19:31:03 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Jani Nikula on Dec 12 at  1:10 am:
> >    On Dec 12, 2011 12:56 AM, "Austin Clements" <[1]amdragon@mit.edu> wrote:
> >    >
> >    > Quoth Dmitry Kurochkin on Dec 12 at  2:00 am:
> >    > > Hi Jani.
> >    > >
> >    > > On Sun, 11 Dec 2011 23:48:20 +0200, Jani Nikula <[2]jani@nikula.org>
> >    wrote:
> >    > > > Let notmuch-poll-script be a function as well as a string. Make
> >    default
> >    > > > value nil instead of an empty string, but allow "" for backwards
> >    > > > compatibility. Add a notmuch poll function to call "notmuch new"
> >    using the
> >    > > > configured notmuch-command.
> >    > > >
> >    > > > This allows taking better advantage of the "notmuch new" hooks from
> >    emacs
> >    > > > without intermediate scripts.
> >    > > >
> >    > >
> >    > > I was just thinking about working on this myself :)
> >    > >
> >    > > I think a better solution would be to allow running a command with
> >    > > arguments.  Creating a elisp function just to run a command with some
> >    > > parameters feels wrong.  This way we would have to add another
> >    function
> >    > > each time we want to add another argument.
> >    >
> >    > This seems a little awkward to me, too, though perhaps it's the best
> >    > way.  Other approaches to consider include accepting a list for
> >    > notmuch-poll-script (e.g., ("notmuch" "new")) or leaving it as a
> >    > string but treating it as a shell command so "notmuch new" would Just
> >    > Work.  Personally, I think the latter is the most intuitive, but it
> >    > would be worth looking at how other customizable external commands are
> >    > done in Emacs.
> >    >
> >    > A function seems powerful, but also like overkill.  Can you give a use
> >    > case for a function that wouldn't be more easily solved by one of the
> >    > above approaches?
> > 
> >    The only reason I had for using a function was running notmuch using
> >    notmuch-command. Any ideas how to do that with the Just Works approach?
> 
> Oh, I see.  I'd missed that.
> 
> So here's another idea, prefaced with a rant.
> 
> It's bothered me for a long time that notmuch-emacs didn't just know
> by default how to check for new mail.  What MUA doesn't know how to
> check for new mail?  Why does a new user of notmuch have to tell it
> how to check for new mail?  Of course, this *had* to be configured
> before because everyone had their own way of checking for new mail.
> Hooks eliminate this unnecessary flexibility and make "notmuch new"
> the one true way to check for new mail---as it ought to be---and in
> turn make the notmuch-poll-script variable obsolete.
> 
> So, what about changing the default "" setting of notmuch-poll-script
> from meaning "do nothing and be useless" to meaning "run notmuch new
> (using notmuch-command)"?  It will then automatically do the right
> thing for new users, while still being backward-compatible and
> allowing an escape hatch for bizarre situations.

Fine with me.  AFAIK no one has asked for using custom functions for
notmuch-poll-script, so adding a sane default may be the simplest and
the best option.

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12  0:31       ` Austin Clements
  2011-12-12  0:39         ` Dmitry Kurochkin
@ 2011-12-12 10:15         ` Tomi Ollila
  2011-12-12 10:21           ` Dmitry Kurochkin
  1 sibling, 1 reply; 54+ messages in thread
From: Tomi Ollila @ 2011-12-12 10:15 UTC (permalink / raw)
  To: Austin Clements, Jani Nikula; +Cc: notmuch

On Sun, 11 Dec 2011 19:31:03 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> 
> So here's another idea, prefaced with a rant.
> 
> It's bothered me for a long time that notmuch-emacs didn't just know
> by default how to check for new mail.  What MUA doesn't know how to
> check for new mail?  Why does a new user of notmuch have to tell it
> how to check for new mail?  Of course, this *had* to be configured
> before because everyone had their own way of checking for new mail.
> Hooks eliminate this unnecessary flexibility and make "notmuch new"
> the one true way to check for new mail---as it ought to be---and in
> turn make the notmuch-poll-script variable obsolete.
> 
> So, what about changing the default "" setting of notmuch-poll-script
> from meaning "do nothing and be useless" to meaning "run notmuch new
> (using notmuch-command)"?  It will then automatically do the right
> thing for new users, while still being backward-compatible and
> allowing an escape hatch for bizarre situations.

+1

So, it could work like this:

(defun notmuch-poll ()
  "FIX DOCSTRING"
  (interactive)
  (if (stringp notmuch notmuch-poll-script)
      (if (string= notmuch-poll-script "")
        (call-process notmuch-command nil nil nil "new")
        (call-process notmuch-poll-script))))

I.e. in case notmuch-poll-script == nil, (or not string)
do nothing. In case notmuch-poll-script == "" execute notmuch new
and if notmuch-poll-script is string with content execute that.

users who want other functionality can reimplement notmuch-poll function
after notmuch has been loaded (and manage things themselves when internal
implementation changes ;().


Tomi

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 10:15         ` Tomi Ollila
@ 2011-12-12 10:21           ` Dmitry Kurochkin
  2011-12-12 11:38             ` Tomi Ollila
  2011-12-12 15:42             ` Austin Clements
  0 siblings, 2 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2011-12-12 10:21 UTC (permalink / raw)
  To: Tomi Ollila, Austin Clements, Jani Nikula; +Cc: notmuch

On Mon, 12 Dec 2011 12:15:44 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, 11 Dec 2011 19:31:03 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > 
> > So here's another idea, prefaced with a rant.
> > 
> > It's bothered me for a long time that notmuch-emacs didn't just know
> > by default how to check for new mail.  What MUA doesn't know how to
> > check for new mail?  Why does a new user of notmuch have to tell it
> > how to check for new mail?  Of course, this *had* to be configured
> > before because everyone had their own way of checking for new mail.
> > Hooks eliminate this unnecessary flexibility and make "notmuch new"
> > the one true way to check for new mail---as it ought to be---and in
> > turn make the notmuch-poll-script variable obsolete.
> > 
> > So, what about changing the default "" setting of notmuch-poll-script
> > from meaning "do nothing and be useless" to meaning "run notmuch new
> > (using notmuch-command)"?  It will then automatically do the right
> > thing for new users, while still being backward-compatible and
> > allowing an escape hatch for bizarre situations.
> 
> +1
> 
> So, it could work like this:
> 
> (defun notmuch-poll ()
>   "FIX DOCSTRING"
>   (interactive)
>   (if (stringp notmuch notmuch-poll-script)
>       (if (string= notmuch-poll-script "")
>         (call-process notmuch-command nil nil nil "new")
>         (call-process notmuch-poll-script))))
> 
> I.e. in case notmuch-poll-script == nil, (or not string)
> do nothing. In case notmuch-poll-script == "" execute notmuch new
> and if notmuch-poll-script is string with content execute that.
> 

I think the following scheme would be slightly better and more
consistent:

* nil - run "notmuch new", the new default
* "" - do nothing
* "script" - run script

Regards,
  Dmitry

> users who want other functionality can reimplement notmuch-poll function
> after notmuch has been loaded (and manage things themselves when internal
> implementation changes ;().
> 
> 
> Tomi
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 10:21           ` Dmitry Kurochkin
@ 2011-12-12 11:38             ` Tomi Ollila
  2011-12-12 15:42             ` Austin Clements
  1 sibling, 0 replies; 54+ messages in thread
From: Tomi Ollila @ 2011-12-12 11:38 UTC (permalink / raw)
  To: Dmitry Kurochkin, Austin Clements, Jani Nikula; +Cc: notmuch

On Mon, 12 Dec 2011 14:21:53 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Mon, 12 Dec 2011 12:15:44 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > On Sun, 11 Dec 2011 19:31:03 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > 
> > > So here's another idea, prefaced with a rant.
> > > 
> > > It's bothered me for a long time that notmuch-emacs didn't just know
> > > by default how to check for new mail.  What MUA doesn't know how to
> > > check for new mail?  Why does a new user of notmuch have to tell it
> > > how to check for new mail?  Of course, this *had* to be configured
> > > before because everyone had their own way of checking for new mail.
> > > Hooks eliminate this unnecessary flexibility and make "notmuch new"
> > > the one true way to check for new mail---as it ought to be---and in
> > > turn make the notmuch-poll-script variable obsolete.
> > > 
> > > So, what about changing the default "" setting of notmuch-poll-script
> > > from meaning "do nothing and be useless" to meaning "run notmuch new
> > > (using notmuch-command)"?  It will then automatically do the right
> > > thing for new users, while still being backward-compatible and
> > > allowing an escape hatch for bizarre situations.
> > 
> > +1
> > 
> > So, it could work like this:
> > 
> > (defun notmuch-poll ()
> >   "FIX DOCSTRING"
> >   (interactive)
> >   (if (stringp notmuch notmuch-poll-script)
> >       (if (string= notmuch-poll-script "")
> >         (call-process notmuch-command nil nil nil "new")
> >         (call-process notmuch-poll-script))))
> > 
> > I.e. in case notmuch-poll-script == nil, (or not string)
> > do nothing. In case notmuch-poll-script == "" execute notmuch new
> > and if notmuch-poll-script is string with content execute that.
> > 
> 
> I think the following scheme would be slightly better and more
> consistent:
> 
> * nil - run "notmuch new", the new default
> * "" - do nothing
> * "script" - run script

You're right. This us MUCH better (something irked me when writing
my suggestion but that was for ultimate backward compability); The
default value for notmuch-poll-script could be changed to nil.

> 
> Regards,
>   Dmitry
> 
> > users who want other functionality can reimplement notmuch-poll function
> > after notmuch has been loaded (and manage things themselves when internal
> > implementation changes ;().
> > 
> > 
> > Tomi

Tomi

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 10:21           ` Dmitry Kurochkin
  2011-12-12 11:38             ` Tomi Ollila
@ 2011-12-12 15:42             ` Austin Clements
  1 sibling, 0 replies; 54+ messages in thread
From: Austin Clements @ 2011-12-12 15:42 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: Tomi Ollila, notmuch

Quoth Dmitry Kurochkin on Dec 12 at  2:21 pm:
> On Mon, 12 Dec 2011 12:15:44 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > On Sun, 11 Dec 2011 19:31:03 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > 
> > > So here's another idea, prefaced with a rant.
> > > 
> > > It's bothered me for a long time that notmuch-emacs didn't just know
> > > by default how to check for new mail.  What MUA doesn't know how to
> > > check for new mail?  Why does a new user of notmuch have to tell it
> > > how to check for new mail?  Of course, this *had* to be configured
> > > before because everyone had their own way of checking for new mail.
> > > Hooks eliminate this unnecessary flexibility and make "notmuch new"
> > > the one true way to check for new mail---as it ought to be---and in
> > > turn make the notmuch-poll-script variable obsolete.
> > > 
> > > So, what about changing the default "" setting of notmuch-poll-script
> > > from meaning "do nothing and be useless" to meaning "run notmuch new
> > > (using notmuch-command)"?  It will then automatically do the right
> > > thing for new users, while still being backward-compatible and
> > > allowing an escape hatch for bizarre situations.
> > 
> > +1
> > 
> > So, it could work like this:
> > 
> > (defun notmuch-poll ()
> >   "FIX DOCSTRING"
> >   (interactive)
> >   (if (stringp notmuch notmuch-poll-script)
> >       (if (string= notmuch-poll-script "")
> >         (call-process notmuch-command nil nil nil "new")
> >         (call-process notmuch-poll-script))))
> > 
> > I.e. in case notmuch-poll-script == nil, (or not string)
> > do nothing. In case notmuch-poll-script == "" execute notmuch new
> > and if notmuch-poll-script is string with content execute that.
> > 
> 
> I think the following scheme would be slightly better and more
> consistent:
> 
> * nil - run "notmuch new", the new default
> * "" - do nothing
> * "script" - run script
> 
> Regards,
>   Dmitry

I like it!

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 21:48 [PATCH] emacs: support "notmuch new" as a notmuch-poll-script Jani Nikula
  2011-12-11 22:00 ` Dmitry Kurochkin
@ 2011-12-12 19:57 ` Jani Nikula
  2011-12-12 20:12   ` Dmitry Kurochkin
  2011-12-12 20:24   ` Austin Clements
  2011-12-12 20:50 ` [PATCH v3] " Jani Nikula
  2 siblings, 2 replies; 54+ messages in thread
From: Jani Nikula @ 2011-12-12 19:57 UTC (permalink / raw)
  To: notmuch

Support nil value for notmuch-poll-script to run "notmuch new" instead of
an external script, and make this the new default. "notmuch new" is run
using the configured notmuch-command.

This allows taking better advantage of the "notmuch new" hooks from emacs
without intermediate scripts.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 emacs/notmuch.el |   34 ++++++++++++++++++++++++----------
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 8936149..5a8ab9d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -965,28 +965,42 @@ same relative position within the new buffer."
     (notmuch-search query oldest-first target-thread target-line continuation)
     (goto-char (point-min))))
 
-(defcustom notmuch-poll-script ""
+(defcustom notmuch-poll-script nil
   "An external script to incorporate new mail into the notmuch database.
 
-If this variable is non empty, then it should name a script to be
-invoked by `notmuch-search-poll-and-refresh-view' and
+This variable controls the action invoked by
+`notmuch-search-poll-and-refresh-view' and
 `notmuch-hello-poll-and-update' (each have a default keybinding
-of 'G'). The script could do any of the following depending on
+of 'G') to incorporate new mail into the notmuch database.
+
+If this variable is non empty, then it should name an external
+script to be run. If set to an empty string, no action is
+invoked. Finally, if set to nil (the default), \"notmuch new\" is
+run using the command specified by `notmuch-command'.
+
+The external script could do any of the following depending on
 the user's needs:
 
 1. Invoke a program to transfer mail to the local mail store
 2. Invoke \"notmuch new\" to incorporate the new mail
-3. Invoke one or more \"notmuch tag\" commands to classify the mail"
-  :type 'string
+3. Invoke one or more \"notmuch tag\" commands to classify the mail
+
+Note that the same can be achieved through \"notmuch new\" hooks."
+  :type '(choice (const :tag "Notmuch new" nil)
+		 (const :tag "Disabled" "")
+		 (string :tag "Custom script"))
   :group 'notmuch)
 
 (defun notmuch-poll ()
-  "Run external script to import mail.
+  "Run \"notmuch new\" or an external script to import mail.
 
-Invokes `notmuch-poll-script' if it is not set to an empty string."
+Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
+depending on the value of `notmuch-poll-script'."
   (interactive)
-  (if (not (string= notmuch-poll-script ""))
-      (call-process notmuch-poll-script nil nil)))
+  (if (stringp notmuch-poll-script)
+      (if (not (string= notmuch-poll-script ""))
+	  (call-process notmuch-poll-script nil nil))
+    (call-process notmuch-command nil nil nil "new")))
 
 (defun notmuch-search-poll-and-refresh-view ()
   "Invoke `notmuch-poll' to import mail, then refresh the current view."
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v2] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 19:57 ` [PATCH v2] " Jani Nikula
@ 2011-12-12 20:12   ` Dmitry Kurochkin
  2011-12-12 20:24   ` Austin Clements
  1 sibling, 0 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2011-12-12 20:12 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Hi Jani.

On Mon, 12 Dec 2011 21:57:28 +0200, Jani Nikula <jani@nikula.org> wrote:
> Support nil value for notmuch-poll-script to run "notmuch new" instead of
> an external script, and make this the new default. "notmuch new" is run
> using the configured notmuch-command.
> 
> This allows taking better advantage of the "notmuch new" hooks from emacs
> without intermediate scripts.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>

Looks good to me.  Few comments regarding documentation below.

> ---
>  emacs/notmuch.el |   34 ++++++++++++++++++++++++----------
>  1 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 8936149..5a8ab9d 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -965,28 +965,42 @@ same relative position within the new buffer."
>      (notmuch-search query oldest-first target-thread target-line continuation)
>      (goto-char (point-min))))
>  
> -(defcustom notmuch-poll-script ""
> +(defcustom notmuch-poll-script nil
>    "An external script to incorporate new mail into the notmuch database.
>  
> -If this variable is non empty, then it should name a script to be
> -invoked by `notmuch-search-poll-and-refresh-view' and
> +This variable controls the action invoked by
> +`notmuch-search-poll-and-refresh-view' and
>  `notmuch-hello-poll-and-update' (each have a default keybinding
> -of 'G'). The script could do any of the following depending on
> +of 'G') to incorporate new mail into the notmuch database.
> +
> +If this variable is non empty, then it should name an external

Please s/non empty/non-empty string/ to make it more clear.

> +script to be run. If set to an empty string, no action is
> +invoked. Finally, if set to nil (the default), \"notmuch new\" is
> +run using the command specified by `notmuch-command'.
> +
> +The external script could do any of the following depending on
>  the user's needs:
>  
>  1. Invoke a program to transfer mail to the local mail store
>  2. Invoke \"notmuch new\" to incorporate the new mail
> -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> -  :type 'string
> +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> +
> +Note that the same can be achieved through \"notmuch new\" hooks."

s/though/using/?

Would be nice to mention that using hooks is the recommended way for
this.

> +  :type '(choice (const :tag "Notmuch new" nil)

Perhaps s/Notmuch/notmuch/ because we are talking about a command here?

Regards,
  Dmitry

> +		 (const :tag "Disabled" "")
> +		 (string :tag "Custom script"))
>    :group 'notmuch)
>  
>  (defun notmuch-poll ()
> -  "Run external script to import mail.
> +  "Run \"notmuch new\" or an external script to import mail.
>  
> -Invokes `notmuch-poll-script' if it is not set to an empty string."
> +Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
> +depending on the value of `notmuch-poll-script'."
>    (interactive)
> -  (if (not (string= notmuch-poll-script ""))
> -      (call-process notmuch-poll-script nil nil)))
> +  (if (stringp notmuch-poll-script)
> +      (if (not (string= notmuch-poll-script ""))
> +	  (call-process notmuch-poll-script nil nil))
> +    (call-process notmuch-command nil nil nil "new")))
>  
>  (defun notmuch-search-poll-and-refresh-view ()
>    "Invoke `notmuch-poll' to import mail, then refresh the current view."
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 19:57 ` [PATCH v2] " Jani Nikula
  2011-12-12 20:12   ` Dmitry Kurochkin
@ 2011-12-12 20:24   ` Austin Clements
  1 sibling, 0 replies; 54+ messages in thread
From: Austin Clements @ 2011-12-12 20:24 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch

Looks good to me.

One documentation nitpick below.

Quoth Jani Nikula on Dec 12 at  9:57 pm:
> Support nil value for notmuch-poll-script to run "notmuch new" instead of
> an external script, and make this the new default. "notmuch new" is run
> using the configured notmuch-command.
> 
> This allows taking better advantage of the "notmuch new" hooks from emacs
> without intermediate scripts.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> ---
>  emacs/notmuch.el |   34 ++++++++++++++++++++++++----------
>  1 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 8936149..5a8ab9d 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -965,28 +965,42 @@ same relative position within the new buffer."
>      (notmuch-search query oldest-first target-thread target-line continuation)
>      (goto-char (point-min))))
>  
> -(defcustom notmuch-poll-script ""
> +(defcustom notmuch-poll-script nil
>    "An external script to incorporate new mail into the notmuch database.
>  
> -If this variable is non empty, then it should name a script to be
> -invoked by `notmuch-search-poll-and-refresh-view' and
> +This variable controls the action invoked by
> +`notmuch-search-poll-and-refresh-view' and
>  `notmuch-hello-poll-and-update' (each have a default keybinding
> -of 'G'). The script could do any of the following depending on
> +of 'G') to incorporate new mail into the notmuch database.
> +
> +If this variable is non empty, then it should name an external
> +script to be run. If set to an empty string, no action is
> +invoked. Finally, if set to nil (the default), \"notmuch new\" is
> +run using the command specified by `notmuch-command'.

The default should probably be given first.  Also, "non empty" is a
bit confusing, since nothing has been said about it being a string at
that point.  So, perhaps something like

If set to nil (the default), new mail is processed by invoking
\"notmuch new\".  Otherwise, this should be set to a string that gives
the name of an external script that processes new mail.  If set to the
empty string, no command will be run.

> +
> +The external script could do any of the following depending on
>  the user's needs:
>  
>  1. Invoke a program to transfer mail to the local mail store
>  2. Invoke \"notmuch new\" to incorporate the new mail
> -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> -  :type 'string
> +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> +
> +Note that the same can be achieved through \"notmuch new\" hooks."
> +  :type '(choice (const :tag "Notmuch new" nil)
> +		 (const :tag "Disabled" "")
> +		 (string :tag "Custom script"))
>    :group 'notmuch)
>  
>  (defun notmuch-poll ()
> -  "Run external script to import mail.
> +  "Run \"notmuch new\" or an external script to import mail.
>  
> -Invokes `notmuch-poll-script' if it is not set to an empty string."
> +Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
> +depending on the value of `notmuch-poll-script'."
>    (interactive)
> -  (if (not (string= notmuch-poll-script ""))
> -      (call-process notmuch-poll-script nil nil)))
> +  (if (stringp notmuch-poll-script)
> +      (if (not (string= notmuch-poll-script ""))
> +	  (call-process notmuch-poll-script nil nil))
> +    (call-process notmuch-command nil nil nil "new")))
>  
>  (defun notmuch-search-poll-and-refresh-view ()
>    "Invoke `notmuch-poll' to import mail, then refresh the current view."

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-11 21:48 [PATCH] emacs: support "notmuch new" as a notmuch-poll-script Jani Nikula
  2011-12-11 22:00 ` Dmitry Kurochkin
  2011-12-12 19:57 ` [PATCH v2] " Jani Nikula
@ 2011-12-12 20:50 ` Jani Nikula
  2011-12-12 20:53   ` Dmitry Kurochkin
                     ` (3 more replies)
  2 siblings, 4 replies; 54+ messages in thread
From: Jani Nikula @ 2011-12-12 20:50 UTC (permalink / raw)
  To: notmuch

Support nil value for notmuch-poll-script to run "notmuch new" instead of
an external script, and make this the new default. "notmuch new" is run
using the configured notmuch-command.

This allows taking better advantage of the "notmuch new" hooks from emacs
without intermediate scripts.

Signed-off-by: Jani Nikula <jani@nikula.org>

---

v3: only documentation changes suggested by Austin and Dmitry.
---
 emacs/notmuch.el |   35 +++++++++++++++++++++++++----------
 1 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 8936149..675a110 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -965,28 +965,43 @@ same relative position within the new buffer."
     (notmuch-search query oldest-first target-thread target-line continuation)
     (goto-char (point-min))))
 
-(defcustom notmuch-poll-script ""
+(defcustom notmuch-poll-script nil
   "An external script to incorporate new mail into the notmuch database.
 
-If this variable is non empty, then it should name a script to be
-invoked by `notmuch-search-poll-and-refresh-view' and
+This variable controls the action invoked by
+`notmuch-search-poll-and-refresh-view' and
 `notmuch-hello-poll-and-update' (each have a default keybinding
-of 'G'). The script could do any of the following depending on
+of 'G') to incorporate new mail into the notmuch database.
+
+If set to nil (the default), new mail is processed by invoking
+\"notmuch new\". Otherwise, this should be set to a string that
+gives the name of an external script that processes new mail. If
+set to the empty string, no command will be run.
+
+The external script could do any of the following depending on
 the user's needs:
 
 1. Invoke a program to transfer mail to the local mail store
 2. Invoke \"notmuch new\" to incorporate the new mail
-3. Invoke one or more \"notmuch tag\" commands to classify the mail"
-  :type 'string
+3. Invoke one or more \"notmuch tag\" commands to classify the mail
+
+Note that the recommended way of achieving the same is using
+\"notmuch new\" hooks."
+  :type '(choice (const :tag "notmuch new" nil)
+		 (const :tag "Disabled" "")
+		 (string :tag "Custom script"))
   :group 'notmuch)
 
 (defun notmuch-poll ()
-  "Run external script to import mail.
+  "Run \"notmuch new\" or an external script to import mail.
 
-Invokes `notmuch-poll-script' if it is not set to an empty string."
+Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
+depending on the value of `notmuch-poll-script'."
   (interactive)
-  (if (not (string= notmuch-poll-script ""))
-      (call-process notmuch-poll-script nil nil)))
+  (if (stringp notmuch-poll-script)
+      (if (not (string= notmuch-poll-script ""))
+	  (call-process notmuch-poll-script nil nil))
+    (call-process notmuch-command nil nil nil "new")))
 
 (defun notmuch-search-poll-and-refresh-view ()
   "Invoke `notmuch-poll' to import mail, then refresh the current view."
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 20:50 ` [PATCH v3] " Jani Nikula
@ 2011-12-12 20:53   ` Dmitry Kurochkin
  2011-12-12 21:13     ` Jani Nikula
  2011-12-12 21:01   ` Austin Clements
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 54+ messages in thread
From: Dmitry Kurochkin @ 2011-12-12 20:53 UTC (permalink / raw)
  To: Jani Nikula, notmuch

On Mon, 12 Dec 2011 22:50:04 +0200, Jani Nikula <jani@nikula.org> wrote:
> Support nil value for notmuch-poll-script to run "notmuch new" instead of
> an external script, and make this the new default. "notmuch new" is run
> using the configured notmuch-command.
> 
> This allows taking better advantage of the "notmuch new" hooks from emacs
> without intermediate scripts.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> 
> ---
> 
> v3: only documentation changes suggested by Austin and Dmitry.
> ---
>  emacs/notmuch.el |   35 +++++++++++++++++++++++++----------
>  1 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 8936149..675a110 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -965,28 +965,43 @@ same relative position within the new buffer."
>      (notmuch-search query oldest-first target-thread target-line continuation)
>      (goto-char (point-min))))
>  
> -(defcustom notmuch-poll-script ""
> +(defcustom notmuch-poll-script nil
>    "An external script to incorporate new mail into the notmuch database.
>  
> -If this variable is non empty, then it should name a script to be
> -invoked by `notmuch-search-poll-and-refresh-view' and
> +This variable controls the action invoked by
> +`notmuch-search-poll-and-refresh-view' and
>  `notmuch-hello-poll-and-update' (each have a default keybinding
> -of 'G'). The script could do any of the following depending on
> +of 'G') to incorporate new mail into the notmuch database.
> +
> +If set to nil (the default), new mail is processed by invoking
> +\"notmuch new\". Otherwise, this should be set to a string that
> +gives the name of an external script that processes new mail. If
> +set to the empty string, no command will be run.

I think this should be "an empty string".  But I may be mistaking.

Regards,
  Dmitry

> +
> +The external script could do any of the following depending on
>  the user's needs:
>  
>  1. Invoke a program to transfer mail to the local mail store
>  2. Invoke \"notmuch new\" to incorporate the new mail
> -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> -  :type 'string
> +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> +
> +Note that the recommended way of achieving the same is using
> +\"notmuch new\" hooks."
> +  :type '(choice (const :tag "notmuch new" nil)
> +		 (const :tag "Disabled" "")
> +		 (string :tag "Custom script"))
>    :group 'notmuch)
>  
>  (defun notmuch-poll ()
> -  "Run external script to import mail.
> +  "Run \"notmuch new\" or an external script to import mail.
>  
> -Invokes `notmuch-poll-script' if it is not set to an empty string."
> +Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
> +depending on the value of `notmuch-poll-script'."
>    (interactive)
> -  (if (not (string= notmuch-poll-script ""))
> -      (call-process notmuch-poll-script nil nil)))
> +  (if (stringp notmuch-poll-script)
> +      (if (not (string= notmuch-poll-script ""))
> +	  (call-process notmuch-poll-script nil nil))
> +    (call-process notmuch-command nil nil nil "new")))
>  
>  (defun notmuch-search-poll-and-refresh-view ()
>    "Invoke `notmuch-poll' to import mail, then refresh the current view."
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 20:50 ` [PATCH v3] " Jani Nikula
  2011-12-12 20:53   ` Dmitry Kurochkin
@ 2011-12-12 21:01   ` Austin Clements
  2011-12-15  4:24   ` David Bremner
  2012-01-12 17:31   ` Pieter Praet
  3 siblings, 0 replies; 54+ messages in thread
From: Austin Clements @ 2011-12-12 21:01 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch

LGTM.

Quoth Jani Nikula on Dec 12 at 10:50 pm:
> Support nil value for notmuch-poll-script to run "notmuch new" instead of
> an external script, and make this the new default. "notmuch new" is run
> using the configured notmuch-command.
> 
> This allows taking better advantage of the "notmuch new" hooks from emacs
> without intermediate scripts.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> 
> ---
> 
> v3: only documentation changes suggested by Austin and Dmitry.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 20:53   ` Dmitry Kurochkin
@ 2011-12-12 21:13     ` Jani Nikula
  2011-12-12 21:24       ` Austin Clements
  0 siblings, 1 reply; 54+ messages in thread
From: Jani Nikula @ 2011-12-12 21:13 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

On Tue, 13 Dec 2011 00:53:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Mon, 12 Dec 2011 22:50:04 +0200, Jani Nikula <jani@nikula.org> wrote:
> > Support nil value for notmuch-poll-script to run "notmuch new" instead of
> > an external script, and make this the new default. "notmuch new" is run
> > using the configured notmuch-command.
> > 
> > This allows taking better advantage of the "notmuch new" hooks from emacs
> > without intermediate scripts.
> > 
> > Signed-off-by: Jani Nikula <jani@nikula.org>
> > 
> > ---
> > 
> > v3: only documentation changes suggested by Austin and Dmitry.
> > ---
> >  emacs/notmuch.el |   35 +++++++++++++++++++++++++----------
> >  1 files changed, 25 insertions(+), 10 deletions(-)
> > 
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index 8936149..675a110 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -965,28 +965,43 @@ same relative position within the new buffer."
> >      (notmuch-search query oldest-first target-thread target-line continuation)
> >      (goto-char (point-min))))
> >  
> > -(defcustom notmuch-poll-script ""
> > +(defcustom notmuch-poll-script nil
> >    "An external script to incorporate new mail into the notmuch database.
> >  
> > -If this variable is non empty, then it should name a script to be
> > -invoked by `notmuch-search-poll-and-refresh-view' and
> > +This variable controls the action invoked by
> > +`notmuch-search-poll-and-refresh-view' and
> >  `notmuch-hello-poll-and-update' (each have a default keybinding
> > -of 'G'). The script could do any of the following depending on
> > +of 'G') to incorporate new mail into the notmuch database.
> > +
> > +If set to nil (the default), new mail is processed by invoking
> > +\"notmuch new\". Otherwise, this should be set to a string that
> > +gives the name of an external script that processes new mail. If
> > +set to the empty string, no command will be run.
> 
> I think this should be "an empty string".  But I may be mistaking.

Shameless copy paste from a native speaker, who am I to argue? :)
Austin?

Jani.


> 
> Regards,
>   Dmitry
> 
> > +
> > +The external script could do any of the following depending on
> >  the user's needs:
> >  
> >  1. Invoke a program to transfer mail to the local mail store
> >  2. Invoke \"notmuch new\" to incorporate the new mail
> > -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> > -  :type 'string
> > +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> > +
> > +Note that the recommended way of achieving the same is using
> > +\"notmuch new\" hooks."
> > +  :type '(choice (const :tag "notmuch new" nil)
> > +		 (const :tag "Disabled" "")
> > +		 (string :tag "Custom script"))
> >    :group 'notmuch)
> >  
> >  (defun notmuch-poll ()
> > -  "Run external script to import mail.
> > +  "Run \"notmuch new\" or an external script to import mail.
> >  
> > -Invokes `notmuch-poll-script' if it is not set to an empty string."
> > +Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
> > +depending on the value of `notmuch-poll-script'."
> >    (interactive)
> > -  (if (not (string= notmuch-poll-script ""))
> > -      (call-process notmuch-poll-script nil nil)))
> > +  (if (stringp notmuch-poll-script)
> > +      (if (not (string= notmuch-poll-script ""))
> > +	  (call-process notmuch-poll-script nil nil))
> > +    (call-process notmuch-command nil nil nil "new")))
> >  
> >  (defun notmuch-search-poll-and-refresh-view ()
> >    "Invoke `notmuch-poll' to import mail, then refresh the current view."
> > -- 
> > 1.7.5.4
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 21:13     ` Jani Nikula
@ 2011-12-12 21:24       ` Austin Clements
  2011-12-12 21:34         ` Dmitry Kurochkin
  0 siblings, 1 reply; 54+ messages in thread
From: Austin Clements @ 2011-12-12 21:24 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch

Quoth Jani Nikula on Dec 12 at 11:13 pm:
> On Tue, 13 Dec 2011 00:53:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Mon, 12 Dec 2011 22:50:04 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > +If set to nil (the default), new mail is processed by invoking
> > > +\"notmuch new\". Otherwise, this should be set to a string that
> > > +gives the name of an external script that processes new mail. If
> > > +set to the empty string, no command will be run.
> > 
> > I think this should be "an empty string".  But I may be mistaking.
> 
> Shameless copy paste from a native speaker, who am I to argue? :)
> Austin?

Either way is grammatically correct.  Really, this is a philosophical
question.  Can two empty strings have different identities?  Or is
there only one empty string in the universe?

(eq "" "") => t
(eq "" (string)) => t
(eq "" (make-string 0 ?a)) => t
(eq "" (substring "a" 1)) => t

It would appear Elisp is squarely in the "there is one empty string"
camp, so "the empty string" would be more correct.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 21:24       ` Austin Clements
@ 2011-12-12 21:34         ` Dmitry Kurochkin
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2011-12-12 21:34 UTC (permalink / raw)
  To: Austin Clements, Jani Nikula; +Cc: notmuch

On Mon, 12 Dec 2011 16:24:51 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Jani Nikula on Dec 12 at 11:13 pm:
> > On Tue, 13 Dec 2011 00:53:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > On Mon, 12 Dec 2011 22:50:04 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > > +If set to nil (the default), new mail is processed by invoking
> > > > +\"notmuch new\". Otherwise, this should be set to a string that
> > > > +gives the name of an external script that processes new mail. If
> > > > +set to the empty string, no command will be run.
> > > 
> > > I think this should be "an empty string".  But I may be mistaking.
> > 
> > Shameless copy paste from a native speaker, who am I to argue? :)
> > Austin?
> 
> Either way is grammatically correct.  Really, this is a philosophical
> question.  Can two empty strings have different identities?  Or is
> there only one empty string in the universe?
> 
> (eq "" "") => t
> (eq "" (string)) => t
> (eq "" (make-string 0 ?a)) => t
> (eq "" (substring "a" 1)) => t
> 
> It would appear Elisp is squarely in the "there is one empty string"
> camp, so "the empty string" would be more correct.

Fine with me then :)

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 20:50 ` [PATCH v3] " Jani Nikula
  2011-12-12 20:53   ` Dmitry Kurochkin
  2011-12-12 21:01   ` Austin Clements
@ 2011-12-15  4:24   ` David Bremner
  2012-01-12 17:31   ` Pieter Praet
  3 siblings, 0 replies; 54+ messages in thread
From: David Bremner @ 2011-12-15  4:24 UTC (permalink / raw)
  To: Jani Nikula, notmuch

On Mon, 12 Dec 2011 22:50:04 +0200, Jani Nikula <jani@nikula.org> wrote:
> Support nil value for notmuch-poll-script to run "notmuch new" instead of
> an external script, and make this the new default. "notmuch new" is run
> using the configured notmuch-command.

pushed.

d

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3] emacs: support "notmuch new" as a notmuch-poll-script
  2011-12-12 20:50 ` [PATCH v3] " Jani Nikula
                     ` (2 preceding siblings ...)
  2011-12-15  4:24   ` David Bremner
@ 2012-01-12 17:31   ` Pieter Praet
  2012-01-12 17:33     ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
  3 siblings, 1 reply; 54+ messages in thread
From: Pieter Praet @ 2012-01-12 17:31 UTC (permalink / raw)
  To: Jani Nikula, notmuch

On Mon, 12 Dec 2011 22:50:04 +0200, Jani Nikula <jani@nikula.org> wrote:
> Support nil value for notmuch-poll-script to run "notmuch new" instead of
> an external script, and make this the new default. "notmuch new" is run
> using the configured notmuch-command.
> 
> This allows taking better advantage of the "notmuch new" hooks from emacs
> without intermediate scripts.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> 
> ---
> 
> v3: only documentation changes suggested by Austin and Dmitry.
> ---
>  emacs/notmuch.el |   35 +++++++++++++++++++++++++----------
>  1 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 8936149..675a110 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -965,28 +965,43 @@ same relative position within the new buffer."
>      (notmuch-search query oldest-first target-thread target-line continuation)
>      (goto-char (point-min))))
>  
> -(defcustom notmuch-poll-script ""
> +(defcustom notmuch-poll-script nil
>    "An external script to incorporate new mail into the notmuch database.
>  
> -If this variable is non empty, then it should name a script to be
> -invoked by `notmuch-search-poll-and-refresh-view' and
> +This variable controls the action invoked by
> +`notmuch-search-poll-and-refresh-view' and
>  `notmuch-hello-poll-and-update' (each have a default keybinding
> -of 'G'). The script could do any of the following depending on
> +of 'G') to incorporate new mail into the notmuch database.
> +
> +If set to nil (the default), new mail is processed by invoking
> +\"notmuch new\". Otherwise, this should be set to a string that
> +gives the name of an external script that processes new mail. If
> +set to the empty string, no command will be run.
> +
> +The external script could do any of the following depending on
>  the user's needs:
>  
>  1. Invoke a program to transfer mail to the local mail store
>  2. Invoke \"notmuch new\" to incorporate the new mail
> -3. Invoke one or more \"notmuch tag\" commands to classify the mail"
> -  :type 'string
> +3. Invoke one or more \"notmuch tag\" commands to classify the mail
> +
> +Note that the recommended way of achieving the same is using
> +\"notmuch new\" hooks."
> +  :type '(choice (const :tag "notmuch new" nil)
> +		 (const :tag "Disabled" "")
> +		 (string :tag "Custom script"))
>    :group 'notmuch)
>  
>  (defun notmuch-poll ()
> -  "Run external script to import mail.
> +  "Run \"notmuch new\" or an external script to import mail.
>  
> -Invokes `notmuch-poll-script' if it is not set to an empty string."
> +Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
> +depending on the value of `notmuch-poll-script'."
>    (interactive)
> -  (if (not (string= notmuch-poll-script ""))
> -      (call-process notmuch-poll-script nil nil)))
> +  (if (stringp notmuch-poll-script)
> +      (if (not (string= notmuch-poll-script ""))
> +	  (call-process notmuch-poll-script nil nil))
> +    (call-process notmuch-command nil nil nil "new")))

Since it seems to be bikeshedding-season:

When `(if (not's don't have an `else'-part, use `unless' instead ?

Patch follows.

>  
>  (defun notmuch-search-poll-and-refresh-view ()
>    "Invoke `notmuch-poll' to import mail, then refresh the current view."
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Nicely complements the new hooks (for which thanks) !


Peace

-- 
Pieter

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-12 17:31   ` Pieter Praet
@ 2012-01-12 17:33     ` Pieter Praet
  2012-01-12 17:40       ` Dmitry Kurochkin
                         ` (3 more replies)
  0 siblings, 4 replies; 54+ messages in thread
From: Pieter Praet @ 2012-01-12 17:33 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Notmuch Mail

Less code, same results, without sacrificing readability.

---
 emacs/notmuch-address.el |    6 +++---
 emacs/notmuch-hello.el   |   20 ++++++++++----------
 emacs/notmuch-show.el    |   12 ++++++------
 emacs/notmuch.el         |    8 ++++----
 4 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 8eba7a0..d72b169 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -37,9 +37,9 @@ line."
 (defvar notmuch-address-history nil)
 
 (defun notmuch-address-message-insinuate ()
-  (if (not (memq notmuch-address-message-alist-member message-completion-alist))
-      (setq message-completion-alist
-	    (push notmuch-address-message-alist-member message-completion-alist))))
+  (unless (memq notmuch-address-message-alist-member message-completion-alist)
+    (setq message-completion-alist
+	  (push notmuch-address-message-alist-member message-completion-alist))))
 
 (defun notmuch-address-options (original)
   (process-lines notmuch-address-command original))
diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 333d4c1..dce2020 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -315,7 +315,7 @@ should be. Returns a cons cell `(tags-per-line width)'."
 
     ;; If the last line was not full (and hence did not include a
     ;; carriage return), insert one now.
-    (if (not (eq (% count tags-per-line) 0))
+    (unless (eq (% count tags-per-line) 0)
 	(widget-insert "\n"))
     found-target-pos))
 
@@ -399,7 +399,7 @@ Complete list of currently available key bindings:
 
   ; Jump through a hoop to get this value from the deprecated variable
   ; name (`notmuch-folders') or from the default value.
-  (if (not notmuch-saved-searches)
+  (unless notmuch-saved-searches
     (setq notmuch-saved-searches (notmuch-saved-searches)))
 
   (if no-display
@@ -565,18 +565,18 @@ Complete list of currently available key bindings:
 	  (widget-insert "\n\n")
 	  (let ((start (point)))
 	    (setq found-target-pos (notmuch-hello-insert-tags alltags-alist widest target))
-	    (if (not final-target-pos)
-		(setq final-target-pos found-target-pos))
+	    (unless final-target-pos
+	      (setq final-target-pos found-target-pos))
 	    (indent-rigidly start (point) notmuch-hello-indent)))
 
 	(widget-insert "\n")
 
-	(if (not notmuch-show-all-tags-list)
-	    (widget-create 'push-button
-			   :notify (lambda (widget &rest ignore)
-				     (setq notmuch-show-all-tags-list t)
-				     (notmuch-hello-update))
-			   "Show all tags")))
+	(unless notmuch-show-all-tags-list
+	  (widget-create 'push-button
+			 :notify (lambda (widget &rest ignore)
+				   (setq notmuch-show-all-tags-list t)
+				   (notmuch-hello-update))
+			 "Show all tags")))
 
       (let ((start (point)))
 	(widget-insert "\n\n")
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5502efd..d3543f0 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -659,8 +659,8 @@ current buffer, if possible."
   ;; part, so we make sure that we're down at the end.
   (goto-char (point-max))
   ;; Ensure that the part ends with a carriage return.
-  (if (not (bolp))
-      (insert "\n")))
+  (unless (bolp)
+    (insert "\n")))
 
 (defun notmuch-show-insert-body (msg body depth)
   "Insert the body BODY at depth DEPTH in the current thread."
@@ -740,8 +740,8 @@ current buffer, if possible."
     (setq body-start (point-marker))
     (notmuch-show-insert-body msg (plist-get msg :body) depth)
     ;; Ensure that the body ends with a newline.
-    (if (not (bolp))
-	(insert "\n"))
+    (unless (bolp)
+      (insert "\n"))
     (setq body-end (point-marker))
     (setq content-end (point-marker))
 
@@ -882,8 +882,8 @@ buffer."
       (run-hooks 'notmuch-show-hook))
 
     ;; Move straight to the first open message
-    (if (not (notmuch-show-message-visible-p))
-	(notmuch-show-next-open-message))
+    (unless (notmuch-show-message-visible-p)
+      (notmuch-show-next-open-message))
 
     ;; Set the header line to the subject of the first open message.
     (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 1e61775..ba84494 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -636,8 +636,8 @@ This function advances the next thread when finished."
 			(if notmuch-search-process-filter-data
 			    (insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))
 			(insert "End of search results.")
-			(if (not (= exit-status 0))
-			    (insert (format " (process returned %d)" exit-status)))
+			(unless (= exit-status 0)
+			  (insert (format " (process returned %d)" exit-status)))
 			(insert "\n")
 			(if (and atbob
 				 (not (string= notmuch-search-target-thread "found")))
@@ -1006,8 +1006,8 @@ Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
 depending on the value of `notmuch-poll-script'."
   (interactive)
   (if (stringp notmuch-poll-script)
-      (if (not (string= notmuch-poll-script ""))
-	  (call-process notmuch-poll-script nil nil))
+      (unless (string= notmuch-poll-script "")
+	(call-process notmuch-poll-script nil nil))
     (call-process notmuch-command nil nil nil "new")))
 
 (defun notmuch-search-poll-and-refresh-view ()
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-12 17:33     ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
@ 2012-01-12 17:40       ` Dmitry Kurochkin
  2012-01-12 22:10       ` Xavier Maillard
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2012-01-12 17:40 UTC (permalink / raw)
  To: Pieter Praet, Jani Nikula; +Cc: Notmuch Mail

On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.
> 

+1

Regards,
  Dmitry

> ---
>  emacs/notmuch-address.el |    6 +++---
>  emacs/notmuch-hello.el   |   20 ++++++++++----------
>  emacs/notmuch-show.el    |   12 ++++++------
>  emacs/notmuch.el         |    8 ++++----
>  4 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index 8eba7a0..d72b169 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -37,9 +37,9 @@ line."
>  (defvar notmuch-address-history nil)
>  
>  (defun notmuch-address-message-insinuate ()
> -  (if (not (memq notmuch-address-message-alist-member message-completion-alist))
> -      (setq message-completion-alist
> -	    (push notmuch-address-message-alist-member message-completion-alist))))
> +  (unless (memq notmuch-address-message-alist-member message-completion-alist)
> +    (setq message-completion-alist
> +	  (push notmuch-address-message-alist-member message-completion-alist))))
>  
>  (defun notmuch-address-options (original)
>    (process-lines notmuch-address-command original))
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index 333d4c1..dce2020 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -315,7 +315,7 @@ should be. Returns a cons cell `(tags-per-line width)'."
>  
>      ;; If the last line was not full (and hence did not include a
>      ;; carriage return), insert one now.
> -    (if (not (eq (% count tags-per-line) 0))
> +    (unless (eq (% count tags-per-line) 0)
>  	(widget-insert "\n"))
>      found-target-pos))
>  
> @@ -399,7 +399,7 @@ Complete list of currently available key bindings:
>  
>    ; Jump through a hoop to get this value from the deprecated variable
>    ; name (`notmuch-folders') or from the default value.
> -  (if (not notmuch-saved-searches)
> +  (unless notmuch-saved-searches
>      (setq notmuch-saved-searches (notmuch-saved-searches)))
>  
>    (if no-display
> @@ -565,18 +565,18 @@ Complete list of currently available key bindings:
>  	  (widget-insert "\n\n")
>  	  (let ((start (point)))
>  	    (setq found-target-pos (notmuch-hello-insert-tags alltags-alist widest target))
> -	    (if (not final-target-pos)
> -		(setq final-target-pos found-target-pos))
> +	    (unless final-target-pos
> +	      (setq final-target-pos found-target-pos))
>  	    (indent-rigidly start (point) notmuch-hello-indent)))
>  
>  	(widget-insert "\n")
>  
> -	(if (not notmuch-show-all-tags-list)
> -	    (widget-create 'push-button
> -			   :notify (lambda (widget &rest ignore)
> -				     (setq notmuch-show-all-tags-list t)
> -				     (notmuch-hello-update))
> -			   "Show all tags")))
> +	(unless notmuch-show-all-tags-list
> +	  (widget-create 'push-button
> +			 :notify (lambda (widget &rest ignore)
> +				   (setq notmuch-show-all-tags-list t)
> +				   (notmuch-hello-update))
> +			 "Show all tags")))
>  
>        (let ((start (point)))
>  	(widget-insert "\n\n")
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5502efd..d3543f0 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -659,8 +659,8 @@ current buffer, if possible."
>    ;; part, so we make sure that we're down at the end.
>    (goto-char (point-max))
>    ;; Ensure that the part ends with a carriage return.
> -  (if (not (bolp))
> -      (insert "\n")))
> +  (unless (bolp)
> +    (insert "\n")))
>  
>  (defun notmuch-show-insert-body (msg body depth)
>    "Insert the body BODY at depth DEPTH in the current thread."
> @@ -740,8 +740,8 @@ current buffer, if possible."
>      (setq body-start (point-marker))
>      (notmuch-show-insert-body msg (plist-get msg :body) depth)
>      ;; Ensure that the body ends with a newline.
> -    (if (not (bolp))
> -	(insert "\n"))
> +    (unless (bolp)
> +      (insert "\n"))
>      (setq body-end (point-marker))
>      (setq content-end (point-marker))
>  
> @@ -882,8 +882,8 @@ buffer."
>        (run-hooks 'notmuch-show-hook))
>  
>      ;; Move straight to the first open message
> -    (if (not (notmuch-show-message-visible-p))
> -	(notmuch-show-next-open-message))
> +    (unless (notmuch-show-message-visible-p)
> +      (notmuch-show-next-open-message))
>  
>      ;; Set the header line to the subject of the first open message.
>      (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 1e61775..ba84494 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -636,8 +636,8 @@ This function advances the next thread when finished."
>  			(if notmuch-search-process-filter-data
>  			    (insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))
>  			(insert "End of search results.")
> -			(if (not (= exit-status 0))
> -			    (insert (format " (process returned %d)" exit-status)))
> +			(unless (= exit-status 0)
> +			  (insert (format " (process returned %d)" exit-status)))
>  			(insert "\n")
>  			(if (and atbob
>  				 (not (string= notmuch-search-target-thread "found")))
> @@ -1006,8 +1006,8 @@ Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
>  depending on the value of `notmuch-poll-script'."
>    (interactive)
>    (if (stringp notmuch-poll-script)
> -      (if (not (string= notmuch-poll-script ""))
> -	  (call-process notmuch-poll-script nil nil))
> +      (unless (string= notmuch-poll-script "")
> +	(call-process notmuch-poll-script nil nil))
>      (call-process notmuch-command nil nil nil "new")))
>  
>  (defun notmuch-search-poll-and-refresh-view ()
> -- 
> 1.7.8.1
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-12 17:33     ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
  2012-01-12 17:40       ` Dmitry Kurochkin
@ 2012-01-12 22:10       ` Xavier Maillard
  2012-01-13  8:23       ` David Edmondson
  2012-01-13 16:06       ` David Edmondson
  3 siblings, 0 replies; 54+ messages in thread
From: Xavier Maillard @ 2012-01-12 22:10 UTC (permalink / raw)
  To: Pieter Praet, Jani Nikula; +Cc: Notmuch Mail


On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.

+1

/Xavier

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-12 17:33     ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
  2012-01-12 17:40       ` Dmitry Kurochkin
  2012-01-12 22:10       ` Xavier Maillard
@ 2012-01-13  8:23       ` David Edmondson
  2012-01-13 12:42         ` Xavier Maillard
  2012-01-14  9:14         ` Pieter Praet
  2012-01-13 16:06       ` David Edmondson
  3 siblings, 2 replies; 54+ messages in thread
From: David Edmondson @ 2012-01-13  8:23 UTC (permalink / raw)
  To: Pieter Praet, Jani Nikula; +Cc: Notmuch Mail

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

On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.

+1, but why not replace non-branching `if' with `when' as well?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-13 12:42         ` Xavier Maillard
@ 2012-01-13 12:37           ` Tomi Ollila
  2012-01-13 13:01           ` David Edmondson
  1 sibling, 0 replies; 54+ messages in thread
From: Tomi Ollila @ 2012-01-13 12:37 UTC (permalink / raw)
  Cc: Notmuch Mail

On Fri, 13 Jan 2012 13:42:55 +0100, Xavier Maillard <xavier@maillard.im> wrote:
> 
> On Fri, 13 Jan 2012 08:23:55 +0000, David Edmondson <dme@dme.org> wrote:
> > On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > Less code, same results, without sacrificing readability.
> > 
> > +1, but why not replace non-branching `if' with `when' as well?
> 
> I tend to use WHEN for case I need to execute multiple
> statements. For the rest, IF is ok.

Same here, (when ...) is (if (progn ...)) for me.

> 
> /Xavier

Tomi

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-13  8:23       ` David Edmondson
@ 2012-01-13 12:42         ` Xavier Maillard
  2012-01-13 12:37           ` Tomi Ollila
  2012-01-13 13:01           ` David Edmondson
  2012-01-14  9:14         ` Pieter Praet
  1 sibling, 2 replies; 54+ messages in thread
From: Xavier Maillard @ 2012-01-13 12:42 UTC (permalink / raw)
  To: David Edmondson, Pieter Praet, Jani Nikula; +Cc: Notmuch Mail


On Fri, 13 Jan 2012 08:23:55 +0000, David Edmondson <dme@dme.org> wrote:
> On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> > Less code, same results, without sacrificing readability.
> 
> +1, but why not replace non-branching `if' with `when' as well?

I tend to use WHEN for case I need to execute multiple
statements. For the rest, IF is ok.

/Xavier

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-13 12:42         ` Xavier Maillard
  2012-01-13 12:37           ` Tomi Ollila
@ 2012-01-13 13:01           ` David Edmondson
  1 sibling, 0 replies; 54+ messages in thread
From: David Edmondson @ 2012-01-13 13:01 UTC (permalink / raw)
  To: Xavier Maillard, Pieter Praet, Jani Nikula; +Cc: Notmuch Mail

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

On Fri, 13 Jan 2012 13:42:55 +0100, Xavier Maillard <xavier@maillard.im> wrote:
> On Fri, 13 Jan 2012 08:23:55 +0000, David Edmondson <dme@dme.org> wrote:
> > On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > Less code, same results, without sacrificing readability.
> > 
> > +1, but why not replace non-branching `if' with `when' as well?
> 
> I tend to use WHEN for case I need to execute multiple
> statements. For the rest, IF is ok.

Understood.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-12 17:33     ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
                         ` (2 preceding siblings ...)
  2012-01-13  8:23       ` David Edmondson
@ 2012-01-13 16:06       ` David Edmondson
  2012-01-14  9:18         ` Pieter Praet
  3 siblings, 1 reply; 54+ messages in thread
From: David Edmondson @ 2012-01-13 16:06 UTC (permalink / raw)
  To: Pieter Praet, Jani Nikula; +Cc: Notmuch Mail

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

On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.

Does this change correctly re-indent the line following the if/unless?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-13  8:23       ` David Edmondson
  2012-01-13 12:42         ` Xavier Maillard
@ 2012-01-14  9:14         ` Pieter Praet
  2012-01-14  9:17           ` [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..." Pieter Praet
  2012-01-14  9:52           ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
  1 sibling, 2 replies; 54+ messages in thread
From: Pieter Praet @ 2012-01-14  9:14 UTC (permalink / raw)
  To: David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Fri, 13 Jan 2012 08:23:55 +0000, David Edmondson <dme@dme.org> wrote:
> On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> > Less code, same results, without sacrificing readability.
> 
> +1, but why not replace non-branching `if' with `when' as well?

I was planning to do that when the `unless' patch was accepted,
but after reading Xavier and Tomi's replies, I've changed my mind.

Looking at "subr.el", it's actually more efficient to use `if'
(implemented in C) instead of `when' (a macro, which essentially
runs "if conf (progn ...)".

The amount of non-branching "(if COND (progn ..."  statements is very
limited though, so if inclined to "fix" them nonetheless, a patch
follows (relative to the previous one!).


Peace

-- 
Pieter

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-14  9:14         ` Pieter Praet
@ 2012-01-14  9:17           ` Pieter Praet
  2012-01-28 12:41             ` David Bremner
  2012-01-31  3:34             ` David Bremner
  2012-01-14  9:52           ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
  1 sibling, 2 replies; 54+ messages in thread
From: Pieter Praet @ 2012-01-14  9:17 UTC (permalink / raw)
  To: David Edmondson, Jani Nikula; +Cc: Notmuch Mail

Less code, same results, without sacrificing readability.

---
 emacs/notmuch-show.el |   20 +++++++++-----------
 emacs/notmuch-wash.el |   47 +++++++++++++++++++++++------------------------
 emacs/notmuch.el      |   28 +++++++++++++---------------
 3 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0cbf354..1e190ae 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1226,11 +1226,10 @@ any effects from previous calls to
       ;; If a small number of lines from the previous message are
       ;; visible, realign so that the top of the current message is at
       ;; the top of the screen.
-      (if (<= (count-screen-lines (window-start) start-of-message)
-	      next-screen-context-lines)
-	  (progn
-	    (goto-char (notmuch-show-message-top))
-	    (notmuch-show-message-adjust)))
+      (when (<= (count-screen-lines (window-start) start-of-message)
+		next-screen-context-lines)
+	(goto-char (notmuch-show-message-top))
+	(notmuch-show-message-adjust))
       ;; Move to the top left of the window.
       (goto-char (window-start)))
      (t
@@ -1423,12 +1422,11 @@ argument, hide all of the messages."
   ;; Move to the next item in the search results, if any.
   (let ((parent-buffer notmuch-show-parent-buffer))
     (notmuch-kill-this-buffer)
-    (if parent-buffer
-	(progn
-	  (switch-to-buffer parent-buffer)
-	  (forward-line)
-	  (if show-next
-	      (notmuch-search-show-thread))))))
+    (when parent-buffer
+      (switch-to-buffer parent-buffer)
+      (forward-line)
+      (if show-next
+	  (notmuch-search-show-thread)))))
 
 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 5c1e830..67143e5 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -336,30 +336,29 @@ patch and then guesses the extent of the patch, there is scope
 for error."
 
   (goto-char (point-min))
-  (if (re-search-forward diff-file-header-re nil t)
-      (progn
-	(beginning-of-line -1)
-	(let ((patch-start (point))
-	      (patch-end (point-max))
-	      part)
-	  (goto-char patch-start)
-	  (if (or
-	       ;; Patch ends with signature.
-	       (re-search-forward notmuch-wash-signature-regexp nil t)
-	       ;; Patch ends with bugtraq comment.
-	       (re-search-forward "^\\*\\*\\* " nil t))
-	      (setq patch-end (match-beginning 0)))
-	  (save-restriction
-	    (narrow-to-region patch-start patch-end)
-	    (setq part (plist-put part :content-type "inline-patch-fake-part"))
-	    (setq part (plist-put part :content (buffer-string)))
-	    (setq part (plist-put part :id -1))
-	    (setq part (plist-put part :filename
-				  (notmuch-wash-subject-to-patch-filename
-				   (plist-get
-				    (plist-get msg :headers) :Subject))))
-	    (delete-region (point-min) (point-max))
-	    (notmuch-show-insert-bodypart nil part depth))))))
+  (when (re-search-forward diff-file-header-re nil t)
+    (beginning-of-line -1)
+    (let ((patch-start (point))
+	  (patch-end (point-max))
+	  part)
+      (goto-char patch-start)
+      (if (or
+	   ;; Patch ends with signature.
+	   (re-search-forward notmuch-wash-signature-regexp nil t)
+	   ;; Patch ends with bugtraq comment.
+	   (re-search-forward "^\\*\\*\\* " nil t))
+	  (setq patch-end (match-beginning 0)))
+      (save-restriction
+	(narrow-to-region patch-start patch-end)
+	(setq part (plist-put part :content-type "inline-patch-fake-part"))
+	(setq part (plist-put part :content (buffer-string)))
+	(setq part (plist-put part :id -1))
+	(setq part (plist-put part :filename
+			      (notmuch-wash-subject-to-patch-filename
+			       (plist-get
+				(plist-get msg :headers) :Subject))))
+	(delete-region (point-min) (point-max))
+	(notmuch-show-insert-bodypart nil part depth)))))
 
 ;;
 
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ba84494..0d37a74 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -631,17 +631,16 @@ This function advances the next thread when finished."
 		  (goto-char (point-max))
 		  (if (eq status 'signal)
 		      (insert "Incomplete search results (search process was killed).\n"))
-		  (if (eq status 'exit)
-		      (progn
-			(if notmuch-search-process-filter-data
-			    (insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))
-			(insert "End of search results.")
-			(unless (= exit-status 0)
-			  (insert (format " (process returned %d)" exit-status)))
-			(insert "\n")
-			(if (and atbob
-				 (not (string= notmuch-search-target-thread "found")))
-			    (set 'never-found-target-thread t))))))
+		  (when (eq status 'exit)
+		    (if notmuch-search-process-filter-data
+			(insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))
+		    (insert "End of search results.")
+		    (unless (= exit-status 0)
+		      (insert (format " (process returned %d)" exit-status)))
+		    (insert "\n")
+		    (if (and atbob
+			     (not (string= notmuch-search-target-thread "found")))
+			(set 'never-found-target-thread t)))))
 	      (when (and never-found-target-thread
 		       notmuch-search-target-line)
 		  (goto-char (point-min))
@@ -818,10 +817,9 @@ non-authors is found, assume that all of the authors match."
 			(put-text-property beg (point) 'notmuch-search-thread-id thread-id)
 			(put-text-property beg (point) 'notmuch-search-authors authors)
 			(put-text-property beg (point) 'notmuch-search-subject subject)
-			(if (string= thread-id notmuch-search-target-thread)
-			    (progn
-			      (set 'found-target beg)
-			      (set 'notmuch-search-target-thread "found"))))
+			(when (string= thread-id notmuch-search-target-thread)
+			  (set 'found-target beg)
+			  (set 'notmuch-search-target-thread "found")))
 		      (set 'line (match-end 0)))
 		  (set 'more nil)
 		  (while (and (< line (length string)) (= (elt string line) ?\n))
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-13 16:06       ` David Edmondson
@ 2012-01-14  9:18         ` Pieter Praet
  2012-01-15 11:55           ` David Edmondson
  0 siblings, 1 reply; 54+ messages in thread
From: Pieter Praet @ 2012-01-14  9:18 UTC (permalink / raw)
  To: David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Fri, 13 Jan 2012 16:06:17 +0000, David Edmondson <dme@dme.org> wrote:
> On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> > Less code, same results, without sacrificing readability.
> 
> Does this change correctly re-indent the line following the if/unless?

It does...

Or so I thought... I appear to have forgotten to correct the indentation
@ `notmuch-hello-insert-tags'.


Unfortunately git doesn't provide a way to diff the whitespace itself;
  (eg. "git diff --word-diff=color --word-diff-regex='[ \t]*'")

Setting "color.diff.whitespace" (and perhaps expanding the scope of
"core.whitespace") only colorizes *erroneous* whitespace...


Does this really warrant a v2, or might we simply leave it as yet
another victim for Tomi's uncrustify-for-elisp [1] ?


Peace

-- 
Pieter

[1] id:"yf639bkexs3.fsf@taco2.nixu.fi"

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-14  9:14         ` Pieter Praet
  2012-01-14  9:17           ` [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..." Pieter Praet
@ 2012-01-14  9:52           ` Pieter Praet
  1 sibling, 0 replies; 54+ messages in thread
From: Pieter Praet @ 2012-01-14  9:52 UTC (permalink / raw)
  To: David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Sat, 14 Jan 2012 10:14:57 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Fri, 13 Jan 2012 08:23:55 +0000, David Edmondson <dme@dme.org> wrote:
> > On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > Less code, same results, without sacrificing readability.
> > 
> > +1, but why not replace non-branching `if' with `when' as well?
> 
> I was planning to do that when the `unless' patch was accepted,
                    ^^
                   submit

> but after reading Xavier and Tomi's replies, I've changed my mind.
                                               ^^^^^^^^^^^^^^^^^^^^
                                               I hadn't, though...

> 
> Looking at "subr.el", it's actually more efficient to use `if'
> (implemented in C) instead of `when' (a macro, which essentially
> runs "if conf (progn ...)".
> 
> The amount of non-branching "(if COND (progn ..."  statements is very
> limited though, so if inclined to "fix" them nonetheless, a patch
> follows (relative to the previous one!).
> 
> 
> Peace
> 
> -- 
> Pieter

Typsos FTW!

-- 
Pieter

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-14  9:18         ` Pieter Praet
@ 2012-01-15 11:55           ` David Edmondson
  2012-01-16 10:56             ` [PATCH v2] " Pieter Praet
  0 siblings, 1 reply; 54+ messages in thread
From: David Edmondson @ 2012-01-15 11:55 UTC (permalink / raw)
  To: Pieter Praet, Jani Nikula; +Cc: Notmuch Mail

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

On Sat, 14 Jan 2012 10:18:46 +0100, Pieter Praet <pieter@praet.org> wrote:
> Does this really warrant a v2, or might we simply leave it as yet
> another victim for Tomi's uncrustify-for-elisp [1] ?

Pushing incorrectly indented code should be banned, in my opinion[1]. I'd
be tempted to have some pre-commit hooks to detect it.

Footnotes: 
[1]  Though I've done it myself.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-15 11:55           ` David Edmondson
@ 2012-01-16 10:56             ` Pieter Praet
  2012-01-16 11:06               ` David Edmondson
                                 ` (2 more replies)
  0 siblings, 3 replies; 54+ messages in thread
From: Pieter Praet @ 2012-01-16 10:56 UTC (permalink / raw)
  To: David Edmondson, Jani Nikula; +Cc: Notmuch Mail

Less code, same results, without sacrificing readability.

---

v2: Fixed indentation oversight @ `notmuch-hello-insert-tags'.

 emacs/notmuch-address.el |    6 +++---
 emacs/notmuch-hello.el   |   22 +++++++++++-----------
 emacs/notmuch-show.el    |   12 ++++++------
 emacs/notmuch.el         |    8 ++++----
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 8eba7a0..d72b169 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -37,9 +37,9 @@ line."
 (defvar notmuch-address-history nil)
 
 (defun notmuch-address-message-insinuate ()
-  (if (not (memq notmuch-address-message-alist-member message-completion-alist))
-      (setq message-completion-alist
-	    (push notmuch-address-message-alist-member message-completion-alist))))
+  (unless (memq notmuch-address-message-alist-member message-completion-alist)
+    (setq message-completion-alist
+	  (push notmuch-address-message-alist-member message-completion-alist))))
 
 (defun notmuch-address-options (original)
   (process-lines notmuch-address-command original))
diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 02017ce..8aa2ad5 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -317,8 +317,8 @@ should be. Returns a cons cell `(tags-per-line width)'."
 
     ;; If the last line was not full (and hence did not include a
     ;; carriage return), insert one now.
-    (if (not (eq (% count tags-per-line) 0))
-	(widget-insert "\n"))
+    (unless (eq (% count tags-per-line) 0)
+      (widget-insert "\n"))
     found-target-pos))
 
 (defun notmuch-hello-goto-search ()
@@ -401,7 +401,7 @@ Complete list of currently available key bindings:
 
   ; Jump through a hoop to get this value from the deprecated variable
   ; name (`notmuch-folders') or from the default value.
-  (if (not notmuch-saved-searches)
+  (unless notmuch-saved-searches
     (setq notmuch-saved-searches (notmuch-saved-searches)))
 
   (if no-display
@@ -567,18 +567,18 @@ Complete list of currently available key bindings:
 	  (widget-insert "\n\n")
 	  (let ((start (point)))
 	    (setq found-target-pos (notmuch-hello-insert-tags alltags-alist widest target))
-	    (if (not final-target-pos)
-		(setq final-target-pos found-target-pos))
+	    (unless final-target-pos
+	      (setq final-target-pos found-target-pos))
 	    (indent-rigidly start (point) notmuch-hello-indent)))
 
 	(widget-insert "\n")
 
-	(if (not notmuch-show-all-tags-list)
-	    (widget-create 'push-button
-			   :notify (lambda (widget &rest ignore)
-				     (setq notmuch-show-all-tags-list t)
-				     (notmuch-hello-update))
-			   "Show all tags")))
+	(unless notmuch-show-all-tags-list
+	  (widget-create 'push-button
+			 :notify (lambda (widget &rest ignore)
+				   (setq notmuch-show-all-tags-list t)
+				   (notmuch-hello-update))
+			 "Show all tags")))
 
       (let ((start (point)))
 	(widget-insert "\n\n")
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 1a250a3..72bf888 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -656,8 +656,8 @@ current buffer, if possible."
   ;; part, so we make sure that we're down at the end.
   (goto-char (point-max))
   ;; Ensure that the part ends with a carriage return.
-  (if (not (bolp))
-      (insert "\n")))
+  (unless (bolp)
+    (insert "\n")))
 
 (defun notmuch-show-insert-body (msg body depth)
   "Insert the body BODY at depth DEPTH in the current thread."
@@ -737,8 +737,8 @@ current buffer, if possible."
     (setq body-start (point-marker))
     (notmuch-show-insert-body msg (plist-get msg :body) depth)
     ;; Ensure that the body ends with a newline.
-    (if (not (bolp))
-	(insert "\n"))
+    (unless (bolp)
+      (insert "\n"))
     (setq body-end (point-marker))
     (setq content-end (point-marker))
 
@@ -879,8 +879,8 @@ buffer."
       (run-hooks 'notmuch-show-hook))
 
     ;; Move straight to the first open message
-    (if (not (notmuch-show-message-visible-p))
-	(notmuch-show-next-open-message))
+    (unless (notmuch-show-message-visible-p)
+      (notmuch-show-next-open-message))
 
     ;; Set the header line to the subject of the first open message.
     (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ef4dcc7..c5fcc80 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -643,8 +643,8 @@ This function advances the next thread when finished."
 			(if notmuch-search-process-filter-data
 			    (insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))
 			(insert "End of search results.")
-			(if (not (= exit-status 0))
-			    (insert (format " (process returned %d)" exit-status)))
+			(unless (= exit-status 0)
+			  (insert (format " (process returned %d)" exit-status)))
 			(insert "\n")
 			(if (and atbob
 				 (not (string= notmuch-search-target-thread "found")))
@@ -1013,8 +1013,8 @@ Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
 depending on the value of `notmuch-poll-script'."
   (interactive)
   (if (stringp notmuch-poll-script)
-      (if (not (string= notmuch-poll-script ""))
-	  (call-process notmuch-poll-script nil nil))
+      (unless (string= notmuch-poll-script "")
+	(call-process notmuch-poll-script nil nil))
     (call-process notmuch-command nil nil nil "new")))
 
 (defun notmuch-search-poll-and-refresh-view ()
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v2] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-16 10:56             ` [PATCH v2] " Pieter Praet
@ 2012-01-16 11:06               ` David Edmondson
  2012-01-16 12:39               ` Tomi Ollila
  2012-01-21 12:21               ` David Bremner
  2 siblings, 0 replies; 54+ messages in thread
From: David Edmondson @ 2012-01-16 11:06 UTC (permalink / raw)
  To: Pieter Praet, Jani Nikula; +Cc: Notmuch Mail

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

On Mon, 16 Jan 2012 11:56:40 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.

+1.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-16 10:56             ` [PATCH v2] " Pieter Praet
  2012-01-16 11:06               ` David Edmondson
@ 2012-01-16 12:39               ` Tomi Ollila
  2012-01-21 12:21               ` David Bremner
  2 siblings, 0 replies; 54+ messages in thread
From: Tomi Ollila @ 2012-01-16 12:39 UTC (permalink / raw)
  To: Pieter Praet, David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Mon, 16 Jan 2012 11:56:40 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.
> 
> ---

+1

Tomi

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2] emacs: globally replace non-branching "(if (not ..." with "(unless ..."
  2012-01-16 10:56             ` [PATCH v2] " Pieter Praet
  2012-01-16 11:06               ` David Edmondson
  2012-01-16 12:39               ` Tomi Ollila
@ 2012-01-21 12:21               ` David Bremner
  2 siblings, 0 replies; 54+ messages in thread
From: David Bremner @ 2012-01-21 12:21 UTC (permalink / raw)
  To: Pieter Praet; +Cc: Notmuch Mail

On Mon, 16 Jan 2012 11:56:40 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.
> 

pushed

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-14  9:17           ` [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..." Pieter Praet
@ 2012-01-28 12:41             ` David Bremner
  2012-01-28 12:55               ` Jani Nikula
  2012-01-30  7:03               ` Pieter Praet
  2012-01-31  3:34             ` David Bremner
  1 sibling, 2 replies; 54+ messages in thread
From: David Bremner @ 2012-01-28 12:41 UTC (permalink / raw)
  To: Pieter Praet, David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.
> 

This looks OK, although the re-indenting makes these kind of changes
painful to review (not that I'm suggesting we should re-indent, just
some random complaining).

d

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-28 12:41             ` David Bremner
@ 2012-01-28 12:55               ` Jani Nikula
  2012-01-28 17:14                 ` David Bremner
  2012-01-30  7:03               ` Pieter Praet
  1 sibling, 1 reply; 54+ messages in thread
From: Jani Nikula @ 2012-01-28 12:55 UTC (permalink / raw)
  To: David Bremner; +Cc: Notmuch Mail, Pieter Praet

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

On Jan 28, 2012 2:41 PM, "David Bremner" <david@tethera.net> wrote:
>
> On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet <pieter@praet.org> wrote:
> > Less code, same results, without sacrificing readability.
> >
>
> This looks OK, although the re-indenting makes these kind of changes
> painful to review (not that I'm suggesting we should re-indent, just
> some random complaining).

Sometimes someone (Dmitry?) sent patches that separated a small functional
change, and the big non-functional indentation change it caused,
separately. Would you prefer (or tolerate ;) that style?

>
> d
>
>

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-28 12:55               ` Jani Nikula
@ 2012-01-28 17:14                 ` David Bremner
  2012-01-30  9:23                   ` David Edmondson
  0 siblings, 1 reply; 54+ messages in thread
From: David Bremner @ 2012-01-28 17:14 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Notmuch Mail

On Sat, 28 Jan 2012 14:55:22 +0200, Jani Nikula <jani@nikula.org> wrote:
> On Jan 28, 2012 2:41 PM, "David Bremner" <david@tethera.net> wrote:
> 
> Sometimes someone (Dmitry?) sent patches that separated a small functional
> change, and the big non-functional indentation change it caused,
> separately. Would you prefer (or tolerate ;) that style?

Hmm, that might be nicer, I'm not 100% sure.

I wouldn't say it's mandatory for a patch like this (and I'd say other
peoples views on what's easy to review are at least as important as mine
here).

d

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-28 12:41             ` David Bremner
  2012-01-28 12:55               ` Jani Nikula
@ 2012-01-30  7:03               ` Pieter Praet
  2012-01-30  7:59                 ` Tomi Ollila
  1 sibling, 1 reply; 54+ messages in thread
From: Pieter Praet @ 2012-01-30  7:03 UTC (permalink / raw)
  To: David Bremner, David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Sat, 28 Jan 2012 08:41:36 -0400, David Bremner <david@tethera.net> wrote:
> On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet <pieter@praet.org> wrote:
> > Less code, same results, without sacrificing readability.
> > 
> 
> This looks OK, although the re-indenting makes these kind of changes
> painful to review (not that I'm suggesting we should re-indent, just
> some random complaining).
> 

You can use `diff-refine-hunk' to see what the actual changes are.

Try this:

  #+begin_src emacs-lisp
    (global-set-key (kbd "C-c /")
                    (lambda()
                      "Refine display of unified diff hunks"
                      (interactive)
                      (save-excursion
                        (goto-char (point-min))
                        (while (re-search-forward
                                diff-hunk-header-re-unified
                                nil t)
                          (diff-refine-hunk)))))
  #+end_src

Work pretty much *anywhere*.

Note: it does NOT work in `notmuch-show-mode' (not even with
`notmuch-wash-convert-inline-patch-to-part' disabled), but this is
easily solved by first running `notmuch-show-view-raw-message' ("V")
and `diff-mode' (to fontify the buffer)...

> d
> 
> 


Peace

-- 
Pieter

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-30  7:03               ` Pieter Praet
@ 2012-01-30  7:59                 ` Tomi Ollila
  2012-02-01 13:46                   ` Pieter Praet
  0 siblings, 1 reply; 54+ messages in thread
From: Tomi Ollila @ 2012-01-30  7:59 UTC (permalink / raw)
  To: Pieter Praet, David Bremner, David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Mon, 30 Jan 2012 08:03:59 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Sat, 28 Jan 2012 08:41:36 -0400, David Bremner <david@tethera.net> wrote:
> > On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > Less code, same results, without sacrificing readability.
> > > 
> > 
> > This looks OK, although the re-indenting makes these kind of changes
> > painful to review (not that I'm suggesting we should re-indent, just
> > some random complaining).
> > 
> 
> You can use `diff-refine-hunk' to see what the actual changes are.
> 
> Try this:
> 
>   #+begin_src emacs-lisp
>     (global-set-key (kbd "C-c /")
>                     (lambda()
>                       "Refine display of unified diff hunks"
>                       (interactive)
>                       (save-excursion
>                         (goto-char (point-min))
>                         (while (re-search-forward
>                                 diff-hunk-header-re-unified
>                                 nil t)
>                           (diff-refine-hunk)))))
>   #+end_src
> 
> Work pretty much *anywhere*.
> 
> Note: it does NOT work in `notmuch-show-mode' (not even with
> `notmuch-wash-convert-inline-patch-to-part' disabled), but this is
> easily solved by first running `notmuch-show-view-raw-message' ("V")
> and `diff-mode' (to fontify the buffer)...

What we need is 'notmuch-devel' minor mode which can do this, dme's
notmuch patching tool, nmbug tag changes and everything...

> 
> > d
> 
> 
> Peace
> 
> -- 
> Pieter

Tomi

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-28 17:14                 ` David Bremner
@ 2012-01-30  9:23                   ` David Edmondson
  2012-02-01 13:46                     ` Pieter Praet
  0 siblings, 1 reply; 54+ messages in thread
From: David Edmondson @ 2012-01-30  9:23 UTC (permalink / raw)
  To: David Bremner, Jani Nikula; +Cc: Notmuch Mail

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

On Sat, 28 Jan 2012 13:14:45 -0400, David Bremner <david@tethera.net> wrote:
> On Sat, 28 Jan 2012 14:55:22 +0200, Jani Nikula <jani@nikula.org> wrote:
> > On Jan 28, 2012 2:41 PM, "David Bremner" <david@tethera.net> wrote:
> > 
> > Sometimes someone (Dmitry?) sent patches that separated a small functional
> > change, and the big non-functional indentation change it caused,
> > separately. Would you prefer (or tolerate ;) that style?
> 
> Hmm, that might be nicer, I'm not 100% sure.
> 
> I wouldn't say it's mandatory for a patch like this (and I'd say other
> peoples views on what's easy to review are at least as important as mine
> here).

Each patch should be valid in the repository without any following
patches (preceding are obviously okay). Incorrect indentation would
disqualify a patch from being 'valid', so it shouldn't be accepted.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-14  9:17           ` [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..." Pieter Praet
  2012-01-28 12:41             ` David Bremner
@ 2012-01-31  3:34             ` David Bremner
  2012-02-01 13:47               ` Pieter Praet
  1 sibling, 1 reply; 54+ messages in thread
From: David Bremner @ 2012-01-31  3:34 UTC (permalink / raw)
  To: Pieter Praet; +Cc: Notmuch Mail

On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.
> 
> ---

Sorry, it conflicts with Jamie's patches that I just pushed.

d

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-30  9:23                   ` David Edmondson
@ 2012-02-01 13:46                     ` Pieter Praet
  2012-02-01 14:21                       ` Dmitry Kurochkin
  0 siblings, 1 reply; 54+ messages in thread
From: Pieter Praet @ 2012-02-01 13:46 UTC (permalink / raw)
  To: David Edmondson, David Bremner, Jani Nikula; +Cc: Notmuch Mail

On Mon, 30 Jan 2012 09:23:40 +0000, David Edmondson <dme@dme.org> wrote:
> On Sat, 28 Jan 2012 13:14:45 -0400, David Bremner <david@tethera.net> wrote:
> > On Sat, 28 Jan 2012 14:55:22 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > On Jan 28, 2012 2:41 PM, "David Bremner" <david@tethera.net> wrote:
> > > 
> > > Sometimes someone (Dmitry?) sent patches that separated a small functional
> > > change, and the big non-functional indentation change it caused,
> > > separately. Would you prefer (or tolerate ;) that style?
> > 
> > Hmm, that might be nicer, I'm not 100% sure.
> > 
> > I wouldn't say it's mandatory for a patch like this (and I'd say other
> > peoples views on what's easy to review are at least as important as mine
> > here).
> 
> Each patch should be valid in the repository without any following
> patches (preceding are obviously okay). Incorrect indentation would
> disqualify a patch from being 'valid', so it shouldn't be accepted.

+1.

Indentation corrections should always be part of the same patch as
the change(s) that invalidated the indentation in the first place.

Spotting the *actual* (non-indentation) changes is a non-issue
when using `diff-refine-hunk' (or a wrapper thereof [1]).


> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

[1] id:"8739ax7jts.fsf@praet.org"

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-30  7:59                 ` Tomi Ollila
@ 2012-02-01 13:46                   ` Pieter Praet
  0 siblings, 0 replies; 54+ messages in thread
From: Pieter Praet @ 2012-02-01 13:46 UTC (permalink / raw)
  To: Tomi Ollila, David Bremner, David Edmondson, Jani Nikula; +Cc: Notmuch Mail

On Mon, 30 Jan 2012 09:59:32 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Mon, 30 Jan 2012 08:03:59 +0100, Pieter Praet <pieter@praet.org> wrote:
> > On Sat, 28 Jan 2012 08:41:36 -0400, David Bremner <david@tethera.net> wrote:
> > > On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > > Less code, same results, without sacrificing readability.
> > > > 
> > > 
> > > This looks OK, although the re-indenting makes these kind of changes
> > > painful to review (not that I'm suggesting we should re-indent, just
> > > some random complaining).
> > > 
> > 
> > You can use `diff-refine-hunk' to see what the actual changes are.
> > 
> > Try this:
> > 
> >   #+begin_src emacs-lisp
> >     (global-set-key (kbd "C-c /")
> >                     (lambda()
> >                       "Refine display of unified diff hunks"
> >                       (interactive)
> >                       (save-excursion
> >                         (goto-char (point-min))
> >                         (while (re-search-forward
> >                                 diff-hunk-header-re-unified
> >                                 nil t)
> >                           (diff-refine-hunk)))))
> >   #+end_src
> > 
> > Work pretty much *anywhere*.
> > 
> > Note: it does NOT work in `notmuch-show-mode' (not even with
> > `notmuch-wash-convert-inline-patch-to-part' disabled), but this is
> > easily solved by first running `notmuch-show-view-raw-message' ("V")
> > and `diff-mode' (to fontify the buffer)...
> 
> What we need is 'notmuch-devel' minor mode which can do this, dme's
> notmuch patching tool, nmbug tag changes and everything...
> 

Full ACK!


> > 
> > > d
> > 
> > 
> > Peace
> > 
> > -- 
> > Pieter
> 
> Tomi


Peace

-- 
Pieter

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-01-31  3:34             ` David Bremner
@ 2012-02-01 13:47               ` Pieter Praet
  2012-02-01 13:50                 ` [PATCH v2] " Pieter Praet
  0 siblings, 1 reply; 54+ messages in thread
From: Pieter Praet @ 2012-02-01 13:47 UTC (permalink / raw)
  To: David Bremner; +Cc: Notmuch Mail

On Mon, 30 Jan 2012 23:34:12 -0400, David Bremner <david@tethera.net> wrote:
> On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet <pieter@praet.org> wrote:
> > Less code, same results, without sacrificing readability.
> > 
> > ---
> 
> Sorry, it conflicts with Jamie's patches that I just pushed.
> 

Please don't apologize for doing the right thing :)

Rebased patch follows.


> d


Peace

-- 
Pieter

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-02-01 13:47               ` Pieter Praet
@ 2012-02-01 13:50                 ` Pieter Praet
  2012-02-01 14:05                   ` David Edmondson
  2012-02-02  1:31                   ` David Bremner
  0 siblings, 2 replies; 54+ messages in thread
From: Pieter Praet @ 2012-02-01 13:50 UTC (permalink / raw)
  To: David Bremner; +Cc: Notmuch Mail

Less code, same results, without sacrificing readability.

---

Rebased to current master.

 emacs/notmuch-show.el |    9 ++++-----
 emacs/notmuch-wash.el |   47 +++++++++++++++++++++++------------------------
 emacs/notmuch.el      |   28 +++++++++++++---------------
 3 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index de9421e..0a945ea 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1364,11 +1364,10 @@ any effects from previous calls to
       ;; If a small number of lines from the previous message are
       ;; visible, realign so that the top of the current message is at
       ;; the top of the screen.
-      (if (<= (count-screen-lines (window-start) start-of-message)
-	      next-screen-context-lines)
-	  (progn
-	    (goto-char (notmuch-show-message-top))
-	    (notmuch-show-message-adjust)))
+      (when (<= (count-screen-lines (window-start) start-of-message)
+		next-screen-context-lines)
+	(goto-char (notmuch-show-message-top))
+	(notmuch-show-message-adjust))
       ;; Move to the top left of the window.
       (goto-char (window-start)))
      (t
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 5c1e830..67143e5 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -336,30 +336,29 @@ patch and then guesses the extent of the patch, there is scope
 for error."
 
   (goto-char (point-min))
-  (if (re-search-forward diff-file-header-re nil t)
-      (progn
-	(beginning-of-line -1)
-	(let ((patch-start (point))
-	      (patch-end (point-max))
-	      part)
-	  (goto-char patch-start)
-	  (if (or
-	       ;; Patch ends with signature.
-	       (re-search-forward notmuch-wash-signature-regexp nil t)
-	       ;; Patch ends with bugtraq comment.
-	       (re-search-forward "^\\*\\*\\* " nil t))
-	      (setq patch-end (match-beginning 0)))
-	  (save-restriction
-	    (narrow-to-region patch-start patch-end)
-	    (setq part (plist-put part :content-type "inline-patch-fake-part"))
-	    (setq part (plist-put part :content (buffer-string)))
-	    (setq part (plist-put part :id -1))
-	    (setq part (plist-put part :filename
-				  (notmuch-wash-subject-to-patch-filename
-				   (plist-get
-				    (plist-get msg :headers) :Subject))))
-	    (delete-region (point-min) (point-max))
-	    (notmuch-show-insert-bodypart nil part depth))))))
+  (when (re-search-forward diff-file-header-re nil t)
+    (beginning-of-line -1)
+    (let ((patch-start (point))
+	  (patch-end (point-max))
+	  part)
+      (goto-char patch-start)
+      (if (or
+	   ;; Patch ends with signature.
+	   (re-search-forward notmuch-wash-signature-regexp nil t)
+	   ;; Patch ends with bugtraq comment.
+	   (re-search-forward "^\\*\\*\\* " nil t))
+	  (setq patch-end (match-beginning 0)))
+      (save-restriction
+	(narrow-to-region patch-start patch-end)
+	(setq part (plist-put part :content-type "inline-patch-fake-part"))
+	(setq part (plist-put part :content (buffer-string)))
+	(setq part (plist-put part :id -1))
+	(setq part (plist-put part :filename
+			      (notmuch-wash-subject-to-patch-filename
+			       (plist-get
+				(plist-get msg :headers) :Subject))))
+	(delete-region (point-min) (point-max))
+	(notmuch-show-insert-bodypart nil part depth)))))
 
 ;;
 
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 72f78ed..5fa239a 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -673,17 +673,16 @@ This function advances the next thread when finished."
 		  (goto-char (point-max))
 		  (if (eq status 'signal)
 		      (insert "Incomplete search results (search process was killed).\n"))
-		  (if (eq status 'exit)
-		      (progn
-			(if notmuch-search-process-filter-data
-			    (insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))
-			(insert "End of search results.")
-			(unless (= exit-status 0)
-			  (insert (format " (process returned %d)" exit-status)))
-			(insert "\n")
-			(if (and atbob
-				 (not (string= notmuch-search-target-thread "found")))
-			    (set 'never-found-target-thread t))))))
+		  (when (eq status 'exit)
+		    (if notmuch-search-process-filter-data
+			(insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))
+		    (insert "End of search results.")
+		    (unless (= exit-status 0)
+		      (insert (format " (process returned %d)" exit-status)))
+		    (insert "\n")
+		    (if (and atbob
+			     (not (string= notmuch-search-target-thread "found")))
+			(set 'never-found-target-thread t)))))
 	      (when (and never-found-target-thread
 		       notmuch-search-target-line)
 		  (goto-char (point-min))
@@ -861,10 +860,9 @@ non-authors is found, assume that all of the authors match."
 			(put-text-property beg (point) 'notmuch-search-thread-id thread-id)
 			(put-text-property beg (point) 'notmuch-search-authors authors)
 			(put-text-property beg (point) 'notmuch-search-subject subject)
-			(if (string= thread-id notmuch-search-target-thread)
-			    (progn
-			      (set 'found-target beg)
-			      (set 'notmuch-search-target-thread "found"))))
+			(when (string= thread-id notmuch-search-target-thread)
+			  (set 'found-target beg)
+			  (set 'notmuch-search-target-thread "found")))
 		      (set 'line (match-end 0)))
 		  (set 'more nil)
 		  (while (and (< line (length string)) (= (elt string line) ?\n))
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v2] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-02-01 13:50                 ` [PATCH v2] " Pieter Praet
@ 2012-02-01 14:05                   ` David Edmondson
  2012-02-02  1:31                   ` David Bremner
  1 sibling, 0 replies; 54+ messages in thread
From: David Edmondson @ 2012-02-01 14:05 UTC (permalink / raw)
  To: Pieter Praet, David Bremner; +Cc: Notmuch Mail

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

On Wed,  1 Feb 2012 14:50:00 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.

I didn't check the indentation, but the sense of the changes is good.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-02-01 13:46                     ` Pieter Praet
@ 2012-02-01 14:21                       ` Dmitry Kurochkin
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Kurochkin @ 2012-02-01 14:21 UTC (permalink / raw)
  To: Pieter Praet, David Edmondson, David Bremner, Jani Nikula; +Cc: Notmuch Mail

On Wed, 01 Feb 2012 14:46:19 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Mon, 30 Jan 2012 09:23:40 +0000, David Edmondson <dme@dme.org> wrote:
> > On Sat, 28 Jan 2012 13:14:45 -0400, David Bremner <david@tethera.net> wrote:
> > > On Sat, 28 Jan 2012 14:55:22 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > > On Jan 28, 2012 2:41 PM, "David Bremner" <david@tethera.net> wrote:
> > > > 
> > > > Sometimes someone (Dmitry?) sent patches that separated a small functional
> > > > change, and the big non-functional indentation change it caused,
> > > > separately. Would you prefer (or tolerate ;) that style?
> > > 
> > > Hmm, that might be nicer, I'm not 100% sure.
> > > 
> > > I wouldn't say it's mandatory for a patch like this (and I'd say other
> > > peoples views on what's easy to review are at least as important as mine
> > > here).
> > 
> > Each patch should be valid in the repository without any following
> > patches (preceding are obviously okay). Incorrect indentation would
> > disqualify a patch from being 'valid', so it shouldn't be accepted.
> 
> +1.
> 
> Indentation corrections should always be part of the same patch as
> the change(s) that invalidated the indentation in the first place.
> 
> Spotting the *actual* (non-indentation) changes is a non-issue
> when using `diff-refine-hunk' (or a wrapper thereof [1]).
> 

Must... resist... getting... into... this...

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."
  2012-02-01 13:50                 ` [PATCH v2] " Pieter Praet
  2012-02-01 14:05                   ` David Edmondson
@ 2012-02-02  1:31                   ` David Bremner
  1 sibling, 0 replies; 54+ messages in thread
From: David Bremner @ 2012-02-02  1:31 UTC (permalink / raw)
  To: Pieter Praet; +Cc: Notmuch Mail

On Wed,  1 Feb 2012 14:50:00 +0100, Pieter Praet <pieter@praet.org> wrote:
> Less code, same results, without sacrificing readability.
> 

pushed.

d

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2012-02-02  1:31 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-11 21:48 [PATCH] emacs: support "notmuch new" as a notmuch-poll-script Jani Nikula
2011-12-11 22:00 ` Dmitry Kurochkin
2011-12-11 22:19   ` Jani Nikula
2011-12-11 22:39     ` Dmitry Kurochkin
2011-12-11 22:58   ` Austin Clements
2011-12-11 23:10     ` Jani Nikula
2011-12-12  0:31       ` Austin Clements
2011-12-12  0:39         ` Dmitry Kurochkin
2011-12-12 10:15         ` Tomi Ollila
2011-12-12 10:21           ` Dmitry Kurochkin
2011-12-12 11:38             ` Tomi Ollila
2011-12-12 15:42             ` Austin Clements
2011-12-12 19:57 ` [PATCH v2] " Jani Nikula
2011-12-12 20:12   ` Dmitry Kurochkin
2011-12-12 20:24   ` Austin Clements
2011-12-12 20:50 ` [PATCH v3] " Jani Nikula
2011-12-12 20:53   ` Dmitry Kurochkin
2011-12-12 21:13     ` Jani Nikula
2011-12-12 21:24       ` Austin Clements
2011-12-12 21:34         ` Dmitry Kurochkin
2011-12-12 21:01   ` Austin Clements
2011-12-15  4:24   ` David Bremner
2012-01-12 17:31   ` Pieter Praet
2012-01-12 17:33     ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
2012-01-12 17:40       ` Dmitry Kurochkin
2012-01-12 22:10       ` Xavier Maillard
2012-01-13  8:23       ` David Edmondson
2012-01-13 12:42         ` Xavier Maillard
2012-01-13 12:37           ` Tomi Ollila
2012-01-13 13:01           ` David Edmondson
2012-01-14  9:14         ` Pieter Praet
2012-01-14  9:17           ` [PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..." Pieter Praet
2012-01-28 12:41             ` David Bremner
2012-01-28 12:55               ` Jani Nikula
2012-01-28 17:14                 ` David Bremner
2012-01-30  9:23                   ` David Edmondson
2012-02-01 13:46                     ` Pieter Praet
2012-02-01 14:21                       ` Dmitry Kurochkin
2012-01-30  7:03               ` Pieter Praet
2012-01-30  7:59                 ` Tomi Ollila
2012-02-01 13:46                   ` Pieter Praet
2012-01-31  3:34             ` David Bremner
2012-02-01 13:47               ` Pieter Praet
2012-02-01 13:50                 ` [PATCH v2] " Pieter Praet
2012-02-01 14:05                   ` David Edmondson
2012-02-02  1:31                   ` David Bremner
2012-01-14  9:52           ` [PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..." Pieter Praet
2012-01-13 16:06       ` David Edmondson
2012-01-14  9:18         ` Pieter Praet
2012-01-15 11:55           ` David Edmondson
2012-01-16 10:56             ` [PATCH v2] " Pieter Praet
2012-01-16 11:06               ` David Edmondson
2012-01-16 12:39               ` Tomi Ollila
2012-01-21 12:21               ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).