From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id CFCB2431FB6 for ; Fri, 28 Dec 2012 11:48:20 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yO7GWXcTtdPX for ; Fri, 28 Dec 2012 11:48:19 -0800 (PST) Received: from dmz-mailsec-scanner-4.mit.edu (DMZ-MAILSEC-SCANNER-4.MIT.EDU [18.9.25.15]) by olra.theworths.org (Postfix) with ESMTP id B859F431FAF for ; Fri, 28 Dec 2012 11:48:19 -0800 (PST) X-AuditID: 1209190f-b7f016d000000e07-51-50ddf783c5a6 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39]) by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP id B4.83.03591.387FDD05; Fri, 28 Dec 2012 14:48:19 -0500 (EST) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id qBSJmHC1004154; Fri, 28 Dec 2012 14:48:17 -0500 Received: from drake.dyndns.org (c-76-21-105-205.hsd1.ca.comcast.net [76.21.105.205]) (authenticated bits=0) (User authenticated as amdragon@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id qBSJmElg023343 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Fri, 28 Dec 2012 14:48:16 -0500 (EST) Received: from amthrax by drake.dyndns.org with local (Exim 4.77) (envelope-from ) id 1Tofuf-0006mX-Dt; Fri, 28 Dec 2012 14:48:13 -0500 From: Austin Clements To: notmuch@notmuchmail.org Subject: [PATCH] emacs: Use the minibuffer for CLI error reporting Date: Fri, 28 Dec 2012 14:48:08 -0500 Message-Id: <1356724088-26032-1-git-send-email-amdragon@mit.edu> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <87wqw2pcqs.fsf@zancas.localnet> References: <87wqw2pcqs.fsf@zancas.localnet> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrDIsWRmVeSWpSXmKPExsUixG6nrtv8/W6AwdLDrBY3WrsZLfbs87JY PZfH4vrNmcwOLB53T3N57Jx1l93j2apbzB5bDr1nDmCJ4rJJSc3JLEst0rdL4Mo4/Gw/W8F9 u4otE1pYGhi/G3QxcnJICJhIPLz1gxHCFpO4cG89WxcjF4eQwD5Gie/PmtlAEkICGxglZq0v gUhcZJL49OsWC4Qzl1Gic+tasHY2AQ2JbfuXg9kiAtISO+/OZgWxmQUqJf4eWc4EYgsLOEl8 +X8OaCoHB4uAqsSWl14gYV4BB4nJt2ezQFyhKNH9bALYYk4BXYnXD6ewgJQLCehI3P3nOYGR fwEjwypG2ZTcKt3cxMyc4tRk3eLkxLy81CJdE73czBK91JTSTYygQOOU5N/B+O2g0iFGAQ5G JR7eE7fvBgixJpYVV+YeYpTkYFIS5d37CijEl5SfUpmRWJwRX1Sak1p8iFGCg1lJhPfLe6Ac b0piZVVqUT5MSpqDRUmc92rKTX8hgfTEktTs1NSC1CKYrAwHh5IEb9A3oEbBotT01Iq0zJwS hDQTByfIcB6g4aogNbzFBYm5xZnpEPlTjIpS4rwOIAkBkERGaR5cLywRvGIUB3pFmNcIpIoH mETgul8BDWYCGmzNcwdkcEkiQkqqgXF+1axajS1u+xmVZeLWK1nbTWJrC89dd1ji8w/t4u1t j84vmSBY58XWuyHOTyTc5LSs/5p9F8Wbp03NSf614I/TZk+ZZ91JeXl7p8sJTVSZxKCaX5fO 4GplO2P6pQubfA++O3bm8ZO0xT5p28ReitTyZOfXPVQTkPvs6Hu8vFtPwXRRwddz9UosxRmJ hlrMRcWJAB1U6sXfAgAA X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Dec 2012 19:48:20 -0000 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). --- emacs/notmuch-lib.el | 92 ++++++++++++++++++++++++++++---------------------- emacs/notmuch.el | 9 +++-- test/emacs | 11 +++--- test/emacs-show | 6 ++-- 4 files changed, 67 insertions(+), 51 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 77a591d..3baab97 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." + + (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))))) + (basic-msg (format "%s exited with status %s" + (car command) exit-status))) + (when (and (null err) (or (null output) (equal output ""))) + ;; We have no details to speak of. Mimic `process-lines'. + (notmuch-logged-error basic-msg)) + (let ((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)) + "Output:\n" + (if (and output (not (equal output ""))) + output + "(none)")))) + (if err + ;; We have an error message straight from the CLI. + (notmuch-logged-error err extra) + ;; We only have combined output from the CLI; don't inundate + ;; the user with it. + (notmuch-logged-error basic-msg extra))))))) (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..8e0a4fd 100755 --- a/test/emacs +++ b/test/emacs @@ -862,18 +862,19 @@ 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 \"*Notmuch errors*\" - (test-output \"ERROR\")))" -test_expect_equal "$(cat OUTPUT ERROR)" "\ + (with-current-buffer \"*Messages*\" + (test-output \"MESSAGES\")) + (test-output))" +test_expect_equal "$(cat OUTPUT MESSAGES)" "\ 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" test_done diff --git a/test/emacs-show b/test/emacs-show index ebf530b..ae9459d 100755 --- a/test/emacs-show +++ b/test/emacs-show @@ -177,10 +177,12 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) (test-output) (with-current-buffer \"*Notmuch errors*\" (test-output \"ERROR\")))" +sed -i -e 's/^\[.*\]$/[XXX]/' 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: +[XXX] This is an error +Command: $PWD/notmuch_fail show --format\\=json --format-version\\=1 --exclude\\=false \\' \\* \\' +Exit status: 1 Output: This is output" -- 1.7.10.4