git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GSoC PATCH v2 0/2] json-writer: describe the jw_* functions
@ 2025-05-12  2:09 Lucas Seiki Oshiro
  2025-05-12  2:09 ` [GSoC PATCH v2 1/2] json-writer: add docstrings to " Lucas Seiki Oshiro
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Lucas Seiki Oshiro @ 2025-05-12  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, ps, karthik.188, Lucas Seiki Oshiro

Hi!

Given that my GSoC project will need the json-writer module for serializing
JSON data, I studied this module and I thought it would be a nice contribution
to make it a little more clear on how to use it. This will be helpful for me and
I hope that it will also be for anyone who want to write JSON inside Git.

The main difference in this v2 is the second commit which provides an
overview on how to use the functions of this module, telling which one should 
be used for each use case. Perhaps only this usage overview is enough, but I'm 
open for suggestions!

PS: this is the first patchset that I'm sending after being approved in GSoC, so
this is also the first one with Mentored-by :-)

Lucas Seiki Oshiro (2):
  json-writer: add docstrings to jw_* functions
  json-writer: describe the usage of jw_* functions

 json-writer.h | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

-- 
2.39.5 (Apple Git-154)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [GSoC PATCH v2 1/2] json-writer: add docstrings to jw_* functions
  2025-05-12  2:09 [GSoC PATCH v2 0/2] json-writer: describe the jw_* functions Lucas Seiki Oshiro
@ 2025-05-12  2:09 ` Lucas Seiki Oshiro
  2025-05-12  8:50   ` Patrick Steinhardt
  2025-05-12  9:36   ` Karthik Nayak
  2025-05-12  2:09 ` [GSoC PATCH v2 2/2] json-writer: describe the usage of " Lucas Seiki Oshiro
  2025-05-12  8:49 ` [GSoC PATCH v2 0/2] json-writer: describe the " Karthik Nayak
  2 siblings, 2 replies; 12+ messages in thread
From: Lucas Seiki Oshiro @ 2025-05-12  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, ps, karthik.188, Lucas Seiki Oshiro

Add a docstring for each function that manipulates json_writers.

Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 json-writer.h | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/json-writer.h b/json-writer.h
index 04413bd1af..aa513e86cb 100644
--- a/json-writer.h
+++ b/json-writer.h
@@ -69,42 +69,175 @@ struct json_writer
 	.open_stack = STRBUF_INIT, \
 }
 
+/*
+ * Initialize a json_writer with empty values.
+ */
 void jw_init(struct json_writer *jw);
+
+/*
+ * Release the internal buffers of a json_writer.
+ */
 void jw_release(struct json_writer *jw);
 
+/*
+ * Begin the json_writer using an object as the top-level data structure. If
+ * pretty is set to 1, the result will be a human-readable and indented JSON,
+ * and if it is set to 0 the result will be minified single-line JSON.
+ */
 void jw_object_begin(struct json_writer *jw, int pretty);
+
+/*
+ * Begin the json_writer using an array as the top-level data structure. If
+ * pretty is set to 1, the result will be a human-readable and indented JSON,
+ * and if it is set to 0 the result will be minified single-line JSON.
+ */
 void jw_array_begin(struct json_writer *jw, int pretty);
 
+/*
+ * Append a string field to the current object of the json_writer, given its key
+ * and its value.
+ */
 void jw_object_string(struct json_writer *jw, const char *key,
 		      const char *value);
+
+/*
+ * Append an int field to the current object of the json_writer, given its key
+ * and its value.
+ */
 void jw_object_intmax(struct json_writer *jw, const char *key, intmax_t value);
+
+/*
+ * Append a double field to the current object of the json_writer, given its key
+ * and its value. The precision parameter can be used for specifying the number
+ * of decimals after the point, using -1 for formatting with the maximum
+ * precision available.
+ */
 void jw_object_double(struct json_writer *jw, const char *key, int precision,
 		      double value);
+
+/*
+ * Append a boolean field set to true to the current object of the json_writer,
+ * given its key.
+ */
 void jw_object_true(struct json_writer *jw, const char *key);
+
+/*
+ * Append a boolean field set to false to the current object of the json_writer,
+ * given its key.
+ */
 void jw_object_false(struct json_writer *jw, const char *key);
+
+/*
+ * Append a boolean field to the current object of the json_writer, given its
+ * key and its value.
+ */
 void jw_object_bool(struct json_writer *jw, const char *key, int value);
+
+/*
+ * Append a null field to the current object of the json_writer, given its key.
+ */
 void jw_object_null(struct json_writer *jw, const char *key);
+
+/*
+ * Append a field to the current object of the json_writer, given its key and
+ * another json_writer that represents its content.
+ */
 void jw_object_sub_jw(struct json_writer *jw, const char *key,
 		      const struct json_writer *value);
 
+/*
+ * Start an object as the value of a field in the current object of the
+ * json_writer, given the field key.
+ */
 void jw_object_inline_begin_object(struct json_writer *jw, const char *key);
+
+/*
+ * Start an array as the value of a field in the current object of the
+ * json_writer, given the field key.
+ */
 void jw_object_inline_begin_array(struct json_writer *jw, const char *key);
 
+/*
+ * Append a string value to the current array of the json_writer.
+ */
 void jw_array_string(struct json_writer *jw, const char *value);
+
+/*
+ * Append an int value to the current array of the json_writer.
+ */
 void jw_array_intmax(struct json_writer *jw, intmax_t value);
+
+/*
+ * Append a double value to the current array of the json_writer. The precision
+ * parameter can be used for specifying the number of decimals after the point,
+ * using -1 for formatting with the maximum precision available.
+ */
 void jw_array_double(struct json_writer *jw, int precision, double value);
+
+/*
+ * Append a true value to the current array of the json_writer.
+ */
 void jw_array_true(struct json_writer *jw);
+
+/*
+ * Append a false value to the current array of the json_writer.
+ */
 void jw_array_false(struct json_writer *jw);
+
+/*
+ * Append a boolean value to the current array of the json_writer.
+ */
 void jw_array_bool(struct json_writer *jw, int value);
+
+/*
+ * Append a null value to the current array of the json_writer.
+ */
 void jw_array_null(struct json_writer *jw);
+
+/*
+ * Append a value to the current array of the json_writer, given the
+ * json_writer that represents its content.
+ */
 void jw_array_sub_jw(struct json_writer *jw, const struct json_writer *value);
+
+/*
+ * Append the first argc values from the argv array of strings to the current
+ * array of the json_writer.
+ *
+ * This function does not provide safety for cases where the array has less than
+ * argc values.
+ */
 void jw_array_argc_argv(struct json_writer *jw, int argc, const char **argv);
+
+/*
+ * Append a null-terminated array of strings to the current array of the
+ * json_writer.
+ */
 void jw_array_argv(struct json_writer *jw, const char **argv);
 
+/*
+ * Start an object as a value in the current array of the json_writer.
+ */
 void jw_array_inline_begin_object(struct json_writer *jw);
+
+/*
+ * Start an array as a value in the current array.
+ */
 void jw_array_inline_begin_array(struct json_writer *jw);
 
+/*
+ * Return if the json_writer is terminated. In other words, if the all the
+ * objects and arrays are already closed.
+ */
 int jw_is_terminated(const struct json_writer *jw);
+
+/*
+ * Terminates the current object or array of the json_writer. In other words,
+ * append a ] if the current array is not closed or } if the current object
+ * is not closed.
+ *
+ * Abort the execution if there's no object or array that can be terminated.
+ */
 void jw_end(struct json_writer *jw);
 
 #endif /* JSON_WRITER_H */
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [GSoC PATCH v2 2/2] json-writer: describe the usage of jw_* functions
  2025-05-12  2:09 [GSoC PATCH v2 0/2] json-writer: describe the jw_* functions Lucas Seiki Oshiro
  2025-05-12  2:09 ` [GSoC PATCH v2 1/2] json-writer: add docstrings to " Lucas Seiki Oshiro
@ 2025-05-12  2:09 ` Lucas Seiki Oshiro
  2025-05-12  8:50   ` Patrick Steinhardt
  2025-05-12  9:41   ` Karthik Nayak
  2025-05-12  8:49 ` [GSoC PATCH v2 0/2] json-writer: describe the " Karthik Nayak
  2 siblings, 2 replies; 12+ messages in thread
From: Lucas Seiki Oshiro @ 2025-05-12  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, ps, karthik.188, Lucas Seiki Oshiro

Provide an overview of the set of functions used for manipulating
json_writers by describing what functions should be used for each
JSON-related task.

Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 json-writer.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/json-writer.h b/json-writer.h
index aa513e86cb..8b7470af67 100644
--- a/json-writer.h
+++ b/json-writer.h
@@ -28,6 +28,34 @@
  * object/array) -or- by building them inline in one pass.  This is a
  * personal style and/or data shape choice.
  *
+ * USAGE:
+ * ======
+ *
+ * - Initialize the json_writer with jw_init.
+ *
+ * - Open an object as the main data structure with jw_object_begin.
+ *   Append a key-value pair to it using the jw_object_<type> functions.
+ *   Conclude with jw_end.
+ *
+ * - Alternatively, open an array as the main data structure with
+ *   jw_array_begin. Append a value to it using the jw_array_<type>
+ *   functions. Conclude with jw_end.
+ *
+ * - Append a new, unterminated array or object to the current
+ *   object using the jw_object_inline_begin_{array, object} functions.
+ *   Similarly, append a new, unterminated array or object to
+ *   the current array using the jw_array_inline_begin_{array, object}
+ *   functions.
+ *
+ * - Append other json_writer as a value to the current array or object
+ *   using the jw_{array, object}_sub_jw functions.
+ *
+ * - Extend the current array with an null-terminated array of strings
+ *   by using jw_array_argv or with a fixed number of elements of a
+ *   array of string by using jw_array_argc_argv.
+ *
+ * - Relase the json_writer after using it by calling jw_release.
+ *
  * See t/helper/test-json-writer.c for various usage examples.
  *
  * LIMITATIONS:
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 0/2] json-writer: describe the jw_* functions
  2025-05-12  2:09 [GSoC PATCH v2 0/2] json-writer: describe the jw_* functions Lucas Seiki Oshiro
  2025-05-12  2:09 ` [GSoC PATCH v2 1/2] json-writer: add docstrings to " Lucas Seiki Oshiro
  2025-05-12  2:09 ` [GSoC PATCH v2 2/2] json-writer: describe the usage of " Lucas Seiki Oshiro
@ 2025-05-12  8:49 ` Karthik Nayak
  2 siblings, 0 replies; 12+ messages in thread
From: Karthik Nayak @ 2025-05-12  8:49 UTC (permalink / raw)
  To: Lucas Seiki Oshiro, git; +Cc: gitster, ps

[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

Hey Lucas,

> Hi!
>
> Given that my GSoC project will need the json-writer module for serializing
> JSON data, I studied this module and I thought it would be a nice contribution
> to make it a little more clear on how to use it. This will be helpful for me and
> I hope that it will also be for anyone who want to write JSON inside Git.
>
> The main difference in this v2 is the second commit which provides an
> overview on how to use the functions of this module, telling which one should
> be used for each use case. Perhaps only this usage overview is enough, but I'm
> open for suggestions!
>

It would be nice if you could reply in-line to previous versions or link
the same here, to help navigate through the older versions :)

> PS: this is the first patchset that I'm sending after being approved in GSoC, so
> this is also the first one with Mentored-by :-)
>
> Lucas Seiki Oshiro (2):
>   json-writer: add docstrings to jw_* functions
>   json-writer: describe the usage of jw_* functions
>
>  json-writer.h | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 161 insertions(+)
>
> --
> 2.39.5 (Apple Git-154)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 1/2] json-writer: add docstrings to jw_* functions
  2025-05-12  2:09 ` [GSoC PATCH v2 1/2] json-writer: add docstrings to " Lucas Seiki Oshiro
@ 2025-05-12  8:50   ` Patrick Steinhardt
  2025-05-13 22:05     ` Lucas Seiki Oshiro
  2025-05-12  9:36   ` Karthik Nayak
  1 sibling, 1 reply; 12+ messages in thread
From: Patrick Steinhardt @ 2025-05-12  8:50 UTC (permalink / raw)
  To: Lucas Seiki Oshiro; +Cc: git, gitster, karthik.188

On Sun, May 11, 2025 at 11:09:34PM -0300, Lucas Seiki Oshiro wrote:
> Add a docstring for each function that manipulates json_writers.
> 
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Mentored-by Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>

I don't think there's a need to add "Mentored-by" trailers to every
commit just because we happen to be your mentors right now :) If we
actually helped then sure, makes sense. But to the best of my knowledge
we didn't, so I'd just leave them out for now.

> diff --git a/json-writer.h b/json-writer.h
> index 04413bd1af..aa513e86cb 100644
> --- a/json-writer.h
> +++ b/json-writer.h
> @@ -69,42 +69,175 @@ struct json_writer
>  	.open_stack = STRBUF_INIT, \
>  }
>  
> +/*
> + * Initialize a json_writer with empty values.
> + */
>  void jw_init(struct json_writer *jw);
> +
> +/*
> + * Release the internal buffers of a json_writer.
> + */
>  void jw_release(struct json_writer *jw);
>  
> +/*
> + * Begin the json_writer using an object as the top-level data structure. If
> + * pretty is set to 1, the result will be a human-readable and indented JSON,
> + * and if it is set to 0 the result will be minified single-line JSON.
> + */
>  void jw_object_begin(struct json_writer *jw, int pretty);

I think it would be interesting to learn _when_ to use this function. Is
it mandatory to call it? Can it be nested? Why is there no corresponding
`jw_object_end()`?

> +/*
> + * Begin the json_writer using an array as the top-level data structure. If
> + * pretty is set to 1, the result will be a human-readable and indented JSON,
> + * and if it is set to 0 the result will be minified single-line JSON.
> + */
>  void jw_array_begin(struct json_writer *jw, int pretty);

Same questions here.

> +/*
> + * Append a string field to the current object of the json_writer, given its key
> + * and its value.
> + */
>  void jw_object_string(struct json_writer *jw, const char *key,
>  		      const char *value);

What happens when called after `jw_array_begin()`? Same question is true
for all the other `jw_object_*` functions.

> +/*
> + * Start an object as the value of a field in the current object of the
> + * json_writer, given the field key.
> + */
>  void jw_object_inline_begin_object(struct json_writer *jw, const char *key);
> +
> +/*
> + * Start an array as the value of a field in the current object of the
> + * json_writer, given the field key.
> + */
>  void jw_object_inline_begin_array(struct json_writer *jw, const char *key);

Do these nest? E.g. can you call `inline_begin_object()` multiple times?

> +/*
> + * Append a string value to the current array of the json_writer.
> + */
>  void jw_array_string(struct json_writer *jw, const char *value);

Same question here as above: what happens when called after
`jw_object_begin()`?

> +/*
> + * Return if the json_writer is terminated. In other words, if the all the

s/if/whether/

Otherwise it reads as if the function wouldn't return in case it's not
terminated.

Patrick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 2/2] json-writer: describe the usage of jw_* functions
  2025-05-12  2:09 ` [GSoC PATCH v2 2/2] json-writer: describe the usage of " Lucas Seiki Oshiro
@ 2025-05-12  8:50   ` Patrick Steinhardt
  2025-05-12  9:41   ` Karthik Nayak
  1 sibling, 0 replies; 12+ messages in thread
From: Patrick Steinhardt @ 2025-05-12  8:50 UTC (permalink / raw)
  To: Lucas Seiki Oshiro; +Cc: git, gitster, karthik.188

On Sun, May 11, 2025 at 11:09:35PM -0300, Lucas Seiki Oshiro wrote:
> Provide an overview of the set of functions used for manipulating
> json_writers by describing what functions should be used for each
> JSON-related task.
> 
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Mentored-by Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>

Same comment here regarding the "Mentored-by" trailers.

> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
>  json-writer.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/json-writer.h b/json-writer.h
> index aa513e86cb..8b7470af67 100644
> --- a/json-writer.h
> +++ b/json-writer.h
> @@ -28,6 +28,34 @@
>   * object/array) -or- by building them inline in one pass.  This is a
>   * personal style and/or data shape choice.
>   *
> + * USAGE:
> + * ======
> + *
> + * - Initialize the json_writer with jw_init.
> + *
> + * - Open an object as the main data structure with jw_object_begin.
> + *   Append a key-value pair to it using the jw_object_<type> functions.
> + *   Conclude with jw_end.
> + *
> + * - Alternatively, open an array as the main data structure with
> + *   jw_array_begin. Append a value to it using the jw_array_<type>
> + *   functions. Conclude with jw_end.
> + *
> + * - Append a new, unterminated array or object to the current
> + *   object using the jw_object_inline_begin_{array, object} functions.
> + *   Similarly, append a new, unterminated array or object to
> + *   the current array using the jw_array_inline_begin_{array, object}
> + *   functions.
> + *
> + * - Append other json_writer as a value to the current array or object
> + *   using the jw_{array, object}_sub_jw functions.
> + *
> + * - Extend the current array with an null-terminated array of strings
> + *   by using jw_array_argv or with a fixed number of elements of a
> + *   array of string by using jw_array_argc_argv.
> + *
> + * - Relase the json_writer after using it by calling jw_release.
> + *
>   * See t/helper/test-json-writer.c for various usage examples.

Nice, this is something I wanted to ask for.

Patrick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 1/2] json-writer: add docstrings to jw_* functions
  2025-05-12  2:09 ` [GSoC PATCH v2 1/2] json-writer: add docstrings to " Lucas Seiki Oshiro
  2025-05-12  8:50   ` Patrick Steinhardt
@ 2025-05-12  9:36   ` Karthik Nayak
  1 sibling, 0 replies; 12+ messages in thread
From: Karthik Nayak @ 2025-05-12  9:36 UTC (permalink / raw)
  To: Lucas Seiki Oshiro, git; +Cc: gitster, ps

[-- Attachment #1: Type: text/plain, Size: 4307 bytes --]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

[snip]

> +/*
> + * Append a double field to the current object of the json_writer, given its key
> + * and its value. The precision parameter can be used for specifying the number
> + * of decimals after the point, using -1 for formatting with the maximum
> + * precision available.

Nit: I would perhaps switch s/using/use to make it present tense and
easier to read.

[snip]

> +/*
> + * Append a field to the current object of the json_writer, given its key and
> + * another json_writer that represents its content.
> + */
>  void jw_object_sub_jw(struct json_writer *jw, const char *key,
>  		      const struct json_writer *value);
>

`json-writer.c` also has a docstring for this function, perhaps we
can remove that and keep the header file as the source of truth?

> +/*
> + * Start an object as the value of a field in the current object of the
> + * json_writer, given the field key.
> + */
>  void jw_object_inline_begin_object(struct json_writer *jw, const char *key);
> +
> +/*
> + * Start an array as the value of a field in the current object of the
> + * json_writer, given the field key.
> + */
>  void jw_object_inline_begin_array(struct json_writer *jw, const char *key);
>
> +/*
> + * Append a string value to the current array of the json_writer.
> + */
>  void jw_array_string(struct json_writer *jw, const char *value);
> +
> +/*
> + * Append an int value to the current array of the json_writer.
> + */
>  void jw_array_intmax(struct json_writer *jw, intmax_t value);
> +
> +/*
> + * Append a double value to the current array of the json_writer. The precision
> + * parameter can be used for specifying the number of decimals after the point,
> + * using -1 for formatting with the maximum precision available.
> + */

Nit: wondering if it might be shorter/nicer to say

  The precision parameter defines the number of significant digits,
  where -1 can be used for maximum precision.

>  void jw_array_double(struct json_writer *jw, int precision, double value);
> +
> +/*
> + * Append a true value to the current array of the json_writer.
> + */
>  void jw_array_true(struct json_writer *jw);
> +
> +/*
> + * Append a false value to the current array of the json_writer.
> + */
>  void jw_array_false(struct json_writer *jw);
> +
> +/*
> + * Append a boolean value to the current array of the json_writer.
> + */
>  void jw_array_bool(struct json_writer *jw, int value);
> +
> +/*
> + * Append a null value to the current array of the json_writer.
> + */
>  void jw_array_null(struct json_writer *jw);
> +
> +/*
> + * Append a value to the current array of the json_writer, given the
> + * json_writer that represents its content.
> + */
>  void jw_array_sub_jw(struct json_writer *jw, const struct json_writer *value);
> +
> +/*
> + * Append the first argc values from the argv array of strings to the current
> + * array of the json_writer.
> + *
> + * This function does not provide safety for cases where the array has less than
> + * argc values.
> + */
>  void jw_array_argc_argv(struct json_writer *jw, int argc, const char **argv);
> +
> +/*
> + * Append a null-terminated array of strings to the current array of the
> + * json_writer.
> + */
>  void jw_array_argv(struct json_writer *jw, const char **argv);
>
> +/*
> + * Start an object as a value in the current array of the json_writer.
> + */
>  void jw_array_inline_begin_object(struct json_writer *jw);
> +
> +/*
> + * Start an array as a value in the current array.
> + */
>  void jw_array_inline_begin_array(struct json_writer *jw);
>
> +/*
> + * Return if the json_writer is terminated. In other words, if the all the
> + * objects and arrays are already closed.
> + */
>  int jw_is_terminated(const struct json_writer *jw);
> +
> +/*
> + * Terminates the current object or array of the json_writer. In other words,
> + * append a ] if the current array is not closed or } if the current object
> + * is not closed.
> + *
> + * Abort the execution if there's no object or array that can be terminated.
> + */
>  void jw_end(struct json_writer *jw);

Thanks, overall this looks good, I would drop the 'given its key' or
'given the value field' and similar  statements, as they don't provide
any additional context, but that is probably just nitpicking.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 2/2] json-writer: describe the usage of jw_* functions
  2025-05-12  2:09 ` [GSoC PATCH v2 2/2] json-writer: describe the usage of " Lucas Seiki Oshiro
  2025-05-12  8:50   ` Patrick Steinhardt
@ 2025-05-12  9:41   ` Karthik Nayak
  2025-05-13 22:22     ` Lucas Seiki Oshiro
  1 sibling, 1 reply; 12+ messages in thread
From: Karthik Nayak @ 2025-05-12  9:41 UTC (permalink / raw)
  To: Lucas Seiki Oshiro, git; +Cc: gitster, ps

[-- Attachment #1: Type: text/plain, Size: 2518 bytes --]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> Provide an overview of the set of functions used for manipulating
> json_writers by describing what functions should be used for each
> JSON-related task.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Mentored-by Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
>  json-writer.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/json-writer.h b/json-writer.h
> index aa513e86cb..8b7470af67 100644
> --- a/json-writer.h
> +++ b/json-writer.h
> @@ -28,6 +28,34 @@
>   * object/array) -or- by building them inline in one pass.  This is a
>   * personal style and/or data shape choice.
>   *
> + * USAGE:
> + * ======
> + *
> + * - Initialize the json_writer with jw_init.
> + *
> + * - Open an object as the main data structure with jw_object_begin.
> + *   Append a key-value pair to it using the jw_object_<type> functions.
> + *   Conclude with jw_end.
> + *
> + * - Alternatively, open an array as the main data structure with
> + *   jw_array_begin. Append a value to it using the jw_array_<type>
> + *   functions. Conclude with jw_end.
> + *
> + * - Append a new, unterminated array or object to the current
> + *   object using the jw_object_inline_begin_{array, object} functions.
> + *   Similarly, append a new, unterminated array or object to
> + *   the current array using the jw_array_inline_begin_{array, object}
> + *   functions.
> + *
> + * - Append other json_writer as a value to the current array or object
> + *   using the jw_{array, object}_sub_jw functions.
> + *
> + * - Extend the current array with an null-terminated array of strings
> + *   by using jw_array_argv or with a fixed number of elements of a
> + *   array of string by using jw_array_argc_argv.
> + *
> + * - Relase the json_writer after using it by calling jw_release.
> + *

s/Relase/Release

Overall this looks good, but I do have to wonder if it is needed given
that your previous patch already has documentation for each function. I
think more documentation is always better, but it shouldn't come at a
cost where we need to ensure that multiple sources of documentation need
to be updated to stay consistent with each other.

But I'll leave that decision to you.

Thanks for working on this!

>   * See t/helper/test-json-writer.c for various usage examples.
>   *
>   * LIMITATIONS:
> --
> 2.39.5 (Apple Git-154)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 1/2] json-writer: add docstrings to jw_* functions
  2025-05-12  8:50   ` Patrick Steinhardt
@ 2025-05-13 22:05     ` Lucas Seiki Oshiro
  2025-05-14  2:41       ` Patrick Steinhardt
  0 siblings, 1 reply; 12+ messages in thread
From: Lucas Seiki Oshiro @ 2025-05-13 22:05 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, gitster, karthik.188


> I don't think there's a need to add "Mentored-by" trailers to every
> commit just because we happen to be your mentors right now :) If we
> actually helped then sure, makes sense. But to the best of my knowledge
> we didn't, so I'd just leave them out for now.

Ok! But given that now both of you are helping me here I think it's
fair to give at least a Helped-by :-)

> void jw_object_begin(struct json_writer *jw, int pretty);
> 
> I think it would be interesting to learn _when_ to use this function. Is
> it mandatory to call it? Can it be nested? Why is there no corresponding
> `jw_object_end()`?
> 
>> void jw_array_begin(struct json_writer *jw, int pretty);
> 
> Same questions here.

A JSON can be a list or an object, composed by other lists or objects.
Those functions, then, define if the current json_writer will output a
list or an object.

Internal lists and objects are declared with
jw_{array, object}_inline_begin_{array, object}, depending if we want
to begin a list or an object and depending if we want to begin it
inside a list or an object.

In all those cases, there's no need to jw_object_end or jw_array_end.
jw_end covers both.

>> void jw_object_string(struct json_writer *jw, const char *key,
>>      const char *value);
> 
> What happens when called after `jw_array_begin()`? Same question is true
> for all the other `jw_object_*` functions.

It raises a bug: "json-writer: array: not in array

> 
>> void jw_object_inline_begin_object(struct json_writer *jw, const char *key);
>> 
>> void jw_object_inline_begin_array(struct json_writer *jw, const char *key);
> 
> Do these nest? E.g. can you call `inline_begin_object()` multiple times?

They are only tested up to the second nesting level. However, based
on the source code it looks like they should.

json_writer has a stack. The *inline_begin* functions basically append { or
[ to the buffer and to the stack.

Perhaps it would be a good idea to include a test for those cases?

> Patrick

Thanks!


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 2/2] json-writer: describe the usage of jw_* functions
  2025-05-12  9:41   ` Karthik Nayak
@ 2025-05-13 22:22     ` Lucas Seiki Oshiro
  2025-05-14  2:40       ` Patrick Steinhardt
  0 siblings, 1 reply; 12+ messages in thread
From: Lucas Seiki Oshiro @ 2025-05-13 22:22 UTC (permalink / raw)
  To: Karthik Nayak; +Cc: git, gitster, ps

> Overall this looks good, but I do have to wonder if it is needed given
> that your previous patch already has documentation for each function.

Agreed, it looks like too much information. This second patch was
created after the review of v1, and to be honest, I think that this
overview is more clear as it focus in the what we want (write a JSON)
instead of documenting each function.

So, if I need to choose one of them, I'll choose this.

> But I'll leave that decision to you.

Ok, I'll keep the second!

> Thanks for working on this!

Thank you!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 2/2] json-writer: describe the usage of jw_* functions
  2025-05-13 22:22     ` Lucas Seiki Oshiro
@ 2025-05-14  2:40       ` Patrick Steinhardt
  0 siblings, 0 replies; 12+ messages in thread
From: Patrick Steinhardt @ 2025-05-14  2:40 UTC (permalink / raw)
  To: Lucas Seiki Oshiro; +Cc: Karthik Nayak, git, gitster

On Tue, May 13, 2025 at 07:22:26PM -0300, Lucas Seiki Oshiro wrote:
> > Overall this looks good, but I do have to wonder if it is needed given
> > that your previous patch already has documentation for each function.
> 
> Agreed, it looks like too much information. This second patch was
> created after the review of v1, and to be honest, I think that this
> overview is more clear as it focus in the what we want (write a JSON)
> instead of documenting each function.
> 
> So, if I need to choose one of them, I'll choose this.
> 
> > But I'll leave that decision to you.
> 
> Ok, I'll keep the second!

I think that both are useful. It's a common pattern to provide a
high-level overview of how a certain subsystem is being used, but to
also document how respective functions work. The former is useful to get
a general understanding, whereas the latter is useful to get a better
understanding of specific edge cases and how for example error handling
works.

Patrick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GSoC PATCH v2 1/2] json-writer: add docstrings to jw_* functions
  2025-05-13 22:05     ` Lucas Seiki Oshiro
@ 2025-05-14  2:41       ` Patrick Steinhardt
  0 siblings, 0 replies; 12+ messages in thread
From: Patrick Steinhardt @ 2025-05-14  2:41 UTC (permalink / raw)
  To: Lucas Seiki Oshiro; +Cc: git, gitster, karthik.188

On Tue, May 13, 2025 at 07:05:21PM -0300, Lucas Seiki Oshiro wrote:
> > void jw_object_begin(struct json_writer *jw, int pretty);
> > 
> > I think it would be interesting to learn _when_ to use this function. Is
> > it mandatory to call it? Can it be nested? Why is there no corresponding
> > `jw_object_end()`?
> > 
> >> void jw_array_begin(struct json_writer *jw, int pretty);
> > 
> > Same questions here.
> 
> A JSON can be a list or an object, composed by other lists or objects.
> Those functions, then, define if the current json_writer will output a
> list or an object.
> 
> Internal lists and objects are declared with
> jw_{array, object}_inline_begin_{array, object}, depending if we want
> to begin a list or an object and depending if we want to begin it
> inside a list or an object.
> 
> In all those cases, there's no need to jw_object_end or jw_array_end.
> jw_end covers both.
> 
> >> void jw_object_string(struct json_writer *jw, const char *key,
> >>      const char *value);
> > 
> > What happens when called after `jw_array_begin()`? Same question is true
> > for all the other `jw_object_*` functions.
> 
> It raises a bug: "json-writer: array: not in array

Okay. Information like this is very valuable context to have in the
per-function docs.

> > 
> >> void jw_object_inline_begin_object(struct json_writer *jw, const char *key);
> >> 
> >> void jw_object_inline_begin_array(struct json_writer *jw, const char *key);
> > 
> > Do these nest? E.g. can you call `inline_begin_object()` multiple times?
> 
> They are only tested up to the second nesting level. However, based
> on the source code it looks like they should.
> 
> json_writer has a stack. The *inline_begin* functions basically append { or
> [ to the buffer and to the stack.
> 
> Perhaps it would be a good idea to include a test for those cases?

That would certainly be welcome :)

Patrick

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-05-14  2:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-12  2:09 [GSoC PATCH v2 0/2] json-writer: describe the jw_* functions Lucas Seiki Oshiro
2025-05-12  2:09 ` [GSoC PATCH v2 1/2] json-writer: add docstrings to " Lucas Seiki Oshiro
2025-05-12  8:50   ` Patrick Steinhardt
2025-05-13 22:05     ` Lucas Seiki Oshiro
2025-05-14  2:41       ` Patrick Steinhardt
2025-05-12  9:36   ` Karthik Nayak
2025-05-12  2:09 ` [GSoC PATCH v2 2/2] json-writer: describe the usage of " Lucas Seiki Oshiro
2025-05-12  8:50   ` Patrick Steinhardt
2025-05-12  9:41   ` Karthik Nayak
2025-05-13 22:22     ` Lucas Seiki Oshiro
2025-05-14  2:40       ` Patrick Steinhardt
2025-05-12  8:49 ` [GSoC PATCH v2 0/2] json-writer: describe the " Karthik Nayak

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 NNTP newsgroup(s).