[PATCH v2 0/9] JSON-based search-mode
Mark Walters
markwalters1009 at gmail.com
Sat Jul 7 09:27:43 PDT 2012
On Fri, 06 Jul 2012, Austin Clements <amdragon at MIT.EDU> wrote:
> Quoth Mark Walters on Jul 05 at 10:44 pm:
>> On Thu, 05 Jul 2012, Austin Clements <amdragon at MIT.EDU> wrote:
>> > 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."
>>
>> I think that point can move when 'retry is returned (past terminators
>> and commas for example). It might also be worth saying that it returns
>> the next value passing command and terminators and whitespace after
>> point.
>
> How about
>
> Returns 'retry if there is insufficient input to parse a complete JSON
> value (though it may still move point over separators or whitespace).
> If the parser is currently inside a compound value and the next token
> ends the list or object, this moves point just past the terminator and
> returns 'end. Otherwise, this moves point to just past the end of the
> value and returns the value.
>
> ?
This sounds good.
>> >
>> > (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.
>>
>> Would `data' be better than `input' (to distinguish allowed whitespace
>> from disallowed data)?
>
> Ah, yes.
>
> "Signal a json-error if there is more data in JP's buffer.
>
> Moves point to the beginning of any trailing data or to the end
> of the buffer if there is only trailing whitespace."
>
> ?
This is good too.
>> > +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)))
>>
>> I think this used to only set `done' if there was not trailing data but
>> now does so anyway?
>
> It still won't set done if there's trailing data because
> notmuch-json-eof will signal an error, which will unwind to the
> condition-case (which will then consume said trailing data and done
> will get set on the next time through the loop).
Ah! I had overlooked that. It all looks good now.
Best wishes
Mark
More information about the notmuch
mailing list