From: Tomi Ollila <tomi.ollila@iki.fi>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/5] emacs: Utilities to manage asynchronous notmuch
	processes
In-Reply-To: <1369934016-22308-3-git-send-email-amdragon@mit.edu>
References: <1369934016-22308-1-git-send-email-amdragon@mit.edu>
	<1369934016-22308-3-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.15.2+115~g12cf6af (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Fri, 31 May 2013 22:01:27 +0300
Message-ID: <m24ndjas8o.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

Austin Clements <amdragon@MIT.EDU> writes:

> This provides a new notmuch-lib utility to start an asynchronous
> notmuch process that handles redirecting of stderr and checking of the
> exit status.  This is similar to `notmuch-call-notmuch-json', but for
> asynchronous processes (and it leaves output processing to the
> caller).
> ---
>  emacs/notmuch-lib.el |   77 +++++++++++++++++++++++++++++++++++++++++++++++---

...

> +(defun notmuch-start-notmuch (name buffer sentinel &rest args)
> +  "Start and return an asynchronous notmuch command.
> +
> +This starts and returns an asynchronous process running
> +`notmuch-command' with ARGS.  The exit status is checked via
> +`notmuch-check-async-exit-status'.  Output written to stderr is
> +redirected and displayed when the process exits (even if the
> +process exits successfully).  NAME and BUFFER are the same as in
> +`start-process'.  SENTINEL is a process sentinel function to call
> +when the process exits, or nil for none.  The caller must *not*
> +invoke `set-process-sentinel' directly on the returned process,
> +as that will interfere with the handling of stderr and the exit
> +status."
> +
> +  ;; There is no way (as of Emacs 24.3) to capture stdout and stderr
> +  ;; separately for asynchronous processes, or even to redirect stderr
> +  ;; to a file, so we use a trivial shell wrapper to send stderr to a
> +  ;; temporary file and clean things up in the sentinel.
> +  (let* ((err-file (make-temp-file "nmerr"))
> +	 ;; Use a pipe
> +	 (process-connection-type nil)
> +	 ;; Find notmuch using Emacs' `exec-path'
> +	 (command (or (executable-find notmuch-command)
> +		      (error "command not found: %s" notmuch-command)))
> +	 (proc (apply #'start-process name buffer
> +		      "sh" "-c"

I'd suggest "/bin/sh".


> +		      "ERR=\"$1\"; shift; exec \"$0\" \"$@\" 2>\"$ERR\""

An alternative to the above ...

"exec 2>\"$1\"; shift; exec \"$0\" \"$@\""

... which one is better is a matter of quar^H^H^H^H^H taste :D

Everything else in this patch series looks good to me (as far as
I understood -- test passed and works as expected).


Tomi

> +		      command err-file args)))
> +    (process-put proc 'err-file err-file)
> +    (process-put proc 'sub-sentinel sentinel)
> +    (process-put proc 'real-command (cons notmuch-command args))
> +    (set-process-sentinel proc #'notmuch-start-notmuch-sentinel)
> +    proc))
> +