Re: [PATCH 2/5] emacs: Utilities to manage asynchronous notmuch processes

[Tomi Ollila <tomi.ollila@iki.fi>]

On Tue, May 21 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> Hi
>
> I am working my way through this set. 
>
> On Sat, 18 May 2013, Austin Clements <amdragon@MIT.EDU> wrote:
>> 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 |   73 +++++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 69 insertions(+), 4 deletions(-)
>>
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index 59b1ce3..a543471 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -383,18 +383,21 @@ signaled error.  This function does not return."
>>    (error "%s" (concat msg (when extra
>>  			    " (see *Notmuch errors* for more details)"))))
>>  
>> -(defun notmuch-check-async-exit-status (proc msg)
>> +(defun notmuch-check-async-exit-status (proc msg &optional command err-file)
>>    "If PROC exited abnormally, pop up an error buffer and signal an error.
>>  
>>  This is a wrapper around `notmuch-check-exit-status' for
>>  asynchronous process sentinels.  PROC and MSG must be the
>> -arguments passed to the sentinel."
>> +arguments passed to the sentinel.  COMMAND and ERR-FILE, if
>> +provided, are passed to `notmuch-check-exit-status'.  If COMMAND
>> +is not provided, it is taken from `process-command'."
>>    (let ((exit-status
>>  	 (case (process-status proc)
>>  	   ((exit) (process-exit-status proc))
>>  	   ((signal) msg))))
>>      (when exit-status
>> -      (notmuch-check-exit-status exit-status (process-command proc)))))
>> +      (notmuch-check-exit-status exit-status (or command (process-command proc))
>> +				 nil err-file))))
>>  
>>  (defun notmuch-check-exit-status (exit-status command &optional output err-file)
>>    "If EXIT-STATUS is non-zero, pop up an error buffer and signal an error.
>> @@ -448,7 +451,7 @@ You may need to restart Emacs or upgrade your notmuch package."))
>>  	))))
>>  
>>  (defun notmuch-call-notmuch-json (&rest args)
>> -  "Invoke `notmuch-command' with `args' and return the parsed JSON output.
>> +  "Invoke `notmuch-command' with ARGS and return the parsed JSON output.
>>  
>>  The returned output will represent objects using property lists
>>  and arrays as lists.  If notmuch exits with a non-zero status,
>
> I think I would prefer that this patch is split here. The stuff above is
> "obviously correct" and could go in independently of the rest.
>  
>> @@ -469,6 +472,68 @@ an error."
>>  	      (json-read)))
>>  	(delete-file err-file)))))
>>  
>> +(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)
>> +	 (proc (apply #'start-process name buffer
>> +		      "sh" "-c"
>> +		      "ERR=\"$1\"; shift; exec \"$0\" \"$@\" 2>\"$ERR\""
>> +		      notmuch-command err-file args)))
>
> I would like some other people to look at this carefully as I won't spot
> errors in quoting or shell side effects or whether PATH is the same as
> emacs would use etc.

The variable expansion for sell works as desired, but PATH question is
interesting:

I tried:

setq exec-path (cons "/bii" exec-path))
(shell-command "echo $PATH")

and "/bii" was not in the head of the output the shell-command executed
(which is interesting... found this text:

    The value of “PATH” is used by emacs when you are running a shell in
    emacs, similar to when you are using a shell in a terminal.

    The exec-path is used by emacs itself to find programs it needs for its
    features, such as spell checking, file compression, compiling, grep,
    diff, etc.

 (in http://ergoemacs.org/emacs/emacs_env_var_paths.htm )

and then:

  Generally, you should not modify exec-path directly. Instead, ensure that
  your PATH environment variable is set appropriately before starting
  Emacs. Trying to modify exec-path independently of PATH can lead to
  confusing results. 

 (in http://www.gnu.org/software/emacs/manual/html_node/elisp/Subprocess-Creation.html))

... the --stderr option in the RFC patch I just sent 
( id:1369161750-12342-1-git-send-email-tomi.ollila@iki.fi ) could solve
quite a few problems here :D

Tomi