[PATCH v3] emacs: Use the minibuffer for CLI error reporting
Tomi Ollila
tomi.ollila at iki.fi
Sat Jan 5 13:33:08 PST 2013
On Thu, Jan 03 2013, Austin Clements <amdragon at 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).
> ---
LGTM.
Mark's suggestion could go to a separate patch -- this one
doesn't (seem to?) make things worse compared what it is before
applying.
If things are currently irritatively borken due to notmuch emitting
more errors then Someone(tm) should make a proper patch following
Mark's suggestion.
Tomi
>
> This version fixes two hard-coded paths in the tests.
>
> emacs/notmuch-lib.el | 92 ++++++++++++++++++++++++++++----------------------
> emacs/notmuch.el | 9 +++--
> test/emacs | 19 ++++++++---
> test/emacs-show | 26 ++++++++++----
> 4 files changed, 90 insertions(+), 56 deletions(-)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 77a591d..d78bcf8 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -316,23 +316,28 @@ string), a property list of face attributes, or a list of these."
> (put-text-property pos next 'face (cons face cur))
> (setq pos next)))))
>
> -(defun notmuch-pop-up-error (msg)
> - "Pop up an error buffer displaying MSG.
> -
> -This will accumulate error messages in the errors buffer until
> -the user dismisses it."
> -
> - (let ((buf (get-buffer-create "*Notmuch errors*")))
> - (with-current-buffer buf
> - (view-mode-enter nil #'kill-buffer)
> - (let ((inhibit-read-only t))
> - (goto-char (point-max))
> - (unless (bobp)
> - (insert "\n"))
> - (insert msg)
> +(defun notmuch-logged-error (msg &optional extra)
> + "Log MSG and EXTRA to *Notmuch errors* and signal MSG.
> +
> +This logs MSG and EXTRA to the *Notmuch errors* buffer and
> +signals MSG as an error. If EXTRA is non-nil, text referring the
> +user to the *Notmuch errors* buffer will be appended to the
> +signaled error. This function does not return."
> +
> + (with-current-buffer (get-buffer-create "*Notmuch errors*")
> + (goto-char (point-max))
> + (unless (bobp)
> + (newline))
> + (save-excursion
> + (insert "[" (current-time-string) "]\n" msg)
> + (unless (bolp)
> + (newline))
> + (when extra
> + (insert extra)
> (unless (bolp)
> - (insert "\n"))))
> - (pop-to-buffer buf)))
> + (newline)))))
> + (error "%s" (concat msg (when extra
> + " (see *Notmuch errors* for more details)"))))
>
> (defun notmuch-check-async-exit-status (proc msg)
> "If PROC exited abnormally, pop up an error buffer and signal an error.
> @@ -363,35 +368,40 @@ contents of ERR-FILE will be included in the error message."
> (cond
> ((eq exit-status 0) t)
> ((eq exit-status 20)
> - (notmuch-pop-up-error "Error: Version mismatch.
> + (notmuch-logged-error "notmuch CLI version mismatch
> Emacs requested an older output format than supported by the notmuch CLI.
> -You may need to restart Emacs or upgrade your notmuch Emacs package.")
> - (error "notmuch CLI version mismatch"))
> +You may need to restart Emacs or upgrade your notmuch Emacs package."))
> ((eq exit-status 21)
> - (notmuch-pop-up-error "Error: Version mismatch.
> + (notmuch-logged-error "notmuch CLI version mismatch
> Emacs requested a newer output format than supported by the notmuch CLI.
> -You may need to restart Emacs or upgrade your notmuch package.")
> - (error "notmuch CLI version mismatch"))
> +You may need to restart Emacs or upgrade your notmuch package."))
> (t
> - (notmuch-pop-up-error
> - (concat
> - (format "Error invoking notmuch. %s exited with %s%s.\n"
> - (mapconcat #'identity command " ")
> - ;; Signal strings look like "Terminated", hence the
> - ;; colon.
> - (if (integerp exit-status) "status " "signal: ")
> - exit-status)
> - (when err-file
> - (concat "Error:\n"
> - (with-temp-buffer
> - (insert-file-contents err-file)
> - (if (eobp)
> - "(no error output)\n"
> - (buffer-string)))))
> - (when (and output (not (equal output "")))
> - (format "Output:\n%s" output))))
> - ;; Mimic `process-lines'
> - (error "%s exited with status %s" (car command) exit-status))))
> + (let* ((err (when err-file
> + (with-temp-buffer
> + (insert-file-contents err-file)
> + (unless (eobp)
> + (buffer-string)))))
> + (extra
> + (concat
> + "command: " (mapconcat #'shell-quote-argument command " ") "\n"
> + (if (integerp exit-status)
> + (format "exit status: %s\n" exit-status)
> + (format "exit signal: %s\n" exit-status))
> + (when err
> + (concat "stderr:\n" err))
> + (when output
> + (concat "stdout:\n" output)))))
> + (if err
> + ;; We have an error message straight from the CLI.
> + (notmuch-logged-error
> + (replace-regexp-in-string "\\s $" "" err) extra)
> + ;; We only have combined output from the CLI; don't inundate
> + ;; the user with it. Mimic `process-lines'.
> + (notmuch-logged-error (format "%s exited with status %s"
> + (car command) exit-status)
> + extra))
> + ;; `notmuch-logged-error' does not return.
> + ))))
>
> (defun notmuch-call-notmuch-json (&rest args)
> "Invoke `notmuch-command' with `args' and return the parsed JSON output.
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 63387a2..c98a4fe 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -654,11 +654,14 @@ of the result."
> ;; showing the search buffer
> (when (or (= exit-status 20) (= exit-status 21))
> (kill-buffer))
> - (condition-case nil
> + (condition-case err
> (notmuch-check-async-exit-status proc msg)
> ;; Suppress the error signal since strange
> - ;; things happen if a sentinel signals.
> - (error (throw 'return nil)))
> + ;; things happen if a sentinel signals. Mimic
> + ;; the top-level's handling of error messages.
> + (error
> + (message "%s" (second err))
> + (throw 'return nil)))
> (if (and atbob
> (not (string= notmuch-search-target-thread "found")))
> (set 'never-found-target-thread t)))))
> diff --git a/test/emacs b/test/emacs
> index 6b18968..f033bdf 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -862,18 +862,27 @@ exit 1
> EOF
> chmod a+x notmuch_fail
> test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
> + (with-current-buffer \"*Messages*\" (erase-buffer))
> (notmuch-search \"tag:inbox\")
> (notmuch-test-wait)
> - (test-output)
> + (with-current-buffer \"*Messages*\"
> + (test-output \"MESSAGES\"))
> (with-current-buffer \"*Notmuch errors*\"
> - (test-output \"ERROR\")))"
> -test_expect_equal "$(cat OUTPUT ERROR)" "\
> + (test-output \"ERROR\"))
> + (test-output))"
> +sed -i -e 's/^\[.*\]$/[XXX]/' ERROR
> +test_expect_equal "$(cat OUTPUT; echo ---; cat MESSAGES; echo ---; cat ERROR)" "\
> Error: Unexpected output from notmuch search:
> This is output
> Error: Unexpected output from notmuch search:
> This is an error
> End of search results.
> -Error invoking notmuch. $PWD/notmuch_fail search --format=json --format-version=1 --sort=newest-first tag:inbox exited with status 1."
> -
> +---
> +$PWD/notmuch_fail exited with status 1 (see *Notmuch errors* for more details)
> +---
> +[XXX]
> +$PWD/notmuch_fail exited with status 1
> +command: $PWD/notmuch_fail search --format\=json --format-version\=1 --sort\=newest-first tag\:inbox
> +exit status: 1"
>
> test_done
> diff --git a/test/emacs-show b/test/emacs-show
> index ebf530b..9f2ccb0 100755
> --- a/test/emacs-show
> +++ b/test/emacs-show
> @@ -172,16 +172,28 @@ exit 1
> EOF
> chmod a+x notmuch_fail
> test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
> - (ignore-errors (notmuch-show \"*\"))
> + (with-current-buffer \"*Messages*\" (erase-buffer))
> + (condition-case err
> + (notmuch-show \"*\")
> + (error (message \"%s\" (second err))))
> (notmuch-test-wait)
> - (test-output)
> + (with-current-buffer \"*Messages*\"
> + (test-output \"MESSAGES\"))
> (with-current-buffer \"*Notmuch errors*\"
> - (test-output \"ERROR\")))"
> -test_expect_equal "$(cat OUTPUT ERROR)" "\
> -Error invoking notmuch. $PWD/notmuch_fail show --format=json --format-version=1 --exclude=false ' * ' exited with status 1.
> -Error:
> + (test-output \"ERROR\"))
> + (test-output))"
> +sed -i -e 's/^\[.*\]$/[XXX]/' ERROR
> +test_expect_equal "$(cat OUTPUT; echo ---; cat MESSAGES; echo ---; cat ERROR)" "\
> +---
> +This is an error (see *Notmuch errors* for more details)
> +---
> +[XXX]
> This is an error
> -Output:
> +command: $PWD/notmuch_fail show --format\\=json --format-version\\=1 --exclude\\=false \\' \\* \\'
> +exit status: 1
> +stderr:
> +This is an error
> +stdout:
> This is output"
>
>
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list