From: Jeff Hostetler <git@jeffhostetler.com>
To: Junio C Hamano <gitster@pobox.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: Fri, 23 Mar 2018 10:13:08 -0400 [thread overview]
Message-ID: <d6ef7034-351f-c1ef-a5d7-8ed684078ba7@jeffhostetler.com> (raw)
In-Reply-To: <xmqq8talnmts.fsf@gitster-ct.c.googlers.com>
On 3/21/2018 5:25 PM, Junio C Hamano wrote:
> 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?
Global.
Sorry, very old habits.
>> +
>> +/*
>> + * 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.
either way is fine. will fix.
>> +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;
> };
make sense. will fix.
>> +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".
yeah, that makes sense.
Thanks
Jeff
next prev parent reply other threads:[~2018-03-23 14:13 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
2018-03-23 14:13 ` Jeff Hostetler [this message]
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=d6ef7034-351f-c1ef-a5d7-8ed684078ba7@jeffhostetler.com \
--to=git@jeffhostetler.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--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).