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

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 21, 2025, 05:44 UTC
Message-ID: <aPcduvnjD0yphja2@pks.im>
In-Reply-To: <20251020181943.6314-3-lucasseikioshiro@gmail.com>

On Mon, Oct 20, 2025 at 01:19:47PM -0300, Lucas Seiki Oshiro wrote:

Show 30 quoted lines

> diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
> index 209afd1b61..1a9d0c50a9 100644
> --- a/Documentation/git-repo.adoc
> +++ b/Documentation/git-repo.adoc
> @@ -8,7 +8,7 @@ git-repo - Retrieve information about the repository
>  SYNOPSIS
>  --------
>  [synopsis]
> -git repo info [--format=(keyvalue|nul)] [-z] [<key>...]
> +git repo info [--format=(keyvalue|nul)] [-z] [--all | <key>...]
>  
>  DESCRIPTION
>  -----------
> @@ -18,13 +18,14 @@ THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
>  
>  COMMANDS
>  --------
> -`info [--format=(keyvalue|nul)] [-z] [<key>...]`::
> +`info [--format=(keyvalue|nul)] [-z] [--all | <key>...]`::
>  	Retrieve metadata-related information about the current repository. Only
>  	the requested data will be returned based on their keys (see "INFO KEYS"
>  	section below).
>  +
>  The values are returned in the same order in which their respective keys were
> -requested.
> +requested. The `--all` flag requests the values for all the available keys.
> +Keys requested after `--all` will be duplicated.
>  +
>  The output format can be chosen through the flag `--format`. Two formats are
>  supported:

The synopsis now disagrees with the new behaviour, as it looks as if you can pick either "--all" or a set of keys. But we now support both at the same time.

I know Junio mentioned this as one of the ways this may operate, and said that accepting both is the "most logical". I personally don't quite agree, and think that having it be either or is a bit saner. After all, what is the use case for listing specific keys twice? I cannot really see why one would ever want that. So I think we should accept either `--all` or keys, and die if they are used in combination.

If Junio continues to prefer the version you have here then so be it. But in that case you'll have to fix the synopsis to reflect that.

Show 27 quoted lines

> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
> index 2beba67889..28635d0f92 100755
> --- a/t/t1900-repo.sh
> +++ b/t/t1900-repo.sh
> @@ -110,4 +119,24 @@ test_expect_success 'git repo info uses the last requested format' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'git repo info --all returns all key-value pairs' '
> +	git repo info $REPO_INFO_KEYS >expect &&
> +	git repo info --all >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git repo info --all <key> duplicates <key>' '
> +	git repo info $REPO_INFO_KEYS object.format >expect &&
> +	git repo info --all object.format >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git repo info --all <invalid key> warns about invalid key' '
> +	git repo info $REPO_INFO_KEYS >expect &&
> +	echo "error: key ${SQ}no.key${SQ} not found" >expect_err &&
> +	test_must_fail git repo info --all no.key >actual 2>actual_err &&
> +	test_cmp expect actual &&
> +	test_cmp expect_err actual_err
> +'

Yeah, these don't quite convince me that using them in combination is sensible :) In fact, there is one more argument against this here: in the current form, the order in which we print the key-value pairs depends on the order in which they are specified on the command line. So far so good.

But with `--all` that's not the case anymore, as we unconditionally print all pairs before the individual keys. So `git repo info --all <key>` produces the same output as `git repo info <key> --all`. Now we're in a mode where only _parts_ of the output depends on the order of our command line arguments, which is inconsistent.

So... yeah, I think accepting either or is the more sensible approach. There is no use case for printing keys twice, and if we do then we have some weird inconsistencies in the ordering.

Thanks!

Patrick