Quoth Mark Walters on Jul 05 at  9:37 am:
> On Tue, 03 Jul 2012, Austin Clements <amdragon at MIT.EDU> wrote:
> > The JSON format eliminates the complex escaping issues that have
> > plagued the text search format.  This uses the incremental JSON parser
> > so that, like the text parser, it can output search results
> > incrementally.
> >
> > This slows down the parser by about ~4X, but puts us in a good
> > position to optimize either by improving the JSON parser (evidence
> > suggests this can reduce the overhead to ~40% over the text format) or
> > by switching to S-expressions (evidence suggests this will more than
> > double performance over the text parser).  [1]
> >
> > This also fixes the incremental search parsing test.
> >
> > [1] id:"20110720205007.GB21316 at mit.edu"
> > ---
> >  emacs/notmuch.el |  113 
> > ++++++++++++++++++++++++++++++++----------------------
> >  test/emacs       |    1 -
> >  2 files changed, 67 insertions(+), 47 deletions(-)
> >
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index 084cec6..2a09a98 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -60,7 +60,7 @@
> >  (require 'notmuch-message)
> >  
> >  (defcustom notmuch-search-result-format
> > -  `(("date" . "%s ")
> > +  `(("date" . "%12s ")
> >      ("count" . "%-7s ")
> >      ("authors" . "%-20s ")
> >      ("subject" . "%s ")
> > @@ -557,17 +557,14 @@ This function advances the next thread when finished."
> >    (notmuch-search-tag '("-inbox"))
> >    (notmuch-search-next-thread))
> >  
> > -(defvar notmuch-search-process-filter-data nil
> > -  "Data that has not yet been processed.")
> > -(make-variable-buffer-local 'notmuch-search-process-filter-data)
> > -
> >  (defun notmuch-search-process-sentinel (proc msg)
> >    "Add a message to let user know when \"notmuch search\" exits"
> >    (let ((buffer (process-buffer proc))
> >     (status (process-status proc))
> >     (exit-status (process-exit-status proc))
> >     (never-found-target-thread nil))
> > -    (if (memq status '(exit signal))
> > +    (when (memq status '(exit signal))
> > +   (kill-buffer (process-get proc 'parse-buf))
> >     (if (buffer-live-p buffer)
> >         (with-current-buffer buffer
> >           (save-excursion
> > @@ -577,8 +574,6 @@ This function advances the next thread when finished."
> >               (if (eq status 'signal)
> >                   (insert "Incomplete search results (search process was 
> > killed).\n"))
> >               (when (eq status 'exit)
> > -               (if notmuch-search-process-filter-data
> > -                   (insert (concat "Error: Unexpected output from notmuch 
> > search:\n" notmuch-search-process-filter-data)))
> >                 (insert "End of search results.")
> >                 (unless (= exit-status 0)
> >                   (insert (format " (process returned %d)" exit-status)))
> > @@ -757,45 +752,62 @@ non-authors is found, assume that all of the authors 
> > match."
> >      (insert (apply #'format string objects))
> >      (insert "\n")))
> >  
> > +(defvar notmuch-search-process-state nil
> > +  "Parsing state of the search process filter.")
> > +
> > +(defvar notmuch-search-json-parser nil
> > +  "Incremental JSON parser for the search process filter.")
> > +
> >  (defun notmuch-search-process-filter (proc string)
> >    "Process and filter the output of \"notmuch search\""
> > -  (let ((buffer (process-buffer proc)))
> > -    (if (buffer-live-p buffer)
> > -   (with-current-buffer buffer
> > -       (let ((line 0)
> > -             (more t)
> > -             (inhibit-read-only t)
> > -             (string (concat notmuch-search-process-filter-data string)))
> > -         (setq notmuch-search-process-filter-data nil)
> > -         (while more
> > -           (while (and (< line (length string)) (= (elt string line) ?\n))
> > -             (setq line (1+ line)))
> > -           (if (string-match "^thread:\\([0-9A-Fa-f]*\\) \\([^][]*\\) 
> > \\[\\([0-9]*\\)/\\([0-9]*\\)\\] \\([^;]*\\); \\(.*\\) (\\([^()]*\\))$" 
> > string line)
> > -               (let* ((thread-id (match-string 1 string))
> > -                      (tags-str (match-string 7 string))
> > -                      (result (list :thread thread-id
> > -                                    :date_relative (match-string 2 string)
> > -                                    :matched (string-to-number
> > -                                              (match-string 3 string))
> > -                                    :total (string-to-number
> > -                                            (match-string 4 string))
> > -                                    :authors (match-string 5 string)
> > -                                    :subject (match-string 6 string)
> > -                                    :tags (if tags-str
> > -                                              (save-match-data
> > -                                                (split-string 
> > tags-str))))))
> > -                 (if (/= (match-beginning 0) line)
> > -                     (notmuch-search-show-error
> > -                      (substring string line (match-beginning 0))))
> > -                 (notmuch-search-show-result result)
> > -                 (set 'line (match-end 0)))
> > -             (set 'more nil)
> > -             (while (and (< line (length string)) (= (elt string line) 
> > ?\n))
> > -               (setq line (1+ line)))
> > -             (if (< line (length string))
> > -                 (setq notmuch-search-process-filter-data (substring 
> > string line)))
> > -             ))))
> > -      (delete-process proc))))
> > +  (let ((results-buf (process-buffer proc))
> > +   (parse-buf (process-get proc 'parse-buf))
> > +   (inhibit-read-only t)
> > +   done)
> > +    (if (not (buffer-live-p results-buf))
> > +   (delete-process proc)
> > +      (with-current-buffer parse-buf
> > +   ;; Insert new data
> > +   (save-excursion
> > +     (goto-char (point-max))
> > +     (insert string)))
> > +      (with-current-buffer results-buf
> > +   (while (not done)
> > +     (condition-case nil
> > +         (case notmuch-search-process-state
> > +           ((begin)
> > +            ;; Enter the results list
> > +            (if (eq (notmuch-json-begin-compound
> > +                     notmuch-search-json-parser) 'retry)
> > +                (setq done t)
> > +              (setq notmuch-search-process-state 'result)))
> > +           ((result)
> > +            ;; Parse a result
> > +            (let ((result (notmuch-json-read notmuch-search-json-parser)))
> > +              (case result
> > +                ((retry) (setq done t))
> > +                ((end) (setq notmuch-search-process-state 'end))
> > +                (otherwise (notmuch-search-show-result result)))))
> > +           ((end)
> > +            ;; Any trailing data is unexpected
> > +            (with-current-buffer parse-buf
> > +              (skip-chars-forward " \t\r\n")
> > +              (if (eobp)
> > +                  (setq done t)
> > +                (signal 'json-error nil)))))
> 
> This looks good to me. Would it make sense to put the "Any trailing
> data" as a tiny function in notmuch-lib? something like 
> 
> (defun notmuch-json-assert-end-of-data ()
>    (skip-chars-forward " \t\r\n") 
>    (if (eobp)
>        (setq done t) 
>      (signal 'json-error nil)))

Also a good idea.

Thanks for the review!  I'll be sending v2 shortly.

> Best wishes
> 
> Mark

Reply via email to