From: Ethan Glasser-Camp <ethan.glasser.camp@gmail.com>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v3 2/3] emacs: Rename incremental JSON internal variables.
In-Reply-To: <1351037602-11157-3-git-send-email-markwalters1009@gmail.com>
References: <1351037602-11157-1-git-send-email-markwalters1009@gmail.com>
	<1351037602-11157-3-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.14+45~g6ea9330 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Tue, 23 Oct 2012 21:36:52 -0400
Message-ID: <876260k5vv.fsf@betacantrips.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

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