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

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

Date: Tue, 23 Oct 2012 21:36:52 -0400

To: Mark Walters, notmuch@notmuchmail.org

Cc:

From: 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

Thread: