Re: [PATCH] repo: add --all to git-repo-info

[Patrick Steinhardt <ps@pks.im>]

On Mon, Sep 15, 2025 at 07:36:17PM -0300, Lucas Seiki Oshiro wrote:
> Add a new flag `--all` to git-repo-info for requesting all the available
> keys. By using this flag, the user can retrieve all the values instead
> of searching what are the desired keys for what they wants.

One thing I wonder is whether we actually need the "--all" flag in the
first place. Right now, when saying `git repo info` without any further
arguments, then the user will be met with complete silence. I don't
really think that this is useful as a default in any way, as it makes it
very difficult for the user to figure out what kind of information
exists in the first place.

So how about we don't introduce a separate flag, but instead detect the
case where the user passed no arguments at all and then print all values
by default? It changes the current behaviour, but on the other hand I
would argue that the current behaviour is not useful in the first place.
And the command is labelled as experimental anyway, so for now we still
can change it.

> diff --git a/builtin/repo.c b/builtin/repo.c
> index bbb0966f2d..906d8a3e12 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -77,6 +77,24 @@ static get_value_fn *get_value_fn_for_key(const char *key)
>  	return found ? found->get_value : NULL;
>  }
>  
> +static void print_field(enum output_format format, const char *key,
> +			struct strbuf *valbuf, struct strbuf *quotbuf)
> +{
> +	strbuf_reset(quotbuf);
> +
> +	switch (format) {
> +	case FORMAT_KEYVALUE:
> +		quote_c_style(valbuf->buf, quotbuf, NULL, 0);
> +		printf("%s=%s\n", key, quotbuf->buf);
> +		break;
> +	case FORMAT_NUL_TERMINATED:
> +		printf("%s\n%s%c", key, valbuf->buf, '\0');
> +		break;
> +	default:
> +		BUG("not a valid output format: %d", format);
> +	}
> +}
> +
>  static int print_fields(int argc, const char **argv,
>  			struct repository *repo,
>  			enum output_format format)

Changes like this which are preparatory refactorings can also be split
out into separate commits. That makes it easier to see and review such a
change standalone.

> @@ -119,6 +124,26 @@ static int print_fields(int argc, const char **argv,
>  	return ret;
>  }
>  
> +static void print_all_fields(struct repository *repo,
> +			     enum output_format format)
> +{
> +	struct strbuf valbuf = STRBUF_INIT;
> +	struct strbuf quotbuf = STRBUF_INIT;
> +
> +	for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
> +		struct field field = repo_info_fields[i];
> +		get_value_fn *get_value = field.get_value;
> +		const char *key = field.key;

Nit: I don't really feel like the `get_value` or `key` variables help
much. I'd just drop those.

> @@ -140,6 +165,7 @@ static int repo_info(int argc, const char **argv, const char *prefix,
>  		     struct repository *repo)
>  {
>  	enum output_format format = FORMAT_KEYVALUE;
> +	int all_keys = 0;
>  	struct option options[] = {
>  		OPT_CALLBACK_F(0, "format", &format, N_("format"),
>  			       N_("output format"),
> @@ -148,11 +174,17 @@ static int repo_info(int argc, const char **argv, const char *prefix,
>  			       N_("synonym for --format=nul"),
>  			       PARSE_OPT_NONEG | PARSE_OPT_NOARG,
>  			       parse_format_cb),
> +		OPT_BOOL(0, "all", &all_keys, N_("return all keys")),
>  		OPT_END()
>  	};
>  
>  	argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
>  
> +	if (all_keys) {
> +		print_all_fields(repo, format);
> +		return 0;
> +	}

Instead of checking for `all_keys` we could check for `if (!argc)` if
you want to follow my suggestion.

>  	return print_fields(argc, argv, repo, format);
>  }
>  
> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
> index 2beba67889..b1391a47b6 100755
> --- a/t/t1900-repo.sh
> +++ b/t/t1900-repo.sh
> @@ -110,4 +110,10 @@ test_expect_success 'git repo info uses the last requested format' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'git repo info --all returns all fields' '
> +	git repo info layout.bare layout.shallow object.format references.format >expect &&
> +	git repo info --all >actual &&
> +	test_cmp expect actual
> +'

This test will obviously need to be adapted every time we add a new
field, which I think is fine. But we should adapt it so that it's easily
extensible without by just adding another line. E.g. like this:

    test_expect_success 'git repo info --all returns all fields' '
            git repo info \
                layout.bare \
                layout.shallow \
                object.format \
                references.format \
                >expect &&
            git repo info --all >actual &&
            test_cmp expect actual
    '

This ensures that it's as simple as adding a new line when we add a
specific field without having to rewrite the whole line.

Patrick