On Thu, 03 Jan 2013, Mark Walters <markwalters1009@gmail.com> wrote: > On Thu, 03 Jan 2013, Austin Clements <amdragon@MIT.EDU> wrote: >> We recently switched to popping up a buffer to report CLI errors, but >> this was too intrusive, especially for transient errors and especially >> since we made fewer things ignore errors. This patch changes this to >> display a basic error message in the minibuffer (using Emacs' usual >> error handling path) and, if there are additional details, to log >> these to a separate error buffer and reference the error buffer from >> the minibuffer message. This is more in line with how Emacs typically >> handles errors, but makes the details available to the user without >> flooding them with the details. >> >> Given this split, we pare down the basic message and make it more >> user-friendly, and also make the verbose message even more detailed >> (and more debugging-oriented). >> --- >> >> This version fixes two hard-coded paths in the tests. > > > This version looks good to me but I have one query we may like to > consider. > > At the moment notmuch-call-notmuch-process returns the stderr mixed with > stdout and we might like to separate that out (particularly as the error > message lists stderr and stdout separately and in this case it all gets > called stdout). This sounds like a good idea to me, though I'd rather do it as a separate patch. Your patch looks good to me (modulo commit message, obviously). Care to roll an official patch? As I mentioned elsewhere, there are several direct calls to notmuch that don't go through any of this error handling (for example, `notmuch-call-notmuch-process' is only used in 'notmuch-tag'). It would be great if Someone (TM) cleaned this up. > I attach a patch that does this: I am not really familiar with this so I > just took Austin's code from notmuch-call-notmuch-json. > > Austin: obviously feel free to fold this into your patch if you think > appropriate. > > Best wishes > > Mark > > From b73395c8efb57111bd4de281797004747de6c2ed Mon Sep 17 00:00:00 2001 > From: Mark Walters <markwalters1009@gmail.com> > Date: Thu, 3 Jan 2013 22:25:02 +0000 > Subject: [PATCH] tweak notmuch-call-notmuch-process > > --- > emacs/notmuch.el | 10 +++++++--- > 1 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index c98a4fe..4f7ee2c 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -540,9 +540,13 @@ If notmuch exits with a non-zero status, output from the process > will appear in a buffer named \"*Notmuch errors*\" and an error > will be signaled." > (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))))) > + (let ((err-file (make-temp-file "nmerr"))) > + (unwind-protect > + (let ((status (apply #'call-process > + notmuch-command nil (list t err-file) nil args))) > + (notmuch-check-exit-status status (cons notmuch-command args) > + (buffer-string) err-file)) > + (delete-file err-file))))) > > (defun notmuch-search-set-tags (tags &optional pos) > (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags))) > -- > 1.7.9.1