From: Austin Clements Date: Fri, 31 May 2013 23:43:47 +0000 (+2000) Subject: Re: [PATCH v2 0/5] Make Emacs search use sexp format X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=c3dd4004e4b2daa41cf9ddb705903cbd87232801;p=notmuch-archives.git Re: [PATCH v2 0/5] Make Emacs search use sexp format --- diff --git a/03/c1d4e40d3debe31cf4123d4318691f9ca42fa3 b/03/c1d4e40d3debe31cf4123d4318691f9ca42fa3 new file mode 100644 index 000000000..40b335962 --- /dev/null +++ b/03/c1d4e40d3debe31cf4123d4318691f9ca42fa3 @@ -0,0 +1,275 @@ +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 8E517429E26 + for ; Fri, 31 May 2013 16:43:58 -0700 (PDT) +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 Wbx4rsonJMDj for ; + Fri, 31 May 2013 16:43:51 -0700 (PDT) +Received: from dmz-mailsec-scanner-6.mit.edu (dmz-mailsec-scanner-6.mit.edu + [18.7.68.35]) + by olra.theworths.org (Postfix) with ESMTP id 7F074431FBD + for ; Fri, 31 May 2013 16:43:51 -0700 (PDT) +X-AuditID: 12074423-b7f826d000001438-ef-51a935b6424f +Received: from mailhub-auth-3.mit.edu ( [18.9.21.43]) + by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP + id CE.9D.05176.6B539A15; Fri, 31 May 2013 19:43:50 -0400 (EDT) +Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) + by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id r4VNhnW0028142; + Fri, 31 May 2013 19:43:50 -0400 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id r4VNhl5b005098 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); + Fri, 31 May 2013 19:43:49 -0400 +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) + (envelope-from ) + id 1UiYz5-0005Vr-FH; Fri, 31 May 2013 19:43:47 -0400 +From: Austin Clements +To: Mark Walters , notmuch@notmuchmail.org +Subject: Re: [PATCH v2 0/5] Make Emacs search use sexp format +In-Reply-To: <87y5aug4gi.fsf@qmul.ac.uk> +References: <1369934016-22308-1-git-send-email-amdragon@mit.edu> + <87y5aug4gi.fsf@qmul.ac.uk> +User-Agent: Notmuch/0.15.2+133~gf9ff9fd (http://notmuchmail.org) Emacs/23.4.1 + (i486-pc-linux-gnu) +Date: Fri, 31 May 2013 19:43:47 -0400 +Message-ID: <87wqqehg0c.fsf@awakening.csail.mit.edu> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFvrEIsWRmVeSWpSXmKPExsUixCmqrbvNdGWgwY9DNhar5/JYXL85k9ni + zcp5rA7MHjtn3WX3OPx1IYvHs1W3mAOYo7hsUlJzMstSi/TtErgydt1cz1yw1aVi7o21jA2M + T0y7GDk5JARMJFrn7GSBsMUkLtxbz9bFyMUhJLCPUWLSrLmsIAkhgY2MEjs2BUIkTjNJzN+x + jBEisYRRYu5uXRCbTUBDYtv+5WBxEQFXiaffPjN3MXJwMANNbZ8XAhIWFrCVOP9tLxOIzQlU + vu/qQaj5MRITbm5nBykXFUiUWLA7BsRkEVCVWPOfHaSCF+jM9yveM0LYghInZz4BO5lZQEvi + xr+XTBMYBWchSc1CklrAyLSKUTYlt0o3NzEzpzg1Wbc4OTEvL7VI10wvN7NELzWldBMjKGjZ + XZR3MP45qHSIUYCDUYmHt2PVikAh1sSy4srcQ4ySHExKory5JisDhfiS8lMqMxKLM+KLSnNS + iw8xSnAwK4nw5s8BKudNSaysSi3Kh0lJc7AoifNeS7npLySQnliSmp2aWpBaBJOV4eBQkuDt + ABkqWJSanlqRlplTgpBm4uAEGc4DNPwhSA1vcUFibnFmOkT+FKOilDjvDZCEAEgiozQPrheW + VF4xigO9Isz7GKSKB5iQ4LpfAQ1mAhr8xHo5yOCSRISUVANjFhPfrhCDuDipv3wPfsTOeBRX + mf3gXdvTRnelBxOTnTi/1N2UuXT5yZc3s1nMlBKMV6fpnP1RHZopsiP40ZHcj56VOe9amY5/ + so89O/vctrwnk75s291dlSF1VOzavt8q/68ozswPtYtRmc1v/VxFakopS9kKEbPf2Yc6fkZ7 + LCyZwnk188wxJZbijERDLeai4kQATNEZEAUDAAA= +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 23:43:58 -0000 + +On Fri, 31 May 2013, Mark Walters wrote: +> 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. + +Oof. I must have been sleepy when I wrote that. Fixed. + +>> (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)