Subject: Re: [GSoC RFC PATCH v2 2/7] repo-info: add the --format flag

From: Junio C Hamano <[email-masked]>
Date: Fri, 20 Jun 2025 14:31:05 -0700

Lucas Seiki Oshiro <[email-masked]> writes:

> Add the --format flag to the repo-info command, allowing the user to
> choose between output formats. Use 'json' by default.
>
> Mentored-by: Karthik Nayak <[email-masked]>
> Mentored-by Patrick Steinhardt <[email-masked]>
> Signed-off-by: Lucas Seiki Oshiro <[email-masked]>
> ---
>  builtin/repo-info.c  | 54 +++++++++++++++++++++++++++++++++++++++++++-
>  t/meson.build        |  1 +
>  t/t1900-repo-info.sh | 22 ++++++++++++++++++
>  3 files changed, 76 insertions(+), 1 deletion(-)
>  create mode 100755 t/t1900-repo-info.sh
>
> diff --git a/builtin/repo-info.c b/builtin/repo-info.c
> index a5c43e253f..cbe1475e30 100644
> --- a/builtin/repo-info.c
> +++ b/builtin/repo-info.c
> @@ -1,21 +1,73 @@
>  #include "builtin.h"
> +#include "json-writer.h"
>  #include "parse-options.h"
>  
> +enum output_format {
> +	FORMAT_JSON
> +};

If you give a trailing comma after FORMAT_JSON here, then a future
step that adds other values to the enum would not have to touch this
line, e.g. instead of

     enum output_format {
    -    FORMAT_JSON
    +    FORMAT_JSON,
    +    FORMAT_FOO
     };

you can do this

     enum output_format {
         FORMAT_JSON,
    +    FORMAT_FOO,
     };

when you add a new value "FOO" to enum.

cf. Documentation/CodingGuidelines

   . since early 2012 with e1327023ea, we have been using an enum
     definition whose last element is followed by a comma.  This, like
     an array initializer that ends with a trailing comma, can be used
     to reduce the patch noise when adding a new identifier at the end.

> +struct repo_info {
> +	struct repository *repo;
> +	enum output_format format;
> +};
> +
> +static void repo_info_init(struct repo_info *repo_info,
> +			   struct repository *repo,
> +			   char *format)
> +{
> +	repo_info->repo = repo;
> +
> +	if (format == NULL || !strcmp(format, "json"))
> +		repo_info->format = FORMAT_JSON;
> +	else
> +		die("invalid format %s", format);
> +}
> +
> +static void repo_info_print_json(struct repo_info *repo_info UNUSED)
> +{
> +	struct json_writer jw;
> +
> +	jw_init(&jw);
> +
> +	jw_object_begin(&jw, 1);
> +	jw_end(&jw);
> +
> +	puts(jw.json.buf);
> +	jw_release(&jw);
> +}
> +
> +static void repo_info_print(struct repo_info *repo_info)
> +{
> +	enum output_format format = repo_info->format;
> +
> +	switch (format) {
> +	case FORMAT_JSON:
> +		repo_info_print_json(repo_info);
> +		break;
> +	}
> +}
> +
>  int cmd_repo_info(int argc,
>  		  const char **argv,
>  		  const char *prefix,
> -		  struct repository *repo UNUSED)
> +		  struct repository *repo)
>  {
>  	const char *const repo_info_usage[] = {
>  		"git repo-info",
>  		NULL
>  	};
> +	struct repo_info repo_info;
> +	char *format = NULL;
>  	struct option options[] = {
> +		OPT_STRING(0, "format", &format, N_("format"),
> +			   N_("output format")),
>  		OPT_END()
>  	};
>  
>  	argc = parse_options(argc, argv, prefix, options, repo_info_usage,
>  			     PARSE_OPT_KEEP_UNKNOWN_OPT);
> +	repo_info_init(&repo_info, repo, format);
> +	repo_info_print(&repo_info);
>  
>  	return 0;
>  }
> diff --git a/t/meson.build b/t/meson.build
> index 50e89e764a..d9ecaba3b7 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -246,6 +246,7 @@ integration_tests = [
>    't1700-split-index.sh',
>    't1701-racy-split-index.sh',
>    't1800-hook.sh',
> +  't1900-repo-info.sh',
>    't2000-conflict-when-checking-files-out.sh',
>    't2002-checkout-cache-u.sh',
>    't2003-checkout-cache-mkdir.sh',
> diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
> new file mode 100755
> index 0000000000..f634e1a285
> --- /dev/null
> +++ b/t/t1900-repo-info.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +test_description='test git repo-info'
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +
> +parse_json () {
> +	tr '\n' ' ' | "$PERL_PATH" "$TEST_DIRECTORY/t0019/parse_json.perl"
> +}
> +
> +test_lazy_prereq PERLJSON '
> +	perl -MJSON -e "exit 0"
> +'
> +
> +test_expect_success PERLJSON 'json: returns empty output with allow-empty' '

Allow-empty?

Instead of "if no command line args, list everything, or show only
the ones specified on the command line", wouldn't it be far easier
to explain if you made it "we show only the ones specified on the
command line, but giving '--all' on the command line behaves as if
you listed all the ones known to us on the command line"?

> +	git repo-info --format=json >output &&
> +	test_line_count = 2 output
> +'
> +
> +test_done