From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/7] emacs: Use unified error handling in
	notmuch-call-notmuch-process
In-Reply-To: <876243hdcc.fsf@qmul.ac.uk>
References: <1355548513-10085-1-git-send-email-amdragon@mit.edu>
	<1355548513-10085-3-git-send-email-amdragon@mit.edu>
	<876243hdcc.fsf@qmul.ac.uk>
User-Agent: Notmuch/0.14+184~gc26cc51 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Sat, 15 Dec 2012 14:51:01 -0500
Message-ID: <874njnxffe.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Sat, 15 Dec 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> On Sat, 15 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
>> This makes notmuch-call-notmuch-process use the unified CLI error
>> handling, which basically refines the error handling this function
>> already did.
>> ---
>>  emacs/notmuch.el |   15 ++++-----------
>>  1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index f9454d8..e25b54e 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -538,17 +538,10 @@ If BARE is set then do not prefix with \"thread:\""
>>  
>>  Output from the process will be presented to the user as an error
>>  and will also appear in a buffer named \"*Notmuch errors*\"."
>
> This comment looks like it is not true (but wasn't true before
> either). I think output will only be presented to the user if notmuch
> exits with a non-zero status?

Fixed.

>> -  (let ((error-buffer (get-buffer-create "*Notmuch errors*")))
>> -    (with-current-buffer error-buffer
>> -	(erase-buffer))
>> -    (if (eq (apply 'call-process notmuch-command nil error-buffer nil args) 0)
>> -	(point)
>> -      (progn
>> -	(with-current-buffer error-buffer
>> -	  (let ((beg (point-min))
>> -		(end (- (point-max) 1)))
>> -	    (error (buffer-substring beg end))
>> -	    ))))))
>> +  (with-temp-buffer
>> +    (let ((status (apply #'call-process notmuch-command nil t nil args)))
>> +      (notmuch-check-exit-status status (cons notmuch-command args)
>> +				 (buffer-string)))))
>>  
>
> Would it be worth separating stderr and stdout here? (Quite plausibly it
> isn't.)

Actually I think it's better to not separate stdout and stderr when we
can avoid it, since stdout may give context for what's on stderr.  When
we're trying to interpret stdout it's important to separate it, but
that's not the case here.

> Best wishes
>
> Mark
>
>
>>  (defun notmuch-search-set-tags (tags &optional pos)
>>    (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))
>> -- 
>> 1.7.10.4