git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] json_writer: new routines to create data in JSON format

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 21, 2018, 21:25 UTC
Message-ID
<xmqq8talnmts.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20180321192827.44330-2-git@jeffhostetler.com>
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...?

Show 11 quoted lines
> 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?
Show 9 quoted lines
> +
> +/*
> + * 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.
Show 6 quoted lines
> +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;
	};
Show 12 quoted lines
> +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".
Previous: git@jeffhostetler.comNext: Jeff Hostetler
Message 3 of 10 in “routines to generate JSON data”
  1. routines to generate JSON datagit@jeffhostetler.com, Mar 21, 2018
  2. json_writer: new routines to create data in JSON formatgit@jeffhostetler.com, Mar 21, 2018
  3. Junio C HamanoMar 21, 2018
  4. Jeff HostetlerMar 23, 2018
  5. René ScharfeMar 23, 2018
  6. Jeff HostetlerMar 23, 2018
  7. René ScharfeMar 23, 2018
  8. Jeff HostetlerMar 23, 2018
  9. Jeff KingMar 22, 2018
  10. Ævar Arnfjörð BjarmasonMar 22, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.