unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [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).