From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 3/5] emacs: Use async process helper for search
Date: Sat, 18 May 2013 08:14:37 +0100 [thread overview]
Message-ID: <87d2sopxnm.fsf@qmul.ac.uk> (raw)
In-Reply-To: <1368851472-5382-4-git-send-email-amdragon@mit.edu>
I have only very briefly looked at this: it seems to not quite apply to
master (one fix up see below)
Also, as far as I can see condition-case-unless-debug (used in patch
2/5) is emacs 24 only.
Best wishes
Mark
On Sat, 18 May 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> Previously, search started the async notmuch process directly. Now,
> it uses `notmuch-start-notmuch'. This simplifies the process sentinel
> a bit and means that we no longer have to worry about errors
> interleaved with the JSON output.
>
> We also update the tests of Emacs error handling, since the error
> output is now separated from the search results buffer.
> ---
> emacs/notmuch.el | 19 +++++--------------
> test/emacs | 36 ++++++++++++++++++++++++++++++++----
> 2 files changed, 37 insertions(+), 18 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 4c1a6ca..b8d9c44 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -653,15 +653,8 @@ of the result."
> ;; For version mismatch, there's no point in
> ;; showing the search buffer
> (when (or (= exit-status 20) (= exit-status 21))
> - (kill-buffer))
> - (condition-case err
> - (notmuch-check-async-exit-status proc msg)
> - ;; Suppress the error signal since strange
> - ;; things happen if a sentinel signals. Mimic
> - ;; the top-level's handling of error messages.
> - (error
> - (message "%s" (error-message-string err))
This line is
(message "%s" (second err))
in master.
> - (throw 'return nil)))
> + (kill-buffer)
> + (throw 'return nil))
> (if (and atbob
> (not (string= notmuch-search-target-thread "found")))
> (set 'never-found-target-thread t)))))
> @@ -938,10 +931,9 @@ Other optional parameters are used as follows:
> (erase-buffer)
> (goto-char (point-min))
> (save-excursion
> - (let ((proc (start-process
> - "notmuch-search" buffer
> - notmuch-command "search"
> - "--format=json" "--format-version=1"
> + (let ((proc (notmuch-start-notmuch
> + "notmuch-search" buffer #'notmuch-search-process-sentinel
> + "search" "--format=json" "--format-version=1"
> (if oldest-first
> "--sort=oldest-first"
> "--sort=newest-first")
> @@ -951,7 +943,6 @@ Other optional parameters are used as follows:
> ;; should be called no matter how the process dies.
> (parse-buf (generate-new-buffer " *notmuch search parse*")))
> (process-put proc 'parse-buf parse-buf)
> - (set-process-sentinel proc 'notmuch-search-process-sentinel)
> (set-process-filter proc 'notmuch-search-process-filter)
> (set-process-query-on-exit-flag proc nil))))
> (run-hooks 'notmuch-search-hook)))
> diff --git a/test/emacs b/test/emacs
> index f033bdf..d38ae8c 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -853,11 +853,10 @@ test_expect_success "Rendering HTML mail with images" \
> 'cat OUTPUT && grep -q smiley OUTPUT'
>
>
> -test_begin_subtest "Search handles subprocess errors"
> +test_begin_subtest "Search handles subprocess error exit codes"
> cat > notmuch_fail <<EOF
> #!/bin/sh
> echo This is output
> -echo This is an error >&2
> exit 1
> EOF
> chmod a+x notmuch_fail
> @@ -874,8 +873,6 @@ 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.
> ---
> $PWD/notmuch_fail exited with status 1 (see *Notmuch errors* for more details)
> @@ -885,4 +882,35 @@ $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_begin_subtest "Search handles subprocess warnings"
> +cat > notmuch_fail <<EOF
> +#!/bin/sh
> +echo This is output
> +echo This is a warning >&2
> +echo This is another warning >&2
> +exit 0
> +EOF
> +chmod a+x notmuch_fail
> +test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
> + (with-current-buffer \"*Messages*\" (erase-buffer))
> + (with-current-buffer \"*Notmuch errors*\" (erase-buffer))
> + (notmuch-search \"tag:inbox\")
> + (notmuch-test-wait)
> + (with-current-buffer \"*Messages*\"
> + (test-output \"MESSAGES\"))
> + (with-current-buffer \"*Notmuch errors*\"
> + (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
> +End of search results.
> +---
> +This is a warning (see *Notmuch errors* for more details)
> +---
> +[XXX]
> +This is a warning
> +This is another warning"
> +
> test_done
> --
> 1.7.10.4
next prev parent reply other threads:[~2013-05-18 7:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-18 4:31 [PATCH 0/5] Make Emacs search use sexp format Austin Clements
2013-05-18 4:31 ` [PATCH 1/5] test: Remove extraneous Emacs error handling test Austin Clements
2013-05-18 4:31 ` [PATCH 2/5] emacs: Utilities to manage asynchronous notmuch processes Austin Clements
2013-05-21 14:24 ` Mark Walters
2013-05-21 18:44 ` Tomi Ollila
2013-05-18 4:31 ` [PATCH 3/5] emacs: Use async process helper for search Austin Clements
2013-05-18 7:14 ` Mark Walters [this message]
2013-05-18 10:48 ` David Bremner
2013-05-18 20:13 ` Austin Clements
2013-05-18 4:31 ` [PATCH 4/5] emacs: Streaming S-expression parser Austin Clements
2013-05-21 15:20 ` Mark Walters
2013-05-22 0:03 ` Austin Clements
2013-05-25 8:59 ` Mark Walters
2013-05-27 19:04 ` Austin Clements
2013-05-27 20:58 ` Mark Walters
2013-05-18 4:31 ` [PATCH 5/5] emacs: Use streaming S-expr parser for search Austin Clements
2013-05-21 14:27 ` Mark Walters
2013-05-21 15:26 ` [PATCH 0/5] Make Emacs search use sexp format Mark Walters
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87d2sopxnm.fsf@qmul.ac.uk \
--to=markwalters1009@gmail.com \
--cc=amdragon@MIT.EDU \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).