03/c1d4e40d3debe31cf4123d4318691f9ca42fa3

   1 Return-Path: <amdragon@mit.edu>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 8E517429E26\r
   6         for <notmuch@notmuchmail.org>; Fri, 31 May 2013 16:43:58 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id Wbx4rsonJMDj for <notmuch@notmuchmail.org>;\r
  16         Fri, 31 May 2013 16:43:51 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-6.mit.edu (dmz-mailsec-scanner-6.mit.edu\r
  18         [18.7.68.35])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 7F074431FBD\r
  20         for <notmuch@notmuchmail.org>; Fri, 31 May 2013 16:43:51 -0700 (PDT)\r
  21 X-AuditID: 12074423-b7f826d000001438-ef-51a935b6424f\r
  22 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  23         by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id CE.9D.05176.6B539A15; Fri, 31 May 2013 19:43:50 -0400 (EDT)\r
  25 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  26         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id r4VNhnW0028142; \r
  27         Fri, 31 May 2013 19:43:50 -0400\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id r4VNhl5b005098\r
  32         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  33         Fri, 31 May 2013 19:43:49 -0400\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1UiYz5-0005Vr-FH; Fri, 31 May 2013 19:43:47 -0400\r
  37 From: Austin Clements <amdragon@MIT.EDU>\r
  38 To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
  39 Subject: Re: [PATCH v2 0/5] Make Emacs search use sexp format\r
  40 In-Reply-To: <87y5aug4gi.fsf@qmul.ac.uk>\r
  41 References: <1369934016-22308-1-git-send-email-amdragon@mit.edu>\r
  42         <87y5aug4gi.fsf@qmul.ac.uk>\r
  43 User-Agent: Notmuch/0.15.2+133~gf9ff9fd (http://notmuchmail.org) Emacs/23.4.1\r
  44         (i486-pc-linux-gnu)\r
  45 Date: Fri, 31 May 2013 19:43:47 -0400\r
  46 Message-ID: <87wqqehg0c.fsf@awakening.csail.mit.edu>\r
  47 MIME-Version: 1.0\r
  48 Content-Type: text/plain; charset=us-ascii\r
  49 X-Brightmail-Tracker:\r
  50  H4sIAAAAAAAAA+NgFvrEIsWRmVeSWpSXmKPExsUixCmqrbvNdGWgwY9DNhar5/JYXL85k9ni\r
  51         zcp5rA7MHjtn3WX3OPx1IYvHs1W3mAOYo7hsUlJzMstSi/TtErgydt1cz1yw1aVi7o21jA2M\r
  52         T0y7GDk5JARMJFrn7GSBsMUkLtxbz9bFyMUhJLCPUWLSrLmsIAkhgY2MEjs2BUIkTjNJzN+x\r
  53         jBEisYRRYu5uXRCbTUBDYtv+5WBxEQFXiaffPjN3MXJwMANNbZ8XAhIWFrCVOP9tLxOIzQlU\r
  54         vu/qQaj5MRITbm5nBykXFUiUWLA7BsRkEVCVWPOfHaSCF+jM9yveM0LYghInZz4BO5lZQEvi\r
  55         xr+XTBMYBWchSc1CklrAyLSKUTYlt0o3NzEzpzg1Wbc4OTEvL7VI10wvN7NELzWldBMjKGjZ\r
  56         XZR3MP45qHSIUYCDUYmHt2PVikAh1sSy4srcQ4ySHExKory5JisDhfiS8lMqMxKLM+KLSnNS\r
  57         iw8xSnAwK4nw5s8BKudNSaysSi3Kh0lJc7AoifNeS7npLySQnliSmp2aWpBaBJOV4eBQkuDt\r
  58         ABkqWJSanlqRlplTgpBm4uAEGc4DNPwhSA1vcUFibnFmOkT+FKOilDjvDZCEAEgiozQPrheW\r
  59         VF4xigO9Isz7GKSKB5iQ4LpfAQ1mAhr8xHo5yOCSRISUVANjFhPfrhCDuDipv3wPfsTOeBRX\r
  60         mf3gXdvTRnelBxOTnTi/1N2UuXT5yZc3s1nMlBKMV6fpnP1RHZopsiP40ZHcj56VOe9amY5/\r
  61         so89O/vctrwnk75s291dlSF1VOzavt8q/68ozswPtYtRmc1v/VxFakopS9kKEbPf2Yc6fkZ7\r
  62         LCyZwnk188wxJZbijERDLeai4kQATNEZEAUDAAA=\r
  63 Cc: tomi.ollila@iki.fi\r
  64 X-BeenThere: notmuch@notmuchmail.org\r
  65 X-Mailman-Version: 2.1.13\r
  66 Precedence: list\r
  67 List-Id: "Use and development of the notmuch mail system."\r
  68         <notmuch.notmuchmail.org>\r
  69 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  70         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  71 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  72 List-Post: <mailto:notmuch@notmuchmail.org>\r
  73 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  74 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  76 X-List-Received-Date: Fri, 31 May 2013 23:43:58 -0000\r
  77 \r
  78 On Fri, 31 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:\r
  79 > This version LGTM +1. I will let you and Tomi decide which shell magic\r
  80 > to go with.\r
  81 >\r
  82 > One very trivial comment on a comment below\r
  83 >\r
  84 > Best wishes\r
  85 >\r
  86 > Mark\r
  87 >\r
  88 > Austin Clements <amdragon@MIT.EDU> writes:\r
  89 >\r
  90 >> This is v2 of id:1368851472-5382-1-git-send-email-amdragon@mit.edu.\r
  91 >> The most substantial change from v1 is that the streaming S-expression\r
  92 >> parser now requires the caller to invoke it from the appropriate\r
  93 >> buffer and no longer attempts to track the buffer itself.  For subtle\r
  94 >> reasons arising from per-window points, the only *correct* way to use\r
  95 >> the interface before required the caller to invoke it from the\r
  96 >> appropriate buffer anyway (or risk losing track of what had been\r
  97 >> parsed).  The only place that currently invokes the streaming\r
  98 >> S-expression parser already satisfied this requirement.\r
  99 >>\r
 100 >> I decided *not* to use --stderr to redirect stderr.  As discussed on\r
 101 >> IRC, --stderr causes serious problems for wrapper scripts, which\r
 102 >> either have to handle --stderr themselves or risk mixing their own\r
 103 >> stderr with stdout (e.g., errors from ssh) or, worse, redirecting\r
 104 >> notmuch's stderr to a (world-readable) file on a *remote* machine.  I\r
 105 >> did fix the exec-path problem that Tomi pointed out, so\r
 106 >> notmuch-command will continue to be searched for in exec-path, like it\r
 107 >> currently is.\r
 108 >>\r
 109 >> The diff from v1 is below, with whitespace changes because of\r
 110 >> re-indentation in the S-expression parser.\r
 111 >>\r
 112 >> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el\r
 113 >> index a543471..180f63d 100644\r
 114 >> --- a/emacs/notmuch-lib.el\r
 115 >> +++ b/emacs/notmuch-lib.el\r
 116 >> @@ -493,10 +493,13 @@ status."\r
 117 >>    (let* ((err-file (make-temp-file "nmerr"))\r
 118 >>       ;; Use a pipe\r
 119 >>       (process-connection-type nil)\r
 120 >> +     ;; Find notmuch using Emacs' `exec-path'\r
 121 >> +     (command (or (executable-find notmuch-command)\r
 122 >> +                  (error "command not found: %s" notmuch-command)))\r
 123 >>       (proc (apply #'start-process name buffer\r
 124 >>                    "sh" "-c"\r
 125 >>                    "ERR=\"$1\"; shift; exec \"$0\" \"$@\" 2>\"$ERR\""\r
 126 >> -                  notmuch-command err-file args)))\r
 127 >> +                  command err-file args)))\r
 128 >>      (process-put proc 'err-file err-file)\r
 129 >>      (process-put proc 'sub-sentinel sentinel)\r
 130 >>      (process-put proc 'real-command (cons notmuch-command args))\r
 131 >> @@ -507,7 +510,7 @@ status."\r
 132 >>    (let ((err-file (process-get proc 'err-file))\r
 133 >>      (sub-sentinel (process-get proc 'sub-sentinel))\r
 134 >>      (real-command (process-get proc 'real-command)))\r
 135 >> -    (condition-case-unless-debug err\r
 136 >> +    (condition-case err\r
 137 >>      (progn\r
 138 >>        ;; Invoke the sub-sentinel, if any\r
 139 >>        (when sub-sentinel\r
 140 >> @@ -530,7 +533,8 @@ status."\r
 141 >>          (when warnings\r
 142 >>            (notmuch-logged-error (car warnings) (cdr warnings)))))\r
 143 >>        (error\r
 144 >> -       ;; Don't signal an error from a sentinel\r
 145 >> +       ;; Emacs behaves strangely if error an error escapes from a\r
 146 >> +       ;; sentinel, so turns errors into messages.\r
 147 >\r
 148 > 2 typos: "error an error" and turns should be turn.\r
 149 \r
 150 Oof.  I must have been sleepy when I wrote that.  Fixed.\r
 151 \r
 152 >>         (message "%s" (error-message-string err))))\r
 153 >>      (ignore-errors (delete-file err-file))))\r
 154 >>  \r
 155 >> diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el\r
 156 >> index 1b7cf64..d59c0e1 100644\r
 157 >> --- a/emacs/notmuch-parser.el\r
 158 >> +++ b/emacs/notmuch-parser.el\r
 159 >> @@ -21,8 +21,8 @@\r
 160 >>  \r
 161 >>  (require 'cl)\r
 162 >>  \r
 163 >> -(defun notmuch-sexp-create-parser (buffer)\r
 164 >> -  "Return a streaming S-expression parser that reads from BUFFER.\r
 165 >> +(defun notmuch-sexp-create-parser ()\r
 166 >> +  "Return a new streaming S-expression parser.\r
 167 >>  \r
 168 >>  This parser is designed to incrementally read an S-expression\r
 169 >>  whose structure is known to the caller.  Like a typical\r
 170 >> @@ -33,12 +33,11 @@ input to be a list and descends into it, allowing its elements to\r
 171 >>  be read one at a time or further descended into.  Both functions\r
 172 >>  can return 'retry to indicate that not enough input is available.\r
 173 >>  \r
 174 >> -The parser always consumes input from BUFFER's point.  Hence, the\r
 175 >> -caller is allowed to delete any data before point and may\r
 176 >> -resynchronize after an error by moving point."\r
 177 >> +The parser always consumes input from point in the current\r
 178 >> +buffer.  Hence, the caller is allowed to delete any data before\r
 179 >> +point and may resynchronize after an error by moving point."\r
 180 >>  \r
 181 >>    (vector 'notmuch-sexp-parser\r
 182 >> -      buffer\r
 183 >>        ;; List depth\r
 184 >>        0\r
 185 >>        ;; Partial parse position marker\r
 186 >> @@ -46,13 +45,12 @@ resynchronize after an error by moving point."\r
 187 >>        ;; Partial parse state\r
 188 >>        nil))\r
 189 >>  \r
 190 >> -(defmacro notmuch-sexp--buffer (sp)        `(aref ,sp 1))\r
 191 >> -(defmacro notmuch-sexp--depth (sp)         `(aref ,sp 2))\r
 192 >> -(defmacro notmuch-sexp--partial-pos (sp)   `(aref ,sp 3))\r
 193 >> -(defmacro notmuch-sexp--partial-state (sp) `(aref ,sp 4))\r
 194 >> +(defmacro notmuch-sexp--depth (sp)         `(aref ,sp 1))\r
 195 >> +(defmacro notmuch-sexp--partial-pos (sp)   `(aref ,sp 2))\r
 196 >> +(defmacro notmuch-sexp--partial-state (sp) `(aref ,sp 3))\r
 197 >>  \r
 198 >>  (defun notmuch-sexp-read (sp)\r
 199 >> -  "Consume and return the value at point in SP's buffer.\r
 200 >> +  "Consume and return the value at point in the current buffer.\r
 201 >>  \r
 202 >>  Returns 'retry if there is insufficient input to parse a complete\r
 203 >>  value (though it may still move point over whitespace).  If the\r
 204 >> @@ -61,14 +59,13 @@ list, this moves point just past the terminator and returns 'end.\r
 205 >>  Otherwise, this moves point to just past the end of the value and\r
 206 >>  returns the value."\r
 207 >>  \r
 208 >> -  (with-current-buffer (notmuch-sexp--buffer sp)\r
 209 >>    (skip-chars-forward " \n\r\t")\r
 210 >>    (cond ((eobp) 'retry)\r
 211 >>      ((= (char-after) ?\))\r
 212 >>       ;; We've reached the end of a list\r
 213 >>       (if (= (notmuch-sexp--depth sp) 0)\r
 214 >>           ;; .. but we weren't in a list.  Let read signal the\r
 215 >> -           ;; error.\r
 216 >> +         ;; error to be consistent with all other code paths.\r
 217 >>           (read (current-buffer))\r
 218 >>         ;; Go up a level and return an end token\r
 219 >>         (decf (notmuch-sexp--depth sp))\r
 220 >> @@ -124,7 +121,7 @@ returns the value."\r
 221 >>                 'retry))\r
 222 >>           (end-of-file\r
 223 >>            (goto-char start)\r
 224 >> -            'retry)))))))\r
 225 >> +          'retry))))))\r
 226 >>  \r
 227 >>  (defun notmuch-sexp-begin-list (sp)\r
 228 >>    "Parse the beginning of a list value and enter the list.\r
 229 >> @@ -136,7 +133,6 @@ returns t.  Later calls to `notmuch-sexp-read' will return the\r
 230 >>  elements inside the list.  If the input in buffer is not the\r
 231 >>  beginning of a list, throw invalid-read-syntax."\r
 232 >>  \r
 233 >> -  (with-current-buffer (notmuch-sexp--buffer sp)\r
 234 >>    (skip-chars-forward " \n\r\t")\r
 235 >>    (cond ((eobp) 'retry)\r
 236 >>      ((= (char-after) ?\()\r
 237 >> @@ -146,7 +142,7 @@ beginning of a list, throw invalid-read-syntax."\r
 238 >>      (t\r
 239 >>       ;; Skip over the bad character like `read' does\r
 240 >>       (forward-char)\r
 241 >> -       (signal 'invalid-read-syntax (list (string (char-before))))))))\r
 242 >> +     (signal 'invalid-read-syntax (list (string (char-before)))))))\r
 243 >>  \r
 244 >>  (defun notmuch-sexp-eof (sp)\r
 245 >>    "Signal an error if there is more data in SP's buffer.\r
 246 >> @@ -154,10 +150,9 @@ beginning of a list, throw invalid-read-syntax."\r
 247 >>  Moves point to the beginning of any trailing data or to the end\r
 248 >>  of the buffer if there is only trailing whitespace."\r
 249 >>  \r
 250 >> -  (with-current-buffer (notmuch-sexp--buffer sp)\r
 251 >>    (skip-chars-forward " \n\r\t")\r
 252 >>    (unless (eobp)\r
 253 >> -      (error "Trailing garbage following expression"))))\r
 254 >> +    (error "Trailing garbage following expression")))\r
 255 >>  \r
 256 >>  (defvar notmuch-sexp--parser nil\r
 257 >>    "The buffer-local notmuch-sexp-parser instance.\r
 258 >> @@ -170,7 +165,7 @@ Used by `notmuch-sexp-parse-partial-list'.")\r
 259 >>  (defun notmuch-sexp-parse-partial-list (result-function result-buffer)\r
 260 >>    "Incrementally parse an S-expression list from the current buffer.\r
 261 >>  \r
 262 >> -This function consume an S-expression list from the current\r
 263 >> +This function consumes an S-expression list from the current\r
 264 >>  buffer, applying RESULT-FUNCTION in RESULT-BUFFER to each\r
 265 >>  complete value in the list.  It operates incrementally and should\r
 266 >>  be called whenever the input buffer has been extended with\r
 267 >> @@ -180,7 +175,7 @@ move point in the input buffer."\r
 268 >>    ;; Set up the initial state\r
 269 >>    (unless (local-variable-p 'notmuch-sexp--parser)\r
 270 >>      (set (make-local-variable 'notmuch-sexp--parser)\r
 271 >> -     (notmuch-sexp-create-parser (current-buffer)))\r
 272 >> +     (notmuch-sexp-create-parser))\r
 273 >>      (set (make-local-variable 'notmuch-sexp--state) 'begin))\r
 274 >>    (let (done)\r
 275 >>      (while (not done)\r