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

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

[Mark Walters]

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.
-- 
1.7.9.1

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

[Mark Walters]

Hi

This patch is buggy as it doesn't take buffer local variables from the
right buffer (so it goes wrong if the buffer changes while the process
is still running). I have a patch which seems to be correct but I want
to do a bit more testing before posting.

Best wishes

Mark



On Sat, 28 Jul 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> 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.
> -- 
> 1.7.9.1

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

[Austin Clements]

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.

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

[Mark Walters]

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.

[PATCH v2 (Draft)] emacs: split async json parser into utility function

[Mark Walters]

Split out the json parser into a utility function.
---

Most of this patch is code movement: but I don't see how to arrange the
patch to show that.

Best wishes

Mark

 emacs/notmuch.el |  101 +++++++++++++++++++++++++++++++-----------------------
 1 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index fd1836f..23ab499 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -809,11 +809,60 @@ non-authors is found, assume that all of the authors match."
     (insert (apply #'format string objects))
     (insert "\n")))
 
-(defvar notmuch-search-process-state nil
-  "Parsing state of the search process filter.")
-
-(defvar notmuch-search-json-parser nil
-  "Incremental JSON parser for the search process filter.")
+(defvar notmuch-json-state nil
+  "State of the JSON parser.")
+
+(defvar notmuch-json-parser nil
+  "Incremental JSON parser.")
+
+(defun notmuch-json-parse-partial-list (result-function error-function results-buf)
+  "Parse a partial JSON list from current buffer.
+
+This function consumes a JSON list from the current buffer,
+applying RESULT-FUNCTION in buffer RESULT-BUFFER 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 in buffer
+RESULT-BUFFER to any input that was skipped."
+  (let (done)
+    (unless (local-variable-p 'notmuch-json-parser)
+      (set (make-local-variable 'notmuch-json-parser)
+	   (notmuch-json-create-parser (current-buffer)))
+      (set (make-local-variable 'notmuch-json-state) 'begin))
+    (while (not done)
+      (condition-case nil
+	  (case notmuch-json-state
+		((begin)
+		 ;; Enter the results list
+		 (if (eq (notmuch-json-begin-compound
+			  notmuch-json-parser) 'retry)
+		     (setq done t)
+		   (setq notmuch-json-state 'result)))
+		((result)
+		 ;; Parse a result
+		 (let ((result (notmuch-json-read notmuch-json-parser)))
+		   (case result
+			 ((retry) (setq done t))
+			 ((end) (setq notmuch-json-state 'end))
+			 (otherwise (with-current-buffer results-buf
+				      (funcall result-function result))))))
+		((end)
+		 ;; Any trailing data is unexpected
+		 (notmuch-json-eof notmuch-json-parser)
+		 (setq done t)))
+	(json-error
+	 ;; Do our best to resynchronize and ensure forward
+	 ;; progress
+	 (let ((bad (buffer-substring (line-beginning-position)
+				      (line-end-position))))
+	   (forward-line)
+	   (with-current-buffer results-buf
+	     (funcall error-function bad))))))
+    ;; Clear out what we've parsed
+    (delete-region (point-min) (point))))
 
 (defun notmuch-search-process-filter (proc string)
   "Process and filter the output of \"notmuch search\""
@@ -827,41 +876,10 @@ non-authors is found, assume that all of the authors match."
 	;; Insert new data
 	(save-excursion
 	  (goto-char (point-max))
-	  (insert string)))
-      (with-current-buffer results-buf
-	(while (not done)
-	  (condition-case nil
-	      (case notmuch-search-process-state
-		((begin)
-		 ;; Enter the results list
-		 (if (eq (notmuch-json-begin-compound
-			  notmuch-search-json-parser) 'retry)
-		     (setq done t)
-		   (setq notmuch-search-process-state 'result)))
-		((result)
-		 ;; Parse a result
-		 (let ((result (notmuch-json-read notmuch-search-json-parser)))
-		   (case result
-		     ((retry) (setq done t))
-		     ((end) (setq notmuch-search-process-state 'end))
-		     (otherwise (notmuch-search-show-result result)))))
-		((end)
-		 ;; Any trailing data is unexpected
-		 (notmuch-json-eof notmuch-search-json-parser)
-		 (setq done t)))
-	    (json-error
-	     ;; Do our best to resynchronize and ensure forward
-	     ;; progress
-	     (notmuch-search-show-error
-	      "%s"
-	      (with-current-buffer parse-buf
-		(let ((bad (buffer-substring (line-beginning-position)
-					     (line-end-position))))
-		  (forward-line)
-		  bad))))))
-	;; Clear out what we've parsed
-	(with-current-buffer parse-buf
-	  (delete-region (point-min) (point)))))))
+	  (insert string))
+	(notmuch-json-parse-partial-list 'notmuch-search-show-result
+					 'notmuch-search-show-error
+					 results-buf)))))
 
 (defun notmuch-search-tag-all (&optional tag-changes)
   "Add/remove tags from all messages in current search buffer.
@@ -973,9 +991,6 @@ Other optional parameters are used as follows:
 	      ;; This buffer will be killed by the sentinel, which
 	      ;; should be called no matter how the process dies.
 	      (parse-buf (generate-new-buffer " *notmuch search parse*")))
-	  (set (make-local-variable 'notmuch-search-process-state) 'begin)
-	  (set (make-local-variable 'notmuch-search-json-parser)
-	       (notmuch-json-create-parser parse-buf))
 	  (process-put proc 'parse-buf parse-buf)
 	  (set-process-sentinel proc 'notmuch-search-process-sentinel)
 	  (set-process-filter proc 'notmuch-search-process-filter)
-- 
1.7.9.1

Re: [PATCH v2 (Draft)] emacs: split async json parser into utility function

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Split out the json parser into a utility function.
> ---
>
> Most of this patch is code movement: but I don't see how to arrange the
> patch to show that.
>

"git gui blame notmuch.el" gives some clues, although it isn't perfect.
Maybe you could explain in the commit message what is _not_ code
movement?

> +(defvar notmuch-json-parser nil
> +  "Incremental JSON parser.")

This docstring needs expanding. I suppose that this is for internal use,
but it isn't clear from the docstring.

Re: [PATCH v2 (Draft)] emacs: split async json parser into utility function

[Ethan Glasser-Camp]

Mark Walters <markwalters1009@gmail.com> writes:

> Split out the json parser into a utility function.
> ---
>
> Most of this patch is code movement: but I don't see how to arrange the
> patch to show that.

Hi! This looks like a straightforward patch and if it will make
notmuch-pick more efficient, I'm in favor.

I tagged this patch moreinfo because David Bremner's suggestions that
you expand on the docstrings for notmuch-json-parser and
notmuch-json-state are good ideas. I'd also propose that you split this
patch into two patches -- one that pulls out the variables and performs
the renames, and the other which breaks out the code into its own
function. This should make the code movement more obvious. I haven't
started full-time work yet so if you would like me to do this, I can ;)

Based on David Bremner's feedback that it might be a good idea to have a
commit message that explains exactly what is code movement and isn't,
here's my proposal for a commit message.

Split out the json parser into a utility function.

This patch breaks out a chunk of notmuch-search-process-filter, with the
following changes:

- notmuch-search-json-parser becomes notmuch-json-parser.
- notmuch-search-parser-state becomes notmuch-json-state.

We also rearrange the json-error case but are careful to always call
error-function in the results buffer.



Ethan