* [PATCH v2 0/7] Improve Emacs CLI error handling
@ 2012-12-15 20:04 Austin Clements
2012-12-15 20:04 ` [PATCH v2 1/7] emacs: Centralize notmuch command " Austin Clements
` (8 more replies)
0 siblings, 9 replies; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
This obsoletes id:1355548513-10085-1-git-send-email-amdragon@mit.edu
and fixes the things Mark and Tomi commented on. The interdiff is
below.
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index cf61635..8f84087 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -326,10 +326,12 @@ the user dismisses it."
(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)
(unless (bolp)
- (insert "\n"))
- (goto-char (point-min))))
+ (insert "\n"))))
(pop-to-buffer buf)))
(defun notmuch-check-async-exit-status (proc msg)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index c20de13..b0fd387 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -536,8 +536,9 @@ If BARE is set then do not prefix with \"thread:\""
(defun notmuch-call-notmuch-process (&rest args)
"Synchronously invoke \"notmuch\" with the given list of arguments.
-Output from the process will be presented to the user as an error
-and will also appear in a buffer named \"*Notmuch errors*\"."
+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)
@@ -649,7 +650,7 @@ of the result."
(insert "Incomplete search results (search process was killed).\n"))
(when (eq status 'exit)
(insert "End of search results.\n")
- (condition-case err
+ (condition-case nil
(notmuch-check-async-exit-status proc msg)
;; Suppress the error signal since strange
;; things happen if a sentinel signals.
diff --git a/test/emacs b/test/emacs
index 88b062c..5067d67 100755
--- a/test/emacs
+++ b/test/emacs
@@ -873,7 +873,7 @@ This is output
Error: Unexpected output from notmuch search:
This is an error
End of search results.
-Error invoking notmuch. /tmp/nmtest/tmp.emacs/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1."
+Error invoking notmuch. $PWD/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1."
test_done
diff --git a/test/emacs-show b/test/emacs-show
index c67c6a4..40fab61 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -178,7 +178,7 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
(with-current-buffer \"*Notmuch errors*\"
(test-output \"ERROR\")))"
test_expect_equal "$(cat OUTPUT ERROR)" "\
-Error invoking notmuch. /tmp/nmtest/tmp.emacs-show/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1.
+Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1.
Error:
This is an error
Output:
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 1/7] emacs: Centralize notmuch command error handling
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
@ 2012-12-15 20:04 ` Austin Clements
2012-12-16 21:39 ` David Bremner
2012-12-15 20:04 ` [PATCH v2 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process Austin Clements
` (7 subsequent siblings)
8 siblings, 1 reply; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
This provides library functions for unified handling of errors from
the notmuch CLI. Follow-up patches will convert some scattered error
handling to use this and add error handling where we currently ignore
errors.
---
emacs/notmuch-lib.el | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 9c4ee71..f757ce7 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -316,6 +316,61 @@ 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)
+ (unless (bolp)
+ (insert "\n"))))
+ (pop-to-buffer buf)))
+
+(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.
+
+If EXIT-STATUS is non-zero, pop up a notmuch error buffer
+describing the error and signal an Elisp error. EXIT-STATUS must
+be a number indicating the exit status code of a process or a
+string describing the signal that terminated the process (such as
+returned by `call-process'). COMMAND must be a list giving the
+command and its arguments. OUTPUT, if provided, is a string
+giving the output of command. ERR-FILE, if provided, is the name
+of a file containing the error output of command. OUTPUT and the
+contents of ERR-FILE will be included in the error message."
+
+ ;; This is implemented as a cond to make it easy to expand.
+ (cond
+ ((eq exit-status 0) t)
+ (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))))
+
;; Compatibility functions for versions of emacs before emacs 23.
;;
;; Both functions here were copied from emacs 23 with the following copyright:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
2012-12-15 20:04 ` [PATCH v2 1/7] emacs: Centralize notmuch command " Austin Clements
@ 2012-12-15 20:04 ` Austin Clements
2012-12-15 20:04 ` [PATCH v2 3/7] emacs: Factor out synchronous notmuch JSON invocations Austin Clements
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
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 | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f9454d8..9da8df4 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -536,19 +536,13 @@ If BARE is set then do not prefix with \"thread:\""
(defun notmuch-call-notmuch-process (&rest args)
"Synchronously invoke \"notmuch\" with the given list of arguments.
-Output from the process will be presented to the user as an error
-and will also appear in a buffer named \"*Notmuch errors*\"."
- (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))
- ))))))
+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)))))
(defun notmuch-search-set-tags (tags &optional pos)
(let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/7] emacs: Factor out synchronous notmuch JSON invocations
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
2012-12-15 20:04 ` [PATCH v2 1/7] emacs: Centralize notmuch command " Austin Clements
2012-12-15 20:04 ` [PATCH v2 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process Austin Clements
@ 2012-12-15 20:04 ` Austin Clements
2012-12-15 20:04 ` [PATCH v2 4/7] emacs: Improve error handling for notmuch-call-notmuch-json Austin Clements
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
Previously this code was duplicated between show and reply. This
factors out synchronously invoking notmuch and parsing the output as
JSON.
---
emacs/notmuch-lib.el | 14 ++++++++++++++
emacs/notmuch-mua.el | 8 +-------
emacs/notmuch-query.el | 11 ++---------
3 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index f757ce7..f534770 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -371,6 +371,20 @@ contents of ERR-FILE will be included in the error message."
;; Mimic `process-lines'
(error "%s exited with status %s" (car command) exit-status))))
+(defun notmuch-call-notmuch-json (&rest args)
+ "Invoke `notmuch-command' with `args' and return the parsed JSON output.
+
+The returned output will represent objects using property lists
+and arrays as lists."
+
+ (with-temp-buffer
+ (apply #'call-process notmuch-command nil (list t nil) nil args)
+ (goto-char (point-min))
+ (let ((json-object-type 'plist)
+ (json-array-type 'list)
+ (json-false 'nil))
+ (json-read))))
+
;; Compatibility functions for versions of emacs before emacs 23.
;;
;; Both functions here were copied from emacs 23 with the following copyright:
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 408b49e..ac2d29e 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -158,13 +158,7 @@ list."
(setq args (append args (list query-string)))
;; Get the reply object as JSON, and parse it into an elisp object.
- (with-temp-buffer
- (apply 'call-process (append (list notmuch-command nil (list t nil) nil) args))
- (goto-char (point-min))
- (let ((json-object-type 'plist)
- (json-array-type 'list)
- (json-false 'nil))
- (setq reply (json-read))))
+ (setq reply (apply #'notmuch-call-notmuch-json args))
;; Extract the original message to simplify the following code.
(setq original (plist-get reply :original))
diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
index d66baea..e7e3520 100644
--- a/emacs/notmuch-query.el
+++ b/emacs/notmuch-query.el
@@ -29,18 +29,11 @@ A thread is a forest or list of trees. A tree is a two element
list where the first element is a message, and the second element
is a possibly empty forest of replies.
"
- (let ((args '("show" "--format=json"))
- (json-object-type 'plist)
- (json-array-type 'list)
- (json-false 'nil))
+ (let ((args '("show" "--format=json")))
(if notmuch-show-process-crypto
(setq args (append args '("--decrypt"))))
(setq args (append args search-terms))
- (with-temp-buffer
- (progn
- (apply 'call-process (append (list notmuch-command nil (list t nil) nil) args))
- (goto-char (point-min))
- (json-read)))))
+ (apply #'notmuch-call-notmuch-json args)))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Mapping functions across collections of messages.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/7] emacs: Improve error handling for notmuch-call-notmuch-json
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
` (2 preceding siblings ...)
2012-12-15 20:04 ` [PATCH v2 3/7] emacs: Factor out synchronous notmuch JSON invocations Austin Clements
@ 2012-12-15 20:04 ` Austin Clements
2012-12-15 20:04 ` [PATCH v2 5/7] test: Test show's handling of subprocess errors Austin Clements
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
This checks for non-zero exit status from JSON CLI calls and pops up
an error buffer with stderr and stdout. A consequence of this is that
show and reply now handle errors, rather than ignoring them.
---
emacs/notmuch-lib.el | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index f534770..7f7022a 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -375,15 +375,23 @@ contents of ERR-FILE will be included in the error message."
"Invoke `notmuch-command' with `args' and return the parsed JSON output.
The returned output will represent objects using property lists
-and arrays as lists."
+and arrays as lists. If notmuch exits with a non-zero status,
+this will pop up a buffer containing notmuch's output and signal
+an error."
(with-temp-buffer
- (apply #'call-process notmuch-command nil (list t nil) nil args)
- (goto-char (point-min))
- (let ((json-object-type 'plist)
- (json-array-type 'list)
- (json-false 'nil))
- (json-read))))
+ (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)
+ (goto-char (point-min))
+ (let ((json-object-type 'plist)
+ (json-array-type 'list)
+ (json-false 'nil))
+ (json-read)))
+ (delete-file err-file)))))
;; Compatibility functions for versions of emacs before emacs 23.
;;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/7] test: Test show's handling of subprocess errors
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
` (3 preceding siblings ...)
2012-12-15 20:04 ` [PATCH v2 4/7] emacs: Improve error handling for notmuch-call-notmuch-json Austin Clements
@ 2012-12-15 20:04 ` Austin Clements
2012-12-15 20:04 ` [PATCH v2 6/7] emacs: Use unified error handling in search Austin Clements
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
---
test/emacs-show | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/test/emacs-show b/test/emacs-show
index b670abf..40fab61 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -163,4 +163,26 @@ EOF
test_expect_equal_file OUTPUT EXPECTED
+test_begin_subtest "Show handles subprocess errors"
+cat > notmuch_fail <<EOF
+#!/bin/sh
+echo This is output
+echo This is an error >&2
+exit 1
+EOF
+chmod a+x notmuch_fail
+test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
+ (ignore-errors (notmuch-show \"*\"))
+ (notmuch-test-wait)
+ (test-output)
+ (with-current-buffer \"*Notmuch errors*\"
+ (test-output \"ERROR\")))"
+test_expect_equal "$(cat OUTPUT ERROR)" "\
+Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1.
+Error:
+This is an error
+Output:
+This is output"
+
+
test_done
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 6/7] emacs: Use unified error handling in search
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
` (4 preceding siblings ...)
2012-12-15 20:04 ` [PATCH v2 5/7] test: Test show's handling of subprocess errors Austin Clements
@ 2012-12-15 20:04 ` Austin Clements
2012-12-15 20:04 ` [PATCH v2 7/7] test: Test search's handling of subprocess errors Austin Clements
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
This slightly changes the output of an existing test since we now
report non-zero exits with a pop-up buffer instead of at the end of
the search results.
---
emacs/notmuch-lib.el | 13 +++++++++++++
emacs/notmuch.el | 13 ++++++++-----
test/emacs-large-search-buffer | 2 +-
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 7f7022a..8f84087 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -334,6 +334,19 @@ the user dismisses it."
(insert "\n"))))
(pop-to-buffer buf)))
+(defun notmuch-check-async-exit-status (proc msg)
+ "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."
+ (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)))))
+
(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.
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9da8df4..b0fd387 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -638,6 +638,7 @@ of the result."
(exit-status (process-exit-status proc))
(never-found-target-thread nil))
(when (memq status '(exit signal))
+ (catch 'return
(kill-buffer (process-get proc 'parse-buf))
(if (buffer-live-p buffer)
(with-current-buffer buffer
@@ -648,17 +649,19 @@ of the result."
(if (eq status 'signal)
(insert "Incomplete search results (search process was killed).\n"))
(when (eq status 'exit)
- (insert "End of search results.")
- (unless (= exit-status 0)
- (insert (format " (process returned %d)" exit-status)))
- (insert "\n")
+ (insert "End of search results.\n")
+ (condition-case nil
+ (notmuch-check-async-exit-status proc msg)
+ ;; Suppress the error signal since strange
+ ;; things happen if a sentinel signals.
+ (error (throw 'return nil)))
(if (and atbob
(not (string= notmuch-search-target-thread "found")))
(set 'never-found-target-thread t)))))
(when (and never-found-target-thread
notmuch-search-target-line)
(goto-char (point-min))
- (forward-line (1- notmuch-search-target-line))))))))
+ (forward-line (1- notmuch-search-target-line)))))))))
(defcustom notmuch-search-line-faces '(("unread" :weight bold)
("flagged" :foreground "blue"))
diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
index 678328d..9dcbef5 100755
--- a/test/emacs-large-search-buffer
+++ b/test/emacs-large-search-buffer
@@ -36,7 +36,7 @@ test_emacs '(notmuch-search "--this-option-does-not-exist")
cat <<EOF >EXPECTED
Error: Unexpected output from notmuch search:
Unrecognized option: --this-option-does-not-exist
-End of search results. (process returned 1)
+End of search results.
EOF
test_expect_equal_file OUTPUT EXPECTED
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 7/7] test: Test search's handling of subprocess errors
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
` (5 preceding siblings ...)
2012-12-15 20:04 ` [PATCH v2 6/7] emacs: Use unified error handling in search Austin Clements
@ 2012-12-15 20:04 ` Austin Clements
2012-12-16 1:19 ` [PATCH v2 0/7] Improve Emacs CLI error handling Mark Walters
2012-12-16 3:08 ` Tomi Ollila
8 siblings, 0 replies; 11+ messages in thread
From: Austin Clements @ 2012-12-15 20:04 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
---
test/emacs | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/test/emacs b/test/emacs
index 5403930..5067d67 100755
--- a/test/emacs
+++ b/test/emacs
@@ -853,4 +853,27 @@ test_expect_success "Rendering HTML mail with images" \
'cat OUTPUT && grep -q smiley OUTPUT'
+test_begin_subtest "Search handles subprocess errors"
+cat > notmuch_fail <<EOF
+#!/bin/sh
+echo This is output
+echo This is an error >&2
+exit 1
+EOF
+chmod a+x notmuch_fail
+test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
+ (notmuch-search \"tag:inbox\")
+ (notmuch-test-wait)
+ (test-output)
+ (with-current-buffer \"*Notmuch errors*\"
+ (test-output \"ERROR\")))"
+test_expect_equal "$(cat OUTPUT 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 --sort=newest-first tag:inbox exited with status 1."
+
+
test_done
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/7] Improve Emacs CLI error handling
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
` (6 preceding siblings ...)
2012-12-15 20:04 ` [PATCH v2 7/7] test: Test search's handling of subprocess errors Austin Clements
@ 2012-12-16 1:19 ` Mark Walters
2012-12-16 3:08 ` Tomi Ollila
8 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2012-12-16 1:19 UTC (permalink / raw)
To: Austin Clements, notmuch; +Cc: tomi.ollila
This series looks good to me. Just one possible thought: would it be
worth time-stamping the errors (mostly in case the user does not quit
the error buffer). But +1 anyway
Best wishes
Mark
On Sat, 15 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> This obsoletes id:1355548513-10085-1-git-send-email-amdragon@mit.edu
> and fixes the things Mark and Tomi commented on. The interdiff is
> below.
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index cf61635..8f84087 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -326,10 +326,12 @@ the user dismisses it."
> (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)
> (unless (bolp)
> - (insert "\n"))
> - (goto-char (point-min))))
> + (insert "\n"))))
> (pop-to-buffer buf)))
>
> (defun notmuch-check-async-exit-status (proc msg)
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index c20de13..b0fd387 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -536,8 +536,9 @@ If BARE is set then do not prefix with \"thread:\""
> (defun notmuch-call-notmuch-process (&rest args)
> "Synchronously invoke \"notmuch\" with the given list of arguments.
>
> -Output from the process will be presented to the user as an error
> -and will also appear in a buffer named \"*Notmuch errors*\"."
> +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)
> @@ -649,7 +650,7 @@ of the result."
> (insert "Incomplete search results (search process was killed).\n"))
> (when (eq status 'exit)
> (insert "End of search results.\n")
> - (condition-case err
> + (condition-case nil
> (notmuch-check-async-exit-status proc msg)
> ;; Suppress the error signal since strange
> ;; things happen if a sentinel signals.
> diff --git a/test/emacs b/test/emacs
> index 88b062c..5067d67 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -873,7 +873,7 @@ This is output
> Error: Unexpected output from notmuch search:
> This is an error
> End of search results.
> -Error invoking notmuch. /tmp/nmtest/tmp.emacs/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1."
> +Error invoking notmuch. $PWD/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1."
>
>
> test_done
> diff --git a/test/emacs-show b/test/emacs-show
> index c67c6a4..40fab61 100755
> --- a/test/emacs-show
> +++ b/test/emacs-show
> @@ -178,7 +178,7 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
> (with-current-buffer \"*Notmuch errors*\"
> (test-output \"ERROR\")))"
> test_expect_equal "$(cat OUTPUT ERROR)" "\
> -Error invoking notmuch. /tmp/nmtest/tmp.emacs-show/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1.
> +Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1.
> Error:
> This is an error
> Output:
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/7] Improve Emacs CLI error handling
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
` (7 preceding siblings ...)
2012-12-16 1:19 ` [PATCH v2 0/7] Improve Emacs CLI error handling Mark Walters
@ 2012-12-16 3:08 ` Tomi Ollila
8 siblings, 0 replies; 11+ messages in thread
From: Tomi Ollila @ 2012-12-16 3:08 UTC (permalink / raw)
To: Austin Clements, notmuch
On Sat, Dec 15 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> This obsoletes id:1355548513-10085-1-git-send-email-amdragon@mit.edu
> and fixes the things Mark and Tomi commented on. The interdiff is
> below.
Ok, new error messages appear at the end. Good.
+1
Tomi
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index cf61635..8f84087 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -326,10 +326,12 @@ the user dismisses it."
> (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)
> (unless (bolp)
> - (insert "\n"))
> - (goto-char (point-min))))
> + (insert "\n"))))
> (pop-to-buffer buf)))
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/7] emacs: Centralize notmuch command error handling
2012-12-15 20:04 ` [PATCH v2 1/7] emacs: Centralize notmuch command " Austin Clements
@ 2012-12-16 21:39 ` David Bremner
0 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2012-12-16 21:39 UTC (permalink / raw)
To: Austin Clements, notmuch; +Cc: tomi.ollila
Austin Clements <amdragon@MIT.EDU> writes:
> This provides library functions for unified handling of errors from
> the notmuch CLI. Follow-up patches will convert some scattered error
> handling to use this and add error handling where we currently ignore
> errors.
pushed,
d
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-12-16 21:40 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-15 20:04 [PATCH v2 0/7] Improve Emacs CLI error handling Austin Clements
2012-12-15 20:04 ` [PATCH v2 1/7] emacs: Centralize notmuch command " Austin Clements
2012-12-16 21:39 ` David Bremner
2012-12-15 20:04 ` [PATCH v2 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process Austin Clements
2012-12-15 20:04 ` [PATCH v2 3/7] emacs: Factor out synchronous notmuch JSON invocations Austin Clements
2012-12-15 20:04 ` [PATCH v2 4/7] emacs: Improve error handling for notmuch-call-notmuch-json Austin Clements
2012-12-15 20:04 ` [PATCH v2 5/7] test: Test show's handling of subprocess errors Austin Clements
2012-12-15 20:04 ` [PATCH v2 6/7] emacs: Use unified error handling in search Austin Clements
2012-12-15 20:04 ` [PATCH v2 7/7] test: Test search's handling of subprocess errors Austin Clements
2012-12-16 1:19 ` [PATCH v2 0/7] Improve Emacs CLI error handling Mark Walters
2012-12-16 3:08 ` Tomi Ollila
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).