Re: [PATCH] emacs: support "notmuch new" as a notmuch-poll-script

[Dmitry Kurochkin <dmitry.kurochkin@gmail.com>]

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