On 3/21/2018 5:25 PM, Junio C Hamano wrote:
g...@jeffhostetler.com writes:

From: Jeff Hostetler <jeffh...@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


Reply via email to