From: Ethan Glasser-Camp <ethan.glasser.camp@gmail.com>
To: Mark Walters <markwalters1009@gmail.com>,
	Austin Clements <amdragon@MIT.EDU>
Subject: Re: [PATCH v2 (Draft)] emacs: split async json parser into utility
	function
In-Reply-To: <87k3xlyoek.fsf@qmul.ac.uk>
References: <87k3xo85tv.fsf@qmul.ac.uk> <20120730013506.GF8502@mit.edu>
	<87obmxyokt.fsf@qmul.ac.uk> <87k3xlyoek.fsf@qmul.ac.uk>
User-Agent: Notmuch/0.14+45~g6ea9330 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Fri, 19 Oct 2012 20:30:07 -0400
Message-ID: <87ehku6l4g.fsf@betacantrips.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: notmuch@notmuchmail.org
Precedence: list

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