51/a8acf64290554fa8d441006c77f71156f0cde8

   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 4BCFE431FB6\r
   6         for <notmuch@notmuchmail.org>; Thu,  5 Jul 2012 11:58:31 -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 ys5Fpnb+gCD1 for <notmuch@notmuchmail.org>;\r
  16         Thu,  5 Jul 2012 11:58:30 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-3.mit.edu (DMZ-MAILSEC-SCANNER-3.MIT.EDU\r
  18         [18.9.25.14])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 24219431FAE\r
  20         for <notmuch@notmuchmail.org>; Thu,  5 Jul 2012 11:58:30 -0700 (PDT)\r
  21 X-AuditID: 1209190e-b7fb56d0000008b2-cc-4ff5e3d48e1f\r
  22 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
  23         by dmz-mailsec-scanner-3.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 7A.15.02226.4D3E5FF4; Thu,  5 Jul 2012 14:58:28 -0400 (EDT)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id q65IwRSL003765; \r
  27         Thu, 5 Jul 2012 14:58:27 -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.6/8.12.4) with ESMTP id q65IwQVx027141\r
  32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  33         Thu, 5 Jul 2012 14:58:27 -0400 (EDT)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1SmrFy-00048a-C8; Thu, 05 Jul 2012 14:58:26 -0400\r
  37 Date: Thu, 5 Jul 2012 14:58:26 -0400\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: Mark Walters <markwalters1009@gmail.com>\r
  40 Subject: Re: [PATCH 8/8] emacs: Switch from text to JSON format for search\r
  41         results\r
  42 Message-ID: <20120705185826.GJ21653@mit.edu>\r
  43 References: <1341354059-29396-1-git-send-email-amdragon@mit.edu>\r
  44         <1341354059-29396-9-git-send-email-amdragon@mit.edu>\r
  45         <87y5myeh8g.fsf@qmul.ac.uk>\r
  46 MIME-Version: 1.0\r
  47 Content-Type: text/plain; charset=us-ascii\r
  48 Content-Disposition: inline\r
  49 In-Reply-To: <87y5myeh8g.fsf@qmul.ac.uk>\r
  50 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  51 X-Brightmail-Tracker:\r
  52  H4sIAAAAAAAAA+NgFupjleLIzCtJLcpLzFFi42IRYrdT173y+Ku/wY81ahar5/JYXL85k9ni\r
  53         zcp5rA7MHjtn3WX3OPx1IYvHs1W3mAOYo7hsUlJzMstSi/TtErgy+mZsYi04bVIxr2EKWwPj\r
  54         U/UuRk4OCQETie8T7rJC2GISF+6tZ+ti5OIQEtjHKHGtfTM7hLOeUeJJ/0wo5wSTxMNZW1kh\r
  55         nCWMEr+e9DGD9LMIqEhMv/QCzGYT0JDYtn85I4gtIqAjcfvQAnYQm1lAX2LlyZlgNcICIRIL\r
  56         z39nArF5gWpuHVrIDDF0KqPEuSdvGCESghInZz5hgWjWkrjx7yVQAweQLS2x/B8HSJgTaNfX\r
  57         9kawclGgG6ac3MY2gVFoFpLuWUi6ZyF0L2BkXsUom5JbpZubmJlTnJqsW5ycmJeXWqRrrJeb\r
  58         WaKXmlK6iREU7JySfDsYvx5UOsQowMGoxMNrnPvFX4g1say4MvcQoyQHk5Io7+urX/2F+JLy\r
  59         UyozEosz4otKc1KLDzFKcDArifD2ZgDleFMSK6tSi/JhUtIcLErivFdSbvoLCaQnlqRmp6YW\r
  60         pBbBZGU4OJQkeB8/AmoULEpNT61Iy8wpQUgzcXCCDOcBGv4CpIa3uCAxtzgzHSJ/ilFRCmg0\r
  61         SEIAJJFRmgfXC0tGrxjFgV4R5n0PUsUDTGRw3a+ABjMBDc5b/AlkcEkiQkqqgdH3Trh6iVcp\r
  62         T2S4RchEYb19FzJs87ZktRQJbawRirF28Pm78eD9GafYUqzfPFx9uG6qwh3pH4uWOmzcmzCv\r
  63         PafUtfXeFRuGG8+/ztwduDbJ13SavSfL7zd/av2Psy+q4T/6TIvDTsZd3rHlEO/M5rXuv+6k\r
  64         ClhE8y3/wt7C47Rz2te3wbIW2UosxRmJhlrMRcWJAL5Ak2ohAwAA\r
  65 Cc: tomi.ollila@iki.fi, notmuch@notmuchmail.org\r
  66 X-BeenThere: notmuch@notmuchmail.org\r
  67 X-Mailman-Version: 2.1.13\r
  68 Precedence: list\r
  69 List-Id: "Use and development of the notmuch mail system."\r
  70         <notmuch.notmuchmail.org>\r
  71 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  72         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  73 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  74 List-Post: <mailto:notmuch@notmuchmail.org>\r
  75 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  76 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  77         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  78 X-List-Received-Date: Thu, 05 Jul 2012 18:58:31 -0000\r
  79 \r
  80 Quoth Mark Walters on Jul 05 at  9:37 am:\r
  81 > On Tue, 03 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:\r
  82 > > The JSON format eliminates the complex escaping issues that have\r
  83 > > plagued the text search format.  This uses the incremental JSON parser\r
  84 > > so that, like the text parser, it can output search results\r
  85 > > incrementally.\r
  86 > >\r
  87 > > This slows down the parser by about ~4X, but puts us in a good\r
  88 > > position to optimize either by improving the JSON parser (evidence\r
  89 > > suggests this can reduce the overhead to ~40% over the text format) or\r
  90 > > by switching to S-expressions (evidence suggests this will more than\r
  91 > > double performance over the text parser).  [1]\r
  92 > >\r
  93 > > This also fixes the incremental search parsing test.\r
  94 > >\r
  95 > > [1] id:"20110720205007.GB21316@mit.edu"\r
  96 > > ---\r
  97 > >  emacs/notmuch.el |  113 ++++++++++++++++++++++++++++++++----------------------\r
  98 > >  test/emacs       |    1 -\r
  99 > >  2 files changed, 67 insertions(+), 47 deletions(-)\r
 100 > >\r
 101 > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el\r
 102 > > index 084cec6..2a09a98 100644\r
 103 > > --- a/emacs/notmuch.el\r
 104 > > +++ b/emacs/notmuch.el\r
 105 > > @@ -60,7 +60,7 @@\r
 106 > >  (require 'notmuch-message)\r
 107 > >  \r
 108 > >  (defcustom notmuch-search-result-format\r
 109 > > -  `(("date" . "%s ")\r
 110 > > +  `(("date" . "%12s ")\r
 111 > >      ("count" . "%-7s ")\r
 112 > >      ("authors" . "%-20s ")\r
 113 > >      ("subject" . "%s ")\r
 114 > > @@ -557,17 +557,14 @@ This function advances the next thread when finished."\r
 115 > >    (notmuch-search-tag '("-inbox"))\r
 116 > >    (notmuch-search-next-thread))\r
 117 > >  \r
 118 > > -(defvar notmuch-search-process-filter-data nil\r
 119 > > -  "Data that has not yet been processed.")\r
 120 > > -(make-variable-buffer-local 'notmuch-search-process-filter-data)\r
 121 > > -\r
 122 > >  (defun notmuch-search-process-sentinel (proc msg)\r
 123 > >    "Add a message to let user know when \"notmuch search\" exits"\r
 124 > >    (let ((buffer (process-buffer proc))\r
 125 > >     (status (process-status proc))\r
 126 > >     (exit-status (process-exit-status proc))\r
 127 > >     (never-found-target-thread nil))\r
 128 > > -    (if (memq status '(exit signal))\r
 129 > > +    (when (memq status '(exit signal))\r
 130 > > +   (kill-buffer (process-get proc 'parse-buf))\r
 131 > >     (if (buffer-live-p buffer)\r
 132 > >         (with-current-buffer buffer\r
 133 > >           (save-excursion\r
 134 > > @@ -577,8 +574,6 @@ This function advances the next thread when finished."\r
 135 > >               (if (eq status 'signal)\r
 136 > >                   (insert "Incomplete search results (search process was killed).\n"))\r
 137 > >               (when (eq status 'exit)\r
 138 > > -               (if notmuch-search-process-filter-data\r
 139 > > -                   (insert (concat "Error: Unexpected output from notmuch search:\n" notmuch-search-process-filter-data)))\r
 140 > >                 (insert "End of search results.")\r
 141 > >                 (unless (= exit-status 0)\r
 142 > >                   (insert (format " (process returned %d)" exit-status)))\r
 143 > > @@ -757,45 +752,62 @@ non-authors is found, assume that all of the authors match."\r
 144 > >      (insert (apply #'format string objects))\r
 145 > >      (insert "\n")))\r
 146 > >  \r
 147 > > +(defvar notmuch-search-process-state nil\r
 148 > > +  "Parsing state of the search process filter.")\r
 149 > > +\r
 150 > > +(defvar notmuch-search-json-parser nil\r
 151 > > +  "Incremental JSON parser for the search process filter.")\r
 152 > > +\r
 153 > >  (defun notmuch-search-process-filter (proc string)\r
 154 > >    "Process and filter the output of \"notmuch search\""\r
 155 > > -  (let ((buffer (process-buffer proc)))\r
 156 > > -    (if (buffer-live-p buffer)\r
 157 > > -   (with-current-buffer buffer\r
 158 > > -       (let ((line 0)\r
 159 > > -             (more t)\r
 160 > > -             (inhibit-read-only t)\r
 161 > > -             (string (concat notmuch-search-process-filter-data string)))\r
 162 > > -         (setq notmuch-search-process-filter-data nil)\r
 163 > > -         (while more\r
 164 > > -           (while (and (< line (length string)) (= (elt string line) ?\n))\r
 165 > > -             (setq line (1+ line)))\r
 166 > > -           (if (string-match "^thread:\\([0-9A-Fa-f]*\\) \\([^][]*\\) \\[\\([0-9]*\\)/\\([0-9]*\\)\\] \\([^;]*\\); \\(.*\\) (\\([^()]*\\))$" string line)\r
 167 > > -               (let* ((thread-id (match-string 1 string))\r
 168 > > -                      (tags-str (match-string 7 string))\r
 169 > > -                      (result (list :thread thread-id\r
 170 > > -                                    :date_relative (match-string 2 string)\r
 171 > > -                                    :matched (string-to-number\r
 172 > > -                                              (match-string 3 string))\r
 173 > > -                                    :total (string-to-number\r
 174 > > -                                            (match-string 4 string))\r
 175 > > -                                    :authors (match-string 5 string)\r
 176 > > -                                    :subject (match-string 6 string)\r
 177 > > -                                    :tags (if tags-str\r
 178 > > -                                              (save-match-data\r
 179 > > -                                                (split-string tags-str))))))\r
 180 > > -                 (if (/= (match-beginning 0) line)\r
 181 > > -                     (notmuch-search-show-error\r
 182 > > -                      (substring string line (match-beginning 0))))\r
 183 > > -                 (notmuch-search-show-result result)\r
 184 > > -                 (set 'line (match-end 0)))\r
 185 > > -             (set 'more nil)\r
 186 > > -             (while (and (< line (length string)) (= (elt string line) ?\n))\r
 187 > > -               (setq line (1+ line)))\r
 188 > > -             (if (< line (length string))\r
 189 > > -                 (setq notmuch-search-process-filter-data (substring string line)))\r
 190 > > -             ))))\r
 191 > > -      (delete-process proc))))\r
 192 > > +  (let ((results-buf (process-buffer proc))\r
 193 > > +   (parse-buf (process-get proc 'parse-buf))\r
 194 > > +   (inhibit-read-only t)\r
 195 > > +   done)\r
 196 > > +    (if (not (buffer-live-p results-buf))\r
 197 > > +   (delete-process proc)\r
 198 > > +      (with-current-buffer parse-buf\r
 199 > > +   ;; Insert new data\r
 200 > > +   (save-excursion\r
 201 > > +     (goto-char (point-max))\r
 202 > > +     (insert string)))\r
 203 > > +      (with-current-buffer results-buf\r
 204 > > +   (while (not done)\r
 205 > > +     (condition-case nil\r
 206 > > +         (case notmuch-search-process-state\r
 207 > > +           ((begin)\r
 208 > > +            ;; Enter the results list\r
 209 > > +            (if (eq (notmuch-json-begin-compound\r
 210 > > +                     notmuch-search-json-parser) 'retry)\r
 211 > > +                (setq done t)\r
 212 > > +              (setq notmuch-search-process-state 'result)))\r
 213 > > +           ((result)\r
 214 > > +            ;; Parse a result\r
 215 > > +            (let ((result (notmuch-json-read notmuch-search-json-parser)))\r
 216 > > +              (case result\r
 217 > > +                ((retry) (setq done t))\r
 218 > > +                ((end) (setq notmuch-search-process-state 'end))\r
 219 > > +                (otherwise (notmuch-search-show-result result)))))\r
 220 > > +           ((end)\r
 221 > > +            ;; Any trailing data is unexpected\r
 222 > > +            (with-current-buffer parse-buf\r
 223 > > +              (skip-chars-forward " \t\r\n")\r
 224 > > +              (if (eobp)\r
 225 > > +                  (setq done t)\r
 226 > > +                (signal 'json-error nil)))))\r
 227 > \r
 228 > This looks good to me. Would it make sense to put the "Any trailing\r
 229 > data" as a tiny function in notmuch-lib? something like \r
 230 > \r
 231 > (defun notmuch-json-assert-end-of-data ()\r
 232 >    (skip-chars-forward " \t\r\n") \r
 233 >    (if (eobp)\r
 234 >        (setq done t) \r
 235 >      (signal 'json-error nil)))\r
 236 \r
 237 Also a good idea.\r
 238 \r
 239 Thanks for the review!  I'll be sending v2 shortly.\r
 240 \r
 241 > Best wishes\r
 242 > \r
 243 > Mark\r