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

[Mark Walters]

On Mon, 30 Jul 2012, Austin Clements <amdragon at 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.