Re: [PATCH] emacs: move async json parser to its own function

[Austin Clements <amdragon@MIT.EDU>]

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.

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.