From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 4/8] emacs: Helper for reporting search parsing errors
In-Reply-To: <1341354059-29396-5-git-send-email-amdragon@mit.edu>
References: <1341354059-29396-1-git-send-email-amdragon@mit.edu>
	<1341354059-29396-5-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.13.2+70~gb6a56e7 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Wed, 04 Jul 2012 09:41:56 +0100
Message-ID: <87ehorhq97.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: tomi.ollila@iki.fi
Precedence: list

On Tue, 03 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> This removes the last bit of direct output from the parsing function.
> With the parser now responsible solely for parsing, we can swap it out
> for another parser.
> ---
>  emacs/notmuch.el |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 4a6490a..a073367 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -745,6 +745,13 @@ non-authors is found, assume that all of the authors match."
>  	(setq notmuch-search-target-thread "found")
>  	(goto-char beg)))))
>  
> +(defun notmuch-search-show-error (string &rest objects)
> +  (save-excursion
> +    (goto-char (point-max))
> +    (insert "Error: Unexpected output from notmuch search:\n")
> +    (insert (apply #'format string objects))
> +    (insert "\n")))
> +
>  (defun notmuch-search-process-filter (proc string)
>    "Process and filter the output of \"notmuch search\""
>    (let ((buffer (process-buffer proc)))
> @@ -767,7 +774,8 @@ non-authors is found, assume that all of the authors match."
>  			   (tags (match-string 6 string))
>  			   (tag-list (if tags (save-match-data (split-string tags)))))
>  		      (if (/= (match-beginning 1) line)
> -			  (insert (concat "Error: Unexpected output from notmuch search:\n" (substring string line (match-beginning 1)) "\n")))
> +			  (notmuch-search-show-error
> +			   (substring string line (match-beginning 1))))
>  		      (notmuch-search-show-result thread-id date count authors subject tag-list)
>  		      (set 'line (match-end 0)))
>  		  (set 'more nil)

Ok so I should have read this patch before replying to the previous
one. This fixes my one trivial concern with Patch 3 and looks good in
itself.

I have read Patches 1-4 and they all look fine and make things cleaner
and all tests pass. As Austin says, they stand on their own and can go
in without the rest of the series.

On to patches 5-8!

Best wishes

Mark