From: Austin Clements <amdragon@MIT.EDU>
To: notmuch@notmuchmail.org
Subject: [PATCH v2 0/9] JSON-based search-mode
Date: Thu,  5 Jul 2012 16:52:18 -0400
Message-Id: <1341521547-15502-1-git-send-email-amdragon@mit.edu>
In-Reply-To: <1341354059-29396-1-git-send-email-amdragon@mit.edu>
References: <1341354059-29396-1-git-send-email-amdragon@mit.edu>
Cc: tomi.ollila@iki.fi
Precedence: list

This should account for all of Mark's and Tomi's comments.  This
version
* renames the "format" variables to "format-string" and "spec" to be
  less confusing,
* reverts to the original behavior of ignoring the user's format
  specification for tags (since we make assumptions about this format
  elsewhere),
* swaps the error helper and search-target patches to fix the
  temporary issue with error message placement,
* adds documentation on point movement in the JSON parser,
* breaks out the JSON EOF testing function,
* beefs up a few commit messages,
* and adds a NEWS patch.

For ease of reviewing, the diff diff is below.

I've written most of a follow-on patch series that cleans up the
handling of metadata and tag changes by attaching the JSON result
object to the result.  This means we don't need a proliferation of
text properties to store the result metadata, and we can make updates
to a result (e.g., tag changes) by updating this result object and
then re-rendering the result line from scratch, rather than trying to
update the result line in place.  This makes it possible to obey user
formatting for the tag list and has other perks like recoloring
results when their tags change.  I'll send it along once this patch
series is accepted.

diff --git a/NEWS b/NEWS
index d29ec5b..a1a6e93 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,23 @@ Maildir tag synchronization
   messages (typically causing new messages to not receive the "unread"
   tag).
 
+Emacs Interface
+---------------
+
+Search now uses the JSON format internally
+
+  This should address problems with unusual characters in authors and
+  subject lines that could confuse the old text-based search parser.
+
+The date shown in search results is no longer padded before applying
+user-specified formatting
+
+  Previously, the date in the search results was padded to fixed width
+  before being formatted with `notmuch-search-result-format`.  It is
+  no longer padded.  The default format has been updated, but if
+  you've customized this variable, you may have to change your date
+  format from `"%s "` to `"%12s "`.
+
 Notmuch 0.13.2 (2012-06-02)
 ===========================
 
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index f7cda33..9e04d97 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -399,8 +399,9 @@ resynchronize after an error by moving point."
 
 Returns 'retry if there is insufficient input to parse the
 beginning of the compound.  If this is able to parse the
-beginning of a compound, it returns t and later calls to
-`notmuch-json-read' will return the compound's elements.
+beginning of a compound, it moves point past the token that opens
+the compound and returns t.  Later calls to `notmuch-json-read'
+will return the compound's elements.
 
 Entering JSON objects is current unimplemented."
 
@@ -423,7 +424,8 @@ Entering JSON objects is current unimplemented."
 Returns 'retry if there is insufficient input to parse a complete
 JSON value.  If the parser is currently inside a compound value
 and the next token ends the list or object, returns 'end.
-Otherwise, returns the value."
+Otherwise, moves point to just past the end of the value and
+returns the value."
 
   (with-current-buffer (notmuch-json-buffer jp)
     (or
@@ -474,11 +476,22 @@ Otherwise, returns the value."
 	       (notmuch-json-partial-pos jp) nil
 	       (notmuch-json-partial-state jp) nil)
 	 ;; Parse the value
-	 (let* ((json-object-type 'plist)
-		(json-array-type 'list)
-		(json-false nil))
+	 (let ((json-object-type 'plist)
+	       (json-array-type 'list)
+	       (json-false nil))
 	   (json-read)))))))
 
+(defun notmuch-json-eof (jp)
+  "Signal a json-error if there is more input in JP's buffer.
+
+Moves point to the beginning of any trailing garbage or to the
+end of the buffer if there is no trailing garbage."
+
+  (with-current-buffer (notmuch-json-buffer jp)
+    (skip-chars-forward " \t\r\n")
+    (unless (eobp)
+      (signal 'json-error (list "Trailing garbage following JSON data")))))
+
 (provide 'notmuch-lib)
 
 ;; Local Variables:
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 2a09a98..fabb7c0 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -702,28 +702,29 @@ non-authors is found, assume that all of the authors match."
 	  (overlay-put overlay 'isearch-open-invisible #'delete-overlay)))
       (insert padding))))
 
-(defun notmuch-search-insert-field (field format result)
+(defun notmuch-search-insert-field (field format-string result)
   (cond
    ((string-equal field "date")
-    (insert (propertize (format format (plist-get result :date_relative))
+    (insert (propertize (format format-string (plist-get result :date_relative))
 			'face 'notmuch-search-date)))
    ((string-equal field "count")
-    (insert (propertize (format format (format "[%s/%s]"
-					       (plist-get result :matched)
-					       (plist-get result :total)))
+    (insert (propertize (format format-string
+				(format "[%s/%s]" (plist-get result :matched)
+					(plist-get result :total)))
 			'face 'notmuch-search-count)))
    ((string-equal field "subject")
-    (insert (propertize (format format (plist-get result :subject))
+    (insert (propertize (format format-string (plist-get result :subject))
 			'face 'notmuch-search-subject)))
 
    ((string-equal field "authors")
-    (notmuch-search-insert-authors format (plist-get result :authors)))
+    (notmuch-search-insert-authors format-string (plist-get result :authors)))
 
    ((string-equal field "tags")
-    (insert
-     (format format (propertize
-		     (mapconcat 'identity (plist-get result :tags) " ")
-		     'font-lock-face 'notmuch-tag-face))))))
+    ;; Ignore format-string here because notmuch-search-set-tags
+    ;; depends on the format of this
+    (insert (concat "(" (propertize
+			 (mapconcat 'identity (plist-get result :tags) " ")
+			 'font-lock-face 'notmuch-tag-face) ")")))))
 
 (defun notmuch-search-show-result (result)
   ;; Ignore excluded matches
@@ -731,8 +732,8 @@ non-authors is found, assume that all of the authors match."
     (let ((beg (point-max)))
       (save-excursion
 	(goto-char beg)
-	(dolist (format notmuch-search-result-format)
-	  (notmuch-search-insert-field (car format) (cdr format) result))
+	(dolist (spec notmuch-search-result-format)
+	  (notmuch-search-insert-field (car spec) (cdr spec) result))
 	(insert "\n")
 	(notmuch-search-color-line beg (point) (plist-get result :tags))
 	(put-text-property beg (point) 'notmuch-search-thread-id
@@ -790,11 +791,8 @@ non-authors is found, assume that all of the authors match."
 		     (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)))))
+		 (notmuch-json-eof notmuch-search-json-parser)
+		 (setq done t)))
 	    (json-error
 	     ;; Do our best to resynchronize and ensure forward
 	     ;; progress