From: Austin Clements <amdragon@MIT.EDU>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/5] emacs: Utilities to manage asynchronous notmuch
	processes
In-Reply-To: <m24ndjas8o.fsf@guru.guru-group.fi>
References: <1369934016-22308-1-git-send-email-amdragon@mit.edu>
	<1369934016-22308-3-git-send-email-amdragon@mit.edu>
	<m24ndjas8o.fsf@guru.guru-group.fi>
User-Agent: Notmuch/0.15.2+133~gf9ff9fd (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Fri, 31 May 2013 19:41:30 -0400
Message-ID: <87zjvahg45.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Fri, 31 May 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> 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".

Done.

>> +		      "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

I like it.  Done.

> 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))
>> +