Re: [PATCH] status: add --json output format to git status

[Junio C Hamano <gitster@pobox.com>]

"Tach via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: tacherasasi <tacherasasi@gmail.com>

> Add a new --json flag to 'git status' that outputs repository state
> in a structured JSON format. This enables reliable machine parsing
> of status information for tools and automation.

The --porcelain format is meant to serve this "parseable for tools
and automation to read" use case.  Was there anything your parser
couldn't fathom out of the format?

> This provides a robust alternative to parsing traditional output
> formats, making it easier to build reliable tools and automation
> around Git status information.

The writer is obviously biased ;-) but I find this a bit hand-wavy
and unconvincing.

> Signed-off-by: Tachera Sasi <tachera@ekilie.com>
> Signed-off-by: tacherasasi <tacherasasi@gmail.com>

It is not like two separate people worked on this patch, or somebody
at ekilie worked on the patch but asked a different person with a
very similar name to send it on their behalf, right?

Perhaps update the user.{name,email} on the in-body From: to match
the one used on the first Sign-off, and drop the second Sign-off?

> diff --git a/builtin/commit.c b/builtin/commit.c
> index fba0dded64a..f1db4fdfd9a 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1540,6 +1540,9 @@ struct repository *repo UNUSED)
>  		OPT_SET_INT(0, "long", &status_format,
>  			    N_("show status in long format (default)"),
>  			    STATUS_FORMAT_LONG),
> +		OPT_SET_INT(0, "json", &status_format,
> +		    N_("show status in JSON format"),
> +		    STATUS_FORMAT_JSON),

There are now several ways to set to status_format now.  It could be
a good idea to make a preliminary UI clean-up to introduce a new
option "--format={short,long,porcelain,porcelain-v1,porcelain-v2}"
as a separate step, declare that any new format will use this new
option without adding a separate option on its own, before adding
any new features.  And then on top that, a new format can be
introduced without cluttering the set of options.

Not that I am enthused to see json here, given that we have
introduced porcelain format for the same purpose eons ago.

> @@ -1603,7 +1606,8 @@ struct repository *repo UNUSED)
>  		       prefix, argv);
>  
>  	if (status_format != STATUS_FORMAT_PORCELAIN &&
> -	    status_format != STATUS_FORMAT_PORCELAIN_V2)
> +	    status_format != STATUS_FORMAT_PORCELAIN_V2 &&
> +	    status_format != STATUS_FORMAT_JSON)
>  		progress_flag = REFRESH_PROGRESS;
>  	repo_read_index(the_repository);
>  	refresh_index(the_repository->index,

Also, we might want to tweak the bit-assignment for the
status_format constants to make this part easier to maintain (e.g.,
ones below 100 are for human consumption and gives progress bar and
ones above 100 are for machine consumption and suppresses progress,
or something like that), again as a separate preliminary clean-up
step before adding any new features.

> @@ -1735,6 +1739,9 @@ int cmd_commit(int argc,
>  		OPT_SET_INT(0, "long", &status_format,
>  			    N_("show status in long format (default)"),
>  			    STATUS_FORMAT_LONG),
> +		OPT_SET_INT(0, "json", &status_format,
> +		    N_("show status in JSON format"),
> +		    STATUS_FORMAT_JSON),

And if we are to clean-up with --format=<name> and build on top,
this part would become somewhat different.  The clean-up would
probably be somewhat like what opt_parse_porcelain() is doing and
"json" or "yaml" or any new format name would join if/else if/
cascade there.

> diff --git a/wt-status.c b/wt-status.c
> index 454601afa15..7192fb4d057 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -2564,6 +2564,101 @@ static void wt_porcelain_v2_print(struct wt_status *s)
>  	}
>  }
>  
> +
> +static void wt_json_print_string_array(struct wt_status *s, const char *name, struct string_list *list)
> +{

We seem to have a json write-out library-ish helpers already; is it
too tied to its current use (i.e. trace output) and too inflexible
to be used here, or does the status output have so specialized and
unusual need as a normal JSON producing application that we need a
special purpose code here with bunch of fprintf() and manual
formatting?

Thanks.