From: Mark Walters Date: Fri, 31 May 2013 22:38:37 +0000 (+0100) Subject: Re: [PATCH v2 0/5] Make Emacs search use sexp format X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=555c70d88f4cd6b770814d19497a70f763363ed7;p=notmuch-archives.git Re: [PATCH v2 0/5] Make Emacs search use sexp format --- diff --git a/f4/8272cf45ef60587135c91830e7e45e085ab34e b/f4/8272cf45ef60587135c91830e7e45e085ab34e new file mode 100644 index 000000000..92c2f66b1 --- /dev/null +++ b/f4/8272cf45ef60587135c91830e7e45e085ab34e @@ -0,0 +1,266 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 6F7C7431FC7 + for ; Fri, 31 May 2013 15:38:51 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -1.098 +X-Spam-Level: +X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5 + tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, + NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] 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 kOoW14aSfhcj for ; + Fri, 31 May 2013 15:38:45 -0700 (PDT) +Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 340EF431FBD + for ; Fri, 31 May 2013 15:38:45 -0700 (PDT) +Received: from smtp.qmul.ac.uk ([138.37.6.40]) + by mail2.qmul.ac.uk with esmtp (Exim 4.71) + (envelope-from ) + id 1UiXy3-0005ti-Bl; Fri, 31 May 2013 23:38:39 +0100 +Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost) + by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71) + (envelope-from ) + id 1UiXy2-0005Mf-Re; Fri, 31 May 2013 23:38:39 +0100 +From: Mark Walters +To: Austin Clements , notmuch@notmuchmail.org +Subject: Re: [PATCH v2 0/5] Make Emacs search use sexp format +In-Reply-To: <1369934016-22308-1-git-send-email-amdragon@mit.edu> +References: <1369934016-22308-1-git-send-email-amdragon@mit.edu> +User-Agent: Notmuch/0.14+255~gff3cc55 (http://notmuchmail.org) Emacs/23.4.1 + (i486-pc-linux-gnu) +Date: Fri, 31 May 2013 23:38:37 +0100 +Message-ID: <87y5aug4gi.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Sender-Host-Address: 93.97.24.31 +X-QM-SPAM-Info: Sender has good ham record. :) +X-QM-Body-MD5: 511131fa4828e08db958fc2e8fc6ab79 (of first 20000 bytes) +X-SpamAssassin-Score: -0.1 +X-SpamAssassin-SpamBar: / +X-SpamAssassin-Report: The QM spam filters have analysed this message to + determine if it is + spam. We require at least 5.0 points to mark a message as spam. + This message scored -0.1 points. + Summary of the scoring: + * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail + provider * (markwalters1009[at]gmail.com) + * -0.1 AWL AWL: From: address is in the auto white-list +X-QM-Scan-Virus: ClamAV says the message is clean +Cc: tomi.ollila@iki.fi +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, 31 May 2013 22:38:51 -0000 + + +This version LGTM +1. I will let you and Tomi decide which shell magic +to go with. + +One very trivial comment on a comment below + +Best wishes + +Mark + +Austin Clements writes: + +> This is v2 of id:1368851472-5382-1-git-send-email-amdragon@mit.edu. +> The most substantial change from v1 is that the streaming S-expression +> parser now requires the caller to invoke it from the appropriate +> buffer and no longer attempts to track the buffer itself. For subtle +> reasons arising from per-window points, the only *correct* way to use +> the interface before required the caller to invoke it from the +> appropriate buffer anyway (or risk losing track of what had been +> parsed). The only place that currently invokes the streaming +> S-expression parser already satisfied this requirement. +> +> I decided *not* to use --stderr to redirect stderr. As discussed on +> IRC, --stderr causes serious problems for wrapper scripts, which +> either have to handle --stderr themselves or risk mixing their own +> stderr with stdout (e.g., errors from ssh) or, worse, redirecting +> notmuch's stderr to a (world-readable) file on a *remote* machine. I +> did fix the exec-path problem that Tomi pointed out, so +> notmuch-command will continue to be searched for in exec-path, like it +> currently is. +> +> The diff from v1 is below, with whitespace changes because of +> re-indentation in the S-expression parser. +> +> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el +> index a543471..180f63d 100644 +> --- a/emacs/notmuch-lib.el +> +++ b/emacs/notmuch-lib.el +> @@ -493,10 +493,13 @@ status." +> (let* ((err-file (make-temp-file "nmerr")) +> ;; Use a pipe +> (process-connection-type nil) +> + ;; Find notmuch using Emacs' `exec-path' +> + (command (or (executable-find notmuch-command) +> + (error "command not found: %s" notmuch-command))) +> (proc (apply #'start-process name buffer +> "sh" "-c" +> "ERR=\"$1\"; shift; exec \"$0\" \"$@\" 2>\"$ERR\"" +> - notmuch-command err-file args))) +> + command err-file args))) +> (process-put proc 'err-file err-file) +> (process-put proc 'sub-sentinel sentinel) +> (process-put proc 'real-command (cons notmuch-command args)) +> @@ -507,7 +510,7 @@ status." +> (let ((err-file (process-get proc 'err-file)) +> (sub-sentinel (process-get proc 'sub-sentinel)) +> (real-command (process-get proc 'real-command))) +> - (condition-case-unless-debug err +> + (condition-case err +> (progn +> ;; Invoke the sub-sentinel, if any +> (when sub-sentinel +> @@ -530,7 +533,8 @@ status." +> (when warnings +> (notmuch-logged-error (car warnings) (cdr warnings))))) +> (error +> - ;; Don't signal an error from a sentinel +> + ;; Emacs behaves strangely if error an error escapes from a +> + ;; sentinel, so turns errors into messages. + +2 typos: "error an error" and turns should be turn. + +> (message "%s" (error-message-string err)))) +> (ignore-errors (delete-file err-file)))) +> +> diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el +> index 1b7cf64..d59c0e1 100644 +> --- a/emacs/notmuch-parser.el +> +++ b/emacs/notmuch-parser.el +> @@ -21,8 +21,8 @@ +> +> (require 'cl) +> +> -(defun notmuch-sexp-create-parser (buffer) +> - "Return a streaming S-expression parser that reads from BUFFER. +> +(defun notmuch-sexp-create-parser () +> + "Return a new streaming S-expression parser. +> +> This parser is designed to incrementally read an S-expression +> whose structure is known to the caller. Like a typical +> @@ -33,12 +33,11 @@ input to be a list and descends into it, allowing its elements to +> be read one at a time or further descended into. Both functions +> can return 'retry to indicate that not enough input is available. +> +> -The parser always consumes input from BUFFER's point. Hence, the +> -caller is allowed to delete any data before point and may +> -resynchronize after an error by moving point." +> +The parser always consumes input from point in the current +> +buffer. Hence, the caller is allowed to delete any data before +> +point and may resynchronize after an error by moving point." +> +> (vector 'notmuch-sexp-parser +> - buffer +> ;; List depth +> 0 +> ;; Partial parse position marker +> @@ -46,13 +45,12 @@ resynchronize after an error by moving point." +> ;; Partial parse state +> nil)) +> +> -(defmacro notmuch-sexp--buffer (sp) `(aref ,sp 1)) +> -(defmacro notmuch-sexp--depth (sp) `(aref ,sp 2)) +> -(defmacro notmuch-sexp--partial-pos (sp) `(aref ,sp 3)) +> -(defmacro notmuch-sexp--partial-state (sp) `(aref ,sp 4)) +> +(defmacro notmuch-sexp--depth (sp) `(aref ,sp 1)) +> +(defmacro notmuch-sexp--partial-pos (sp) `(aref ,sp 2)) +> +(defmacro notmuch-sexp--partial-state (sp) `(aref ,sp 3)) +> +> (defun notmuch-sexp-read (sp) +> - "Consume and return the value at point in SP's buffer. +> + "Consume and return the value at point in the current buffer. +> +> Returns 'retry if there is insufficient input to parse a complete +> value (though it may still move point over whitespace). If the +> @@ -61,14 +59,13 @@ list, this moves point just past the terminator and returns 'end. +> Otherwise, this moves point to just past the end of the value and +> returns the value." +> +> - (with-current-buffer (notmuch-sexp--buffer sp) +> (skip-chars-forward " \n\r\t") +> (cond ((eobp) 'retry) +> ((= (char-after) ?\)) +> ;; We've reached the end of a list +> (if (= (notmuch-sexp--depth sp) 0) +> ;; .. but we weren't in a list. Let read signal the +> - ;; error. +> + ;; error to be consistent with all other code paths. +> (read (current-buffer)) +> ;; Go up a level and return an end token +> (decf (notmuch-sexp--depth sp)) +> @@ -124,7 +121,7 @@ returns the value." +> 'retry)) +> (end-of-file +> (goto-char start) +> - 'retry))))))) +> + 'retry)))))) +> +> (defun notmuch-sexp-begin-list (sp) +> "Parse the beginning of a list value and enter the list. +> @@ -136,7 +133,6 @@ returns t. Later calls to `notmuch-sexp-read' will return the +> elements inside the list. If the input in buffer is not the +> beginning of a list, throw invalid-read-syntax." +> +> - (with-current-buffer (notmuch-sexp--buffer sp) +> (skip-chars-forward " \n\r\t") +> (cond ((eobp) 'retry) +> ((= (char-after) ?\() +> @@ -146,7 +142,7 @@ beginning of a list, throw invalid-read-syntax." +> (t +> ;; Skip over the bad character like `read' does +> (forward-char) +> - (signal 'invalid-read-syntax (list (string (char-before)))))))) +> + (signal 'invalid-read-syntax (list (string (char-before))))))) +> +> (defun notmuch-sexp-eof (sp) +> "Signal an error if there is more data in SP's buffer. +> @@ -154,10 +150,9 @@ beginning of a list, throw invalid-read-syntax." +> Moves point to the beginning of any trailing data or to the end +> of the buffer if there is only trailing whitespace." +> +> - (with-current-buffer (notmuch-sexp--buffer sp) +> (skip-chars-forward " \n\r\t") +> (unless (eobp) +> - (error "Trailing garbage following expression")))) +> + (error "Trailing garbage following expression"))) +> +> (defvar notmuch-sexp--parser nil +> "The buffer-local notmuch-sexp-parser instance. +> @@ -170,7 +165,7 @@ Used by `notmuch-sexp-parse-partial-list'.") +> (defun notmuch-sexp-parse-partial-list (result-function result-buffer) +> "Incrementally parse an S-expression list from the current buffer. +> +> -This function consume an S-expression list from the current +> +This function consumes an S-expression list from the current +> buffer, applying RESULT-FUNCTION in RESULT-BUFFER to each +> complete value in the list. It operates incrementally and should +> be called whenever the input buffer has been extended with +> @@ -180,7 +175,7 @@ move point in the input buffer." +> ;; Set up the initial state +> (unless (local-variable-p 'notmuch-sexp--parser) +> (set (make-local-variable 'notmuch-sexp--parser) +> - (notmuch-sexp-create-parser (current-buffer))) +> + (notmuch-sexp-create-parser)) +> (set (make-local-variable 'notmuch-sexp--state) 'begin)) +> (let (done) +> (while (not done)