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

  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).