Re: [PATCH 2/2] notmuch.el:notmuch-search-process-filter: Rewritten. Cope with incomplete lines.

[Austin Clements <amdragon@mit.edu>]

Nice catch.

Is there a reason you keep the remaining data in a string instead of
taking the more idiomatic elisp approach of leaving it in the process
buffer?  In fact, the code would probably be simpler if you
immediately appended the string to the process buffer like a normal
process-filter and then peeled things away using buffer-oriented
regexp functions like looking-at.  Elisp is a lot better at
manipulating buffers than it is at manipulating strings.

On Wed, Feb 2, 2011 at 6:56 PM, Thomas Schwinge <thomas@schwinge.name> wrote:
> This issue has been lying in ambush as of 2009-11-24's commit
> 93af7b574598637c2766dd1f8ef343962c9a8efb.
>
> Signed-off-by: Thomas Schwinge <thomas@schwinge.name>
> ---
>  emacs/notmuch.el |   70 +++++++++++++++++++++++++++++++----------------------
>  1 files changed, 41 insertions(+), 29 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 3d82f0d..35ccee6 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -641,9 +641,6 @@ non-authors is found, assume that all of the authors match."
>     (propertize authors 'face 'notmuch-search-matching-authors)))
>
>  (defun notmuch-search-insert-authors (format-string authors)
> -  ;; Save the match data to avoid interfering with
> -  ;; `notmuch-search-process-filter'.
> -  (save-match-data
>     (let* ((formatted-authors (format format-string authors))
>           (formatted-sample (format format-string ""))
>           (visible-string formatted-authors)
> @@ -709,7 +706,7 @@ non-authors is found, assume that all of the authors match."
>          (setq overlay (make-overlay start (point)))
>          (overlay-put overlay 'invisible invis-spec)
>          (overlay-put overlay 'isearch-open-invisible #'notmuch-search-isearch-authors-show)))
> -      (insert padding))))
> +      (insert padding)))
>
>  (defun notmuch-search-insert-field (field date count authors subject tags)
>   (cond
> @@ -736,6 +733,10 @@ non-authors is found, assume that all of the authors match."
>          do (notmuch-search-insert-field field date count authors subject tags)))
>   (insert "\n"))
>
> +(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-filter (proc string)
>   "Process and filter the output of \"notmuch search\""
>   (let ((buffer (process-buffer proc))
> @@ -743,31 +744,41 @@ non-authors is found, assume that all of the authors match."
>     (if (buffer-live-p buffer)
>        (with-current-buffer buffer
>          (save-excursion
> -           (let ((line 0)
> -                 (more t)
> -                 (inhibit-read-only t))
> -             (while more
> -               (if (string-match "^\\(thread:[0-9A-Fa-f]*\\) \\([^][]*\\) \\(\\[[0-9/]*\\]\\) \\([^;]*\\); \\(.*\\) (\\([^()]*\\))$" string line)
> -                   (let* ((thread-id (match-string 1 string))
> -                          (date (match-string 2 string))
> -                          (count (match-string 3 string))
> -                          (authors (match-string 4 string))
> -                          (subject (match-string 5 string))
> -                          (tags (match-string 6 string))
> -                          (tag-list (if tags (save-match-data (split-string tags)))))
> -                     (goto-char (point-max))
> -                     (let ((beg (point-marker)))
> -                       (notmuch-search-show-result date count authors subject tags)
> -                       (notmuch-search-color-line beg (point-marker) tag-list)
> -                       (put-text-property beg (point-marker) 'notmuch-search-thread-id thread-id)
> -                       (put-text-property beg (point-marker) 'notmuch-search-authors authors)
> -                       (put-text-property beg (point-marker) 'notmuch-search-subject subject)
> -                       (if (string= thread-id notmuch-search-target-thread)
> -                           (progn
> -                             (set 'found-target beg)
> -                             (set 'notmuch-search-target-thread "found"))))
> -                     (set 'line (match-end 0)))
> -                 (set 'more nil)))))
> +           (let ((inhibit-read-only t)
> +                 ;; We may have a partial line saved from the last iteration.
> +                 (string (concat notmuch-search-process-filter-data string))
> +                 (start 0))
> +             (goto-char (point-max))
> +             ;; Split `string' into lines.
> +             (while (string-match "\n" string start)
> +               (let ((line (substring string start (match-beginning 0))))
> +                 ;; Save the beginning of the next line already here, so that
> +                 ;; we can mangle the match data later on.
> +                 (setq start (match-end 0))
> +                 (if (string-match
> +                      "^\\(thread:[0-9A-Fa-f]*\\) \\([^][]*\\) \\(\\[[0-9/]*\\]\\) \\([^;]*\\); \\(.*\\) (\\([^()]*\\))$"
> +                      line)
> +                     (let* ((thread-id (match-string 1 line))
> +                            (date (match-string 2 line))
> +                            (count (match-string 3 line))
> +                            (authors (match-string 4 line))
> +                            (subject (match-string 5 line))
> +                            (tags (match-string 6 line))
> +                            (tag-list (if tags (split-string tags))))
> +                       (let ((beg (point-marker)))
> +                         (notmuch-search-show-result date count authors subject tags)
> +                         (notmuch-search-color-line beg (point-marker) tag-list)
> +                         (put-text-property beg (point-marker) 'notmuch-search-thread-id thread-id)
> +                         (put-text-property beg (point-marker) 'notmuch-search-authors authors)
> +                         (put-text-property beg (point-marker) 'notmuch-search-subject subject)
> +                         (if (string= thread-id notmuch-search-target-thread)
> +                             (setq found-target beg
> +                                   notmuch-search-target-thread "found"))))
> +                   ;; Non-conforming line.
> +                   (insert (concat "Non-conforming line (ignored): <" line ">.\n")))))
> +             ;; Save the remainder after the last line break for the next
> +             ;; interation.
> +             (setq notmuch-search-process-filter-data (substring string start))))
>          (if found-target
>              (goto-char found-target)))
>       (delete-process proc))))
> @@ -858,6 +869,7 @@ The optional parameters are used as follows:
>                       "--sort=newest-first")
>                     query)))
>          (set-process-sentinel proc 'notmuch-search-process-sentinel)
> +         (setq notmuch-search-process-filter-data nil)
>          (set-process-filter proc 'notmuch-search-process-filter))))
>     (run-hooks 'notmuch-search-hook)))
>
> --
> 1.7.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>