On Mon, 30 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote: > This seems like a good idea, but as a generic interface, this seems > suboptimal. In particular, it seems odd that a parsing function would > have to know about a process and require the caller to set up various > process properties and buffer-local variables. What about something > dedicated to incrementally parsing lists, like the async parser but > more specialized? Something along the lines of, > > (defun notmuch-json-parse-partial-list (result-function error-function > &optional buffer) > "Parse a partial JSON list from BUFFER (or the current buffer). > > This function consumes a JSON list from BUFFER, applying > RESULT-FUNCTION to each complete value in the list. It operates > incrementally and should be called whenever the buffer has been > extended with additional data. > > If there is a syntax error, this will attempt to resynchronize with > the input and will apply ERROR-FUNCTION to any input that was > skipped. > > This calls RESULT-FUNCTION and ERROR-FUNCTION with the same current > buffer as this function is called with (that is, this function does > not visibly switch to BUFFER)." > ...) > > This could use buffer-local variables for tracking the async parser > state as well its own state. It could even set these up automatically > when called for the first time in a buffer without them set, making it > very DWIM. It would clearly require a little more help from the > caller for process management than your patch does (and a little less > for parser setup), but I think the genericity would be worth it. This all seems good: I will send a draft patch in a reply to this email. The one change I have made is to make the function called with current-buffer the parse-buffer and give the results-buffer as an argument. This seems more natural to me as the caller has probably just added data to the parse-buffer, and it slightly simplifies the function. The other change from the current state is that the parser and state variable are buffer local to the parse-buffer rather than the results-buffer. Best wishes Mark > > Quoth Mark Walters on Jul 28 at 12:48 pm: >> >> We separate out the json parser into its own function. >> --- >> >> Hi >> >> Notmuch pick uses the new asynchronous json parser and the code to do so >> is almost identical to that for the search mode. Thus separate out the >> parsing in search mode into a more general function that can easily be >> used by both pick and search. >> >> This saves nearly 50 lines of duplicated code in notmuch-pick.el. >> >> The function notmuch-json-async-parse should probably be move in >> notmuch-lib but that can be a follow on patch. >> >> Best wishes >> >> Mark >> >> emacs/notmuch.el | 46 ++++++++++++++++++++++++++++++++++++---------- >> 1 files changed, 36 insertions(+), 10 deletions(-) >> >> diff --git a/emacs/notmuch.el b/emacs/notmuch.el >> index fd1836f..ee01028 100644 >> --- a/emacs/notmuch.el >> +++ b/emacs/notmuch.el >> @@ -816,7 +816,32 @@ non-authors is found, assume that all of the authors match." >> "Incremental JSON parser for the search process filter.") >> >> (defun notmuch-search-process-filter (proc string) >> - "Process and filter the output of \"notmuch search\"" >> + "Process and filter the output of \"notmuch search\" using the asynchronous parser." >> + (setq notmuch-search-process-state >> + (notmuch-json-async-parse proc >> + string >> + notmuch-search-process-state >> + notmuch-search-json-parser >> + 'notmuch-search-show-result >> + 'notmuch-search-show-error))) >> + >> +(defun notmuch-json-async-parse (proc string process-state parser result-function error-function) >> + "Process and filter the output using the asynchronous parser. >> + >> +This function steps into the first level of JSON nesting and then >> +applies RESULT-FUNCTION to each complete JSON object as it comes >> +in. >> + >> +PROC is the process: it should have a results buffer as >> +process-buffer and a 'parse-buf for the incoming json. >> +PROCESS-STATE the current state of filter process >> +STRING the incoming data >> +PARSER the parser >> +RESULT-FUNCTION a function to call on complete pieces of json >> +ERROR-FUNCTION the function to call on errors >> + >> +The function returns the new PROCESS-STATE" >> + >> (let ((results-buf (process-buffer proc)) >> (parse-buf (process-get proc 'parse-buf)) >> (inhibit-read-only t) >> @@ -831,28 +856,28 @@ non-authors is found, assume that all of the authors match." >> (with-current-buffer results-buf >> (while (not done) >> (condition-case nil >> - (case notmuch-search-process-state >> + (case process-state >> ((begin) >> ;; Enter the results list >> (if (eq (notmuch-json-begin-compound >> - notmuch-search-json-parser) 'retry) >> + parser) 'retry) >> (setq done t) >> - (setq notmuch-search-process-state 'result))) >> + (setq process-state 'result))) >> ((result) >> ;; Parse a result >> - (let ((result (notmuch-json-read notmuch-search-json-parser))) >> + (let ((result (notmuch-json-read parser))) >> (case result >> ((retry) (setq done t)) >> - ((end) (setq notmuch-search-process-state 'end)) >> - (otherwise (notmuch-search-show-result result))))) >> + ((end) (setq process-state 'end)) >> + (otherwise (funcall result-function result))))) >> ((end) >> ;; Any trailing data is unexpected >> - (notmuch-json-eof notmuch-search-json-parser) >> + (notmuch-json-eof parser) >> (setq done t))) >> (json-error >> ;; Do our best to resynchronize and ensure forward >> ;; progress >> - (notmuch-search-show-error >> + (funcall error-function >> "%s" >> (with-current-buffer parse-buf >> (let ((bad (buffer-substring (line-beginning-position) >> @@ -861,7 +886,8 @@ non-authors is found, assume that all of the authors match." >> bad)))))) >> ;; Clear out what we've parsed >> (with-current-buffer parse-buf >> - (delete-region (point-min) (point))))))) >> + (delete-region (point-min) (point)))) >> + process-state))) >> >> (defun notmuch-search-tag-all (&optional tag-changes) >> "Add/remove tags from all messages in current search buffer.