Conflict of interest between JSON_C_TO_STRING_SPACED and JSON_C_TO_STRING_PRETTY

[lhunath]

The goal of JSON_C_TO_STRING_SPACED is to include minimal spacing in the output.

The goal of JSON_C_TO_STRING_PRETTY is to indent the output.

JSON_C_TO_STRING_SPACED's minimal spacing includes spacing inside objects and spacing after colons.

JSON_C_TO_STRING_PRETTY's indenting includes spacing inside objects and newlines.

If you want output which has both minimal spacing and is indented (eg. indenting + space after colon), you need to enable both flags, but this currently causes bad indenting.

Therefore, I propose that if JSON_C_TO_STRING_PRETTY is enabled, JSON_C_TO_STRING_SPACED's object-spacing is ignored (because pretty already provides its own object spacing). Alternatively, JSON_C_TO_STRING_PRETTY could be changed to be a complete subset of JSON_C_TO_STRING_SPACED's minimal spacing, which means it would also have to add a space after the colon.

As implemented right now, there is a strange conflict where pretty is not spacing + indenting, and to get all spacing features, your indenting cannot be done right.

[lhunath]

First proposal, "if JSON_C_TO_STRING_PRETTY is enabled, JSON_C_TO_STRING_SPACED's object-spacing is ignored (because pretty already provides its own object spacing)":

diff --git json_object.c json_object.c
index 8a86bc6..c5bca7c 100644
--- json_object.c
+++ json_object.c
@@ -395,7 +395,7 @@ static int json_object_object_to_json_string(struct json_object* jso,
                                printbuf_strappend(pb, "\n");
                }
                had_children = 1;
-               if (flags & JSON_C_TO_STRING_SPACED)
+               if (flags & JSON_C_TO_STRING_SPACED && !(flags & JSON_C_TO_STRING_PRETTY))
                        printbuf_strappend(pb, " ");
                indent(pb, level+1, flags);
                printbuf_strappend(pb, "\"");
@@ -416,7 +416,7 @@ static int json_object_object_to_json_string(struct json_object* jso,
                        printbuf_strappend(pb, "\n");
                indent(pb,level,flags);
        }
-       if (flags & JSON_C_TO_STRING_SPACED)
+       if (flags & JSON_C_TO_STRING_SPACED && !(flags & JSON_C_TO_STRING_PRETTY))
                return printbuf_strappend(pb, /*{*/ " }");
        else
                return printbuf_strappend(pb, /*{*/ "}");
@@ -1127,7 +1127,7 @@ static int json_object_array_to_json_string(struct json_object* jso,
                                printbuf_strappend(pb, "\n");
                }
                had_children = 1;
-               if (flags & JSON_C_TO_STRING_SPACED)
+               if (flags & JSON_C_TO_STRING_SPACED && !(flags & JSON_C_TO_STRING_PRETTY))
                        printbuf_strappend(pb, " ");
                indent(pb, level + 1, flags);
                val = json_object_array_get_idx(jso, ii);
@@ -1144,7 +1144,7 @@ static int json_object_array_to_json_string(struct json_object* jso,
                indent(pb,level,flags);
        }

-       if (flags & JSON_C_TO_STRING_SPACED)
+       if (flags & JSON_C_TO_STRING_SPACED && !(flags & JSON_C_TO_STRING_PRETTY))
                return printbuf_strappend(pb, " ]");
        return printbuf_strappend(pb, "]");
 }

[hawicz]

A simpler change would be to just clear the JSON_C_TO_STRING_PRETTY bit at the start of the function.

[lhunath]

Not entirely sure I understand what you're suggesting. JSON_C_TO_STRING_PRETTY and JSON_C_TO_STRING_SPACED definitely have a distinct effect and likely shouldn't just be ignored.

[hawicz]

er.. yeah, never mind my previous comment.

[dota17]

Nice issue and it could be closed @hawicz

[hawicz]

Long since fixed, closing.