Re: [GSoC PATCH v2 1/2] json-writer: add docstrings to jw_* functions

From: Patrick Steinhardt <ps@pks.im>
Date: May 12, 2025, 08:50 UTC
Message-ID: <aCG2XkW2lJ1qYHKW@pks.im>
In-Reply-To: <20250512020935.73140-2-lucasseikioshiro@gmail.com>

On Sun, May 11, 2025 at 11:09:34PM -0300, Lucas Seiki Oshiro wrote:

Show 5 quoted lines

> 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.

Show 24 quoted lines

> 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()`?

Show 6 quoted lines

> +/*
> + * 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.

Show 6 quoted lines

> +/*
> + * 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.

Show 11 quoted lines

> +/*
> + * 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