From: Junio C Hamano <gitster@pobox.com>
To: git@jeffhostetler.com
Cc: git@vger.kernel.org, peff@peff.net, avarab@gmail.com,
Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH v2] json_writer: new routines to create data in JSON format
Date: Wed, 21 Mar 2018 14:25:03 -0700 [thread overview]
Message-ID: <xmqq8talnmts.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180321192827.44330-2-git@jeffhostetler.com> (git@jeffhostetler.com's message of "Wed, 21 Mar 2018 19:28:27 +0000")
git@jeffhostetler.com writes:
> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Add basic routines to generate data in JSON format.
And the point of having capability to write JSON data in our
codebase is...?
> diff --git a/json-writer.c b/json-writer.c
> new file mode 100644
> index 0000000..89a6abb
> --- /dev/null
> +++ b/json-writer.c
> @@ -0,0 +1,321 @@
> +#include "cache.h"
> +#include "json-writer.h"
> +
> +static char g_ch_open[2] = { '{', '[' };
> +static char g_ch_close[2] = { '}', ']' };
What's "g_" prefix?
> +
> +/*
> + * Append JSON-quoted version of the given string to 'out'.
> + */
> +static void append_quoted_string(struct strbuf *out, const char *in)
> +{
> + strbuf_addch(out, '"');
> + for (/**/; *in; in++) {
> + unsigned char c = (unsigned char)*in;
It is clear enough to lose /**/, i.e.
for (; *in; in++) {
but for this one. I wonder if
unsigned char c;
strbuf_addch(out, '"');
while ((c = *in++) != '\0') {
...
is easier to follow, though.
> +static inline void begin(struct json_writer *jw, int is_array)
> +{
> + ALLOC_GROW(jw->levels, jw->nr + 1, jw->alloc);
> +
> + jw->levels[jw->nr].level_is_array = !!is_array;
> + jw->levels[jw->nr].level_is_empty = 1;
An element of this array is a struct that represents a level, and
everybody who accesses an element of that type knows it is talking
about a level by the field that has the array being named as
.levels[] (also [*1*]). In such a context, it is a bit too loud to
name the fields with level_$blah. IOW,
struct json_writer_level
{
unsigned is_array : 1;
unsigned is_empty : 1;
};
> +struct json_writer_level
> +{
> + unsigned level_is_array : 1;
> + unsigned level_is_empty : 1;
> +};
> +
> +struct json_writer
> +{
> + struct json_writer_level *levels;
> + int nr, alloc;
> + struct strbuf json;
> +};
[Footnote]
*1* I personally prefer to call an array of things "thing[]", not
"things[]", because then you can refer to an individual element
e.g. "thing[4]" and read it as "the fourth thing".
Unless the code often treats an array as a whole, that is, in
which case, things[] is OK as you'll be calling the whole thing
with the plural name (e.g. call that function and give all the
things by passing things[]).
In this case, one level instance is an element of a stack, and
the code would be accessing one level at a time most of the
time, so "writer.level[4].is_empty" would read more naturally
than "writer.levels[4].level_is_empty".
next prev parent reply other threads:[~2018-03-21 21:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-21 19:28 [PATCH v2] routines to generate JSON data git
2018-03-21 19:28 ` [PATCH v2] json_writer: new routines to create data in JSON format git
2018-03-21 21:25 ` Junio C Hamano [this message]
2018-03-23 14:13 ` Jeff Hostetler
2018-03-23 18:01 ` René Scharfe
2018-03-23 19:55 ` Jeff Hostetler
2018-03-23 20:11 ` René Scharfe
2018-03-23 20:32 ` Jeff Hostetler
2018-03-22 6:05 ` [PATCH v2] routines to generate JSON data Jeff King
2018-03-22 8:38 ` Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq8talnmts.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=git@jeffhostetler.com \
--cc=git@vger.kernel.org \
--cc=jeffhost@microsoft.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).