From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Cc: tomi.ollila@iki.fi
Subject: Re: [PATCH v2 0/9] JSON-based search-mode
Date: Thu, 05 Jul 2012 22:44:30 +0100 [thread overview]
Message-ID: <87y5mx3mtd.fsf@qmul.ac.uk> (raw)
In-Reply-To: <1341521547-15502-1-git-send-email-amdragon@mit.edu>
On Thu, 05 Jul 2012, Austin Clements <amdragon@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.
>
> (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)?
> +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?
Best wishes
Mark
> (json-error
> ;; Do our best to resynchronize and ensure forward
> ;; progress
next prev parent reply other threads:[~2012-07-05 21:44 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-03 22:20 [PATCH 0/8] JSON-based search-mode Austin Clements
2012-07-03 22:20 ` [PATCH 1/8] emacs: Clean up notmuch-search-show-result Austin Clements
2012-07-04 7:53 ` Mark Walters
2012-07-04 16:22 ` Austin Clements
2012-07-04 16:31 ` Tomi Ollila
2012-07-04 20:47 ` Mark Walters
2012-07-04 21:00 ` Austin Clements
2012-07-03 22:20 ` [PATCH 2/8] emacs: Separate search line parsing and display Austin Clements
2012-07-03 22:20 ` [PATCH 3/8] emacs: Move search-target logic to `notmuch-search-show-result' Austin Clements
2012-07-04 8:34 ` Mark Walters
2012-07-04 16:17 ` Austin Clements
2012-07-03 22:20 ` [PATCH 4/8] emacs: Helper for reporting search parsing errors Austin Clements
2012-07-04 8:41 ` Mark Walters
2012-07-03 22:20 ` [PATCH 5/8] emacs: Pass plist to `notmuch-search-show-result' Austin Clements
2012-07-03 22:20 ` [PATCH 6/8] test: New test for incremental search output parsing Austin Clements
2012-07-03 22:20 ` [PATCH 7/8] emacs: Implement an incremental JSON parser Austin Clements
2012-07-05 8:30 ` Mark Walters
2012-07-05 18:36 ` Austin Clements
2012-07-03 22:20 ` [PATCH 8/8] emacs: Switch from text to JSON format for search results Austin Clements
2012-07-05 8:37 ` Mark Walters
2012-07-05 18:58 ` Austin Clements
2012-07-04 16:37 ` [PATCH 0/8] JSON-based search-mode Tomi Ollila
2012-07-05 20:52 ` [PATCH v2 0/9] " Austin Clements
2012-07-05 20:52 ` [PATCH v2 1/9] emacs: Clean up notmuch-search-show-result Austin Clements
2012-07-05 20:52 ` [PATCH v2 2/9] emacs: Separate search line parsing and display Austin Clements
2012-07-05 20:52 ` [PATCH v2 3/9] emacs: Helper for reporting search parsing errors Austin Clements
2012-07-05 20:52 ` [PATCH v2 4/9] emacs: Move search-target logic to `notmuch-search-show-result' Austin Clements
2012-07-05 20:52 ` [PATCH v2 5/9] emacs: Pass plist " Austin Clements
2012-07-05 20:52 ` [PATCH v2 6/9] test: New test for incremental search output parsing Austin Clements
2012-07-05 20:52 ` [PATCH v2 7/9] emacs: Implement an incremental JSON parser Austin Clements
2012-07-05 20:52 ` [PATCH v2 8/9] emacs: Switch from text to JSON format for search results Austin Clements
2012-07-05 20:52 ` [PATCH v2 9/9] News for JSON-based search Austin Clements
2012-07-05 21:44 ` Mark Walters [this message]
2012-07-06 0:29 ` [PATCH v2 0/9] JSON-based search-mode Austin Clements
2012-07-07 16:27 ` Mark Walters
2012-07-09 21:42 ` [PATCH v3 " Austin Clements
2012-07-09 21:42 ` [PATCH v3 1/9] emacs: Clean up notmuch-search-show-result Austin Clements
2012-07-13 3:14 ` David Bremner
2012-07-09 21:42 ` [PATCH v3 2/9] emacs: Separate search line parsing and display Austin Clements
2012-07-09 21:42 ` [PATCH v3 3/9] emacs: Helper for reporting search parsing errors Austin Clements
2012-07-09 21:42 ` [PATCH v3 4/9] emacs: Move search-target logic to `notmuch-search-show-result' Austin Clements
2012-07-09 21:42 ` [PATCH v3 5/9] emacs: Pass plist " Austin Clements
2012-07-09 21:42 ` [PATCH v3 6/9] test: New test for incremental search output parsing Austin Clements
2012-07-09 21:42 ` [PATCH v3 7/9] emacs: Implement an incremental JSON parser Austin Clements
2012-07-09 21:42 ` [PATCH v3 8/9] emacs: Switch from text to JSON format for search results Austin Clements
2012-07-09 21:42 ` [PATCH v3 9/9] News for JSON-based search Austin Clements
2012-07-11 6:55 ` [PATCH v3 0/9] JSON-based search-mode Mark Walters
2012-07-11 8:48 ` Tomi Ollila
2012-07-21 17:37 ` [PATCH v4 0/8] emacs: JSON-based search cleanups Austin Clements
2012-07-21 17:37 ` [PATCH v4 1/8] emacs: Record thread search result object in a text property Austin Clements
2012-07-21 17:37 ` [PATCH v4 2/8] emacs: Use text properties instead of overlays for tag coloring Austin Clements
2012-07-21 17:37 ` [PATCH v4 3/8] emacs: Update tags by rewriting the search result line in place Austin Clements
2012-07-21 17:37 ` [PATCH v4 4/8] emacs: Use result text properties for search result iteration Austin Clements
2012-07-21 17:37 ` [PATCH v4 5/8] emacs: Replace other search text properties with result property Austin Clements
2012-07-21 17:37 ` [PATCH v4 6/8] emacs: Allow custom tags formatting Austin Clements
2012-07-21 17:37 ` [PATCH v4 7/8] emacs: Fix navigation of multi-line search result formats Austin Clements
2012-08-02 6:51 ` Jani Nikula
2012-08-02 7:19 ` [PATCH] emacs: fix a bug introduced by the recent search cleanups Mark Walters
2012-08-02 7:59 ` Jani Nikula
2012-08-02 14:22 ` Austin Clements
2012-08-03 1:00 ` David Bremner
2012-07-21 17:37 ` [PATCH v4 8/8] News for " Austin Clements
2012-07-21 17:56 ` [PATCH v4 0/8] emacs: JSON-based " Austin Clements
2012-07-22 15:27 ` Mark Walters
2012-07-22 18:45 ` Jameson Graef Rollins
2012-07-24 12:34 ` David Bremner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y5mx3mtd.fsf@qmul.ac.uk \
--to=markwalters1009@gmail.com \
--cc=amdragon@MIT.EDU \
--cc=notmuch@notmuchmail.org \
--cc=tomi.ollila@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).