[PATCH v3 0/3] split async json parser into utility function

[PATCH v3 0/3] split async json parser into utility function

[Mark Walters]

This series splits the async json parser into its own function. The
previous version is at id:"87k3xlyoek.fsf@qmul.ac.uk". 

This version splits that patch into three bits: the first contains all
the actual changes, the second changes some variable names and fixes
the whitespace, and the third just moves some code. 

The only functional change compared to that version is a small
correction to the way the error function was called. It also gives a
better docstring for some of the variables.

Best wishes

Mark



Mark Walters (3):
  emacs: Split out the incremental json parser into its own function.
  emacs: Rename incremental JSON internal variables.
  emacs: Code movement for the incremental JSON parser.

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

-- 
1.7.9.1

[PATCH v3 1/3] emacs: Split out the incremental json parser into its own function.

[Mark Walters]

This patch splits out the incremental json parser into its own
function.

It moves the main logic of the parser to happen inside the parse
buffer rather than inside the results buffer, but makes sure all
results and all errors are displayed in the results buffer.

It also changes the local parser variables from being buffer
local to the results buffer to being buffer local to the parse buffer,
and sets them up automatically so the caller does not need to.

Finally to keep the diff small this patch does not fix the whitespace,
not complete the code movement (these are done in subsequent patches)
but it should contain all the functional changes.
---
 emacs/notmuch.el |   39 +++++++++++++++++++++++++++------------
 1 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 1c43d3e..72a73dc 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -830,8 +830,28 @@ 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
+	  (insert string))
+	(notmuch-json-parse-partial-list 'notmuch-search-show-result
+					 'notmuch-search-show-error
+					 results-buf)))))
+
+(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-search-json-parser)
+      (set (make-local-variable 'notmuch-search-json-parser)
+	   (notmuch-json-create-parser (current-buffer)))
+      (set (make-local-variable 'notmuch-search-process-state) 'begin))
 	(while (not done)
 	  (condition-case nil
 	      (case notmuch-search-process-state
@@ -847,7 +867,8 @@ non-authors is found, assume that all of the authors match."
 		   (case result
 		     ((retry) (setq done t))
 		     ((end) (setq notmuch-search-process-state 'end))
-		     (otherwise (notmuch-search-show-result result)))))
+			 (otherwise (with-current-buffer results-buf
+				      (funcall result-function result))))))
 		((end)
 		 ;; Any trailing data is unexpected
 		 (notmuch-json-eof notmuch-search-json-parser)
@@ -855,16 +876,13 @@ non-authors is found, assume that all of the authors match."
 	    (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))))))
+	   (with-current-buffer results-buf
+	     (funcall error-function "%s" bad))))))
 	;; Clear out what we've parsed
-	(with-current-buffer parse-buf
-	  (delete-region (point-min) (point)))))))
+    (delete-region (point-min) (point))))
 
 (defun notmuch-search-tag-all (&optional tag-changes)
   "Add/remove tags from all messages in current search buffer.
@@ -976,9 +994,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

[PATCH v3 2/3] emacs: Rename incremental JSON internal variables.

[Mark Walters]

This patch just renames the internal variables for the JSON parser now
it is no longer specific to search mode. It also fixes up the white
space after the previous patch. There should be no functional changes.
---
 emacs/notmuch.el |   46 +++++++++++++++++++++++-----------------------
 1 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 72a73dc..7e5d052 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -812,11 +812,11 @@ 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-json-state nil
+  "State of the internal JSON parser.")
 
-(defvar notmuch-search-json-parser nil
-  "Incremental JSON parser for the search process filter.")
+(defvar notmuch-json-parser nil
+  "Internal Incremental JSON parser Object.")
 
 (defun notmuch-search-process-filter (proc string)
   "Process and filter the output of \"notmuch search\""
@@ -848,40 +848,40 @@ 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-search-json-parser)
-      (set (make-local-variable 'notmuch-search-json-parser)
+    (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-search-process-state) 'begin))
-	(while (not done)
-	  (condition-case nil
-	      (case notmuch-search-process-state
+      (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-search-json-parser) 'retry)
+			  notmuch-json-parser) 'retry)
 		     (setq done t)
-		   (setq notmuch-search-process-state 'result)))
+		   (setq notmuch-json-state 'result)))
 		((result)
 		 ;; Parse a result
-		 (let ((result (notmuch-json-read notmuch-search-json-parser)))
+		 (let ((result (notmuch-json-read notmuch-json-parser)))
 		   (case result
-		     ((retry) (setq done t))
-		     ((end) (setq notmuch-search-process-state 'end))
+			 ((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-search-json-parser)
+		 (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)
+	(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 "%s" bad))))))
-	;; Clear out what we've parsed
+    ;; Clear out what we've parsed
     (delete-region (point-min) (point))))
 
 (defun notmuch-search-tag-all (&optional tag-changes)
-- 
1.7.9.1

[PATCH v3 3/3] emacs: Code movement for the incremental JSON parser.

[Mark Walters]

This just moves the notmuch-search-process-filter after the newly
split out incremental json parser. I think this removes a warning in
some versions of emacs.

There should be no functional change.
---
 emacs/notmuch.el |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 7e5d052..ab253b7 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -818,23 +818,6 @@ non-authors is found, assume that all of the authors match."
 (defvar notmuch-json-parser nil
   "Internal Incremental JSON parser Object.")
 
-(defun notmuch-search-process-filter (proc string)
-  "Process and filter the output of \"notmuch search\""
-  (let ((results-buf (process-buffer proc))
-	(parse-buf (process-get proc 'parse-buf))
-	(inhibit-read-only t)
-	done)
-    (if (not (buffer-live-p results-buf))
-	(delete-process proc)
-      (with-current-buffer parse-buf
-	;; Insert new data
-	(save-excursion
-	  (goto-char (point-max))
-	  (insert string))
-	(notmuch-json-parse-partial-list 'notmuch-search-show-result
-					 'notmuch-search-show-error
-					 results-buf)))))
-
 (defun notmuch-json-parse-partial-list (result-function error-function results-buf)
   "Parse a partial JSON list from current buffer.
 
@@ -884,6 +867,23 @@ RESULT-BUFFER to any input that was skipped."
     ;; 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\""
+  (let ((results-buf (process-buffer proc))
+	(parse-buf (process-get proc 'parse-buf))
+	(inhibit-read-only t)
+	done)
+    (if (not (buffer-live-p results-buf))
+	(delete-process proc)
+      (with-current-buffer parse-buf
+	;; Insert new data
+	(save-excursion
+	  (goto-char (point-max))
+	  (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.
 
-- 
1.7.9.1

Re: [PATCH v3 2/3] emacs: Rename incremental JSON internal variables.

[Ethan Glasser-Camp]

Mark Walters <markwalters1009@gmail.com> writes:

> This patch just renames the internal variables for the JSON parser now
> it is no longer specific to search mode. It also fixes up the white
> space after the previous patch. There should be no functional changes.

This series looks very good to me. I still have a couple of quibbles that I wouldn't say
should hold the series up. First, your commit messages have periods at
the end of the first line. I think it is considered good practice that
they not (see, e.g.,
http://blogs.gnome.org/danni/2011/10/25/a-guide-to-writing-git-commit-messages/ ).

> -(defvar notmuch-search-process-state nil
> -  "Parsing state of the search process filter.")
> +(defvar notmuch-json-state nil
> +  "State of the internal JSON parser.")

How about,

"State of the shared JSON parser.

This variable will be a symbol, corresponding to the state of the JSON
parser's state machine. 'begin means we haven't yet entered a JSON
list. 'result means we are ready to parse a JSON object. 'end means we
have exited the JSON list."

(Or whatever the correct meanings of the parser's states are...)

> -(defvar notmuch-search-json-parser nil
> -  "Incremental JSON parser for the search process filter.")
> +(defvar notmuch-json-parser nil
> +  "Internal Incremental JSON parser Object.")

How about,

"Shared notmuch JSON parser object, as created by
(notmuch-json-create-parser).

Any notmuch code can parse part of a JSON list object by setting this
variable to their JSON parser and calling
notmuch-json-parse-partial-list (which see)."

(Or whatever one is supposed to do with the parser object...)

Ethan