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

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

[Lucas Seiki Oshiro]

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.

Helped-by: Karthik Nayak <karthik.188@gmail.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
Hi!

This patch is an epilogue of my GSoC, as it was requested to have some
way to retrieve all the values without needing to pass all the keys.

This is built on top of the current master, a483264b01 (The ninth
batch, 2025-09-15).

 Documentation/git-repo.adoc |  6 ++--
 builtin/repo.c              | 62 ++++++++++++++++++++++++++++---------
 t/t1900-repo.sh             |  6 ++++
 3 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 209afd1b61..2caf093a9a 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,13 @@ 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 all keys.
 +
 The output format can be chosen through the flag `--format`. Two formats are
 supported:
diff --git a/builtin/repo.c b/builtin/repo.c
index bbb0966f2d..906d8a3e12 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -9,7 +9,7 @@
 #include "shallow.h"
 
 static const char *const repo_usage[] = {
-	"git repo info [--format=(keyvalue|nul)] [-z] [<key>...]",
+	"git repo info [--format=(keyvalue|nul)] [-z] [--all] [<key>...]",
 	NULL
 };
 
@@ -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)
@@ -97,21 +115,8 @@ static int print_fields(int argc, const char **argv,
 		}
 
 		strbuf_reset(&valbuf);
-		strbuf_reset(&quotbuf);
-
 		get_value(repo, &valbuf);
-
-		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);
-		}
+		print_field(format, key, &valbuf, &quotbuf);
 	}
 
 	strbuf_release(&valbuf);
@@ -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;
+
+		strbuf_reset(&valbuf);
+		get_value(repo, &valbuf);
+		print_field(format, key, &valbuf, &quotbuf);
+	}
+
+	strbuf_release(&valbuf);
+	strbuf_release(&quotbuf);
+}
+
 static int parse_format_cb(const struct option *opt,
 			   const char *arg, int unset UNUSED)
 {
@@ -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;
+	}
+
 	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
+'
+
 test_done
-- 
2.39.5 (Apple Git-154)

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

[Junio C Hamano]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

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

I initially read these three lines as "we let you grab all the keys
(without value), so that the caller do it once and then iterate over
them, asking for the values individually".

I think "for requesting all the available keys" can be tweaked to
avoid such a misunderstanding?

    for requesting values for all the available keys

or something, perhaps?

> -git repo info [--format=(keyvalue|nul)] [-z] [<key>...]
> +git repo info [--format=(keyvalue|nul)] [-z] [--all] [<key>...]

Wouldn't it be more like

	..... [--all | <key>...]

or does giving both --all and an indiviual key do something
interesting (like, just make sure these individual keys are valid,
but otherwise do the same as a simple --all)?

> +`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 all keys.

"requests values for all the keys."

>  	argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
>  
> +	if (all_keys) {
> +		print_all_fields(repo, format);
> +		return 0;
> +	}
> +
>  	return print_fields(argc, argv, repo, format);

OK, so "git repo info --all no-such-key" will silently ignore
no-such-key.  I do not have much problem as long as it is
documented, but there are a few equally plausible alternative
designs.

 * "git repo info --all anything" ignores "anything" no matter what
   they are, as "--all" makes all keys on the command line ignored.

 * The same as above, but it warns about the extra command line
   arguments that are ignored.

 * "git repo info --all object.format" is rejected merely because
   "--all" is defined to be incompatible with giving any individual
   key.

 * "git repo info --all object.format" works as if the command is
   given all the defined keys and then object.format, i.e.
   object.format is reported twice.  If you ask "git repo info
   --all no.such.key", it would fail while asking for no.such.key
   because there is no such key.

I think the first one is what you have implemented.

I see no practical reason why anybody want to pass a concrete key
when asking "--all", but the first one feels the least intuitive one
among these four.  I think the last one is the most logical that
lets users discover why it behaves that way the most easily, even
though it is debatable that succeeding and doing exactly what was
requested in that way is better than rejecting (or perhaps ignoring
with warning) these requests with extra command line arguments.

> 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

We would want tests that asks "--all object.format" and "--all no.key",
after deciding what should happen.

Thanks.

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

[Patrick Steinhardt]

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

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

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

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

If we were talking about interactive tool, I might agree, but for a
tool for scriptors,

    tool "$@"

should not silently turn into

    tool --all

when $# == 0.  That is asking for surprises.

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

[Patrick Steinhardt]

On Tue, Sep 16, 2025 at 09:19:02AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 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.
> 
> If we were talking about interactive tool, I might agree, but for a
> tool for scriptors,
> 
>     tool "$@"
> 
> should not silently turn into
> 
>     tool --all
> 
> when $# == 0.  That is asking for surprises.

That's fair. Let's stick with a flag then.

Patrick

[PATCH v3 0/2] repo: add --all to git-repo-info

[Lucas Seiki Oshiro]

Hi!

This is the third version of the patchset that adds a new flag --all
to git-repo-info for requesting the values of all the available flags.

Now, in this version, git-repo-info will abort if it is called with --all
and some key.

Here's the range-diff against v2:

1:  5f72f07589 = 1:  0db9aad2bc repo: factor out field printing to dedicated function
2:  b8158bb7b8 ! 2:  b6ecdc2c2f repo: add --all to git-repo-info
    @@ Documentation/git-repo.adoc: THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHAN
      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:
    @@ builtin/repo.c: static int print_fields(int argc, const char **argv,
     +
     +	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;
     +
     +		strbuf_reset(&valbuf);
    -+		get_value(repo, &valbuf);
    -+		print_field(format, key, &valbuf, &quotbuf);
    ++		field.get_value(repo, &valbuf);
    ++		print_field(format, field.key, &valbuf, &quotbuf);
     +	}
     +
     +	strbuf_release(&valbuf);
    @@ builtin/repo.c: static int repo_info(int argc, const char **argv, const char *pr
      
      	argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
      
    -+	if (all_keys)
    ++	if (all_keys) {
    ++		if (argc)
    ++			die(_("--all and <key> cannot be used together"));
    ++
     +		print_all_fields(repo, format);
    ++		return 0;
    ++	}
     +
      	return print_fields(argc, argv, repo, format);
      }
    @@ t/t1900-repo.sh: test_expect_success 'git repo info uses the last requested form
     +	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_expect_success 'git repo info --all <key> aborts' '
    ++	echo "fatal: --all and <key> cannot be used together" >expect &&
    ++	test_must_fail git repo info --all object.format 2>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
    -+'
     +
      test_done

Lucas Seiki Oshiro (2):
  repo: factor out field printing to dedicated function
  repo: add --all to git-repo-info

 Documentation/git-repo.adoc |  6 ++--
 builtin/repo.c              | 63 ++++++++++++++++++++++++++++---------
 t/t1900-repo.sh             | 21 +++++++++++++
 3 files changed, 72 insertions(+), 18 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH v3 1/2] repo: factor out field printing to dedicated function

[Lucas Seiki Oshiro]

Move the field printing in git-repo-info to a new function called
`print_field`, allowing it to be called by functions other than
`print_fields`.

Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 builtin/repo.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/builtin/repo.c b/builtin/repo.c
index bbb0966f2d..3b071e9a50 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)
@@ -97,21 +115,8 @@ static int print_fields(int argc, const char **argv,
 		}
 
 		strbuf_reset(&valbuf);
-		strbuf_reset(&quotbuf);
-
 		get_value(repo, &valbuf);
-
-		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);
-		}
+		print_field(format, key, &valbuf, &quotbuf);
 	}
 
 	strbuf_release(&valbuf);
-- 
2.50.1 (Apple Git-155)

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

[Lucas Seiki Oshiro]

Add a new flag `--all` to git-repo-info for requesting values for 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.

Helped-by: Karthik Nayak <karthik.188@gmail.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 Documentation/git-repo.adoc |  6 +++---
 builtin/repo.c              | 30 +++++++++++++++++++++++++++++-
 t/t1900-repo.sh             | 21 +++++++++++++++++++++
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 209afd1b61..e61af9ce3b 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,13 @@ 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.
 +
 The output format can be chosen through the flag `--format`. Two formats are
 supported:
diff --git a/builtin/repo.c b/builtin/repo.c
index 3b071e9a50..67d647bb3c 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -9,7 +9,7 @@
 #include "shallow.h"
 
 static const char *const repo_usage[] = {
-	"git repo info [--format=(keyvalue|nul)] [-z] [<key>...]",
+	"git repo info [--format=(keyvalue|nul)] [-z] [--all | <key>...]",
 	NULL
 };
 
@@ -124,6 +124,24 @@ 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];
+
+		strbuf_reset(&valbuf);
+		field.get_value(repo, &valbuf);
+		print_field(format, field.key, &valbuf, &quotbuf);
+	}
+
+	strbuf_release(&valbuf);
+	strbuf_release(&quotbuf);
+}
+
 static int parse_format_cb(const struct option *opt,
 			   const char *arg, int unset UNUSED)
 {
@@ -145,6 +163,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"),
@@ -153,11 +172,20 @@ 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) {
+		if (argc)
+			die(_("--all and <key> cannot be used together"));
+
+		print_all_fields(repo, format);
+		return 0;
+	}
+
 	return print_fields(argc, argv, repo, format);
 }
 
diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
index 2beba67889..51d55f11a5 100755
--- a/t/t1900-repo.sh
+++ b/t/t1900-repo.sh
@@ -4,6 +4,15 @@ test_description='test git repo-info'
 
 . ./test-lib.sh
 
+# git-repo-info keys. It must contain the same keys listed in the const
+# repo_info_fields, in lexicographical order.
+REPO_INFO_KEYS='
+	layout.bare
+	layout.shallow
+	object.format
+	references.format
+'
+
 # Test whether a key-value pair is correctly returned
 #
 # Usage: test_repo_info <label> <init command> <repo_name> <key> <expected value>
@@ -110,4 +119,16 @@ 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> aborts' '
+	echo "fatal: --all and <key> cannot be used together" >expect &&
+	test_must_fail git repo info --all object.format 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3 1/2] repo: factor out field printing to dedicated function

[Eric Sunshine]

On Sun, Oct 26, 2025 at 6:54 PM Lucas Seiki Oshiro
<lucasseikioshiro@gmail.com> wrote:
> Move the field printing in git-repo-info to a new function called
> `print_field`, allowing it to be called by functions other than
> `print_fields`.
>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
> diff --git a/builtin/repo.c b/builtin/repo.c
> @@ -77,6 +77,24 @@ static get_value_fn *get_value_fn_for_key(const char *key)
> +static void print_field(enum output_format format, const char *key,
> +                       struct strbuf *valbuf, struct strbuf *quotbuf)
> +{

Let's not pass in 'valbuf' as a 'struct strbuf *' since doing so gives
the false impression that this function will be modifying the strbuf
(which it does not do). Instead, pass in the narrower `const char
*value` which indicates clearly that this function will not be
modifying the value.

> +       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);
> +       }
> +}

Moreover, I'd also say that since this is not on a critical path, you
should avoid the premature optimization of passing in `strfbuf
*quotebuf` and instead make `quotebuf` local to this function.

    static void print_field(enum output_format format,
        const char *key, const char *value)
    {
        struct strbuf quotbuf = STRBUF_INIT;
        ...stuff...
        strbuf_release(&quotbuf);
    }

>  static int print_fields(int argc, const char **argv,
>                         struct repository *repo,
>                         enum output_format format)
> @@ -97,21 +115,8 @@ static int print_fields(int argc, const char **argv,
>                 }
>
>                 strbuf_reset(&valbuf);
> -               strbuf_reset(&quotbuf);
> -
>                 get_value(repo, &valbuf);
> -
> -               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);
> -               }
> +               print_field(format, key, &valbuf, &quotbuf);
>         }
>
>         strbuf_release(&valbuf);

Re: [PATCH v3 1/2] repo: factor out field printing to dedicated function

[Eric Sunshine]

On Sun, Oct 26, 2025 at 7:53 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sun, Oct 26, 2025 at 6:54 PM Lucas Seiki Oshiro
> <lucasseikioshiro@gmail.com> wrote:
> > +       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);
> > +       }
> > +}
>
> Moreover, I'd also say that since this is not on a critical path, you
> should avoid the premature optimization of passing in `strfbuf
> *quotebuf` and instead make `quotebuf` local to this function.
>
>     static void print_field(enum output_format format,
>         const char *key, const char *value)
>     {
>         struct strbuf quotbuf = STRBUF_INIT;
>         ...stuff...
>         strbuf_release(&quotbuf);
>     }

Or, even better, scope the strbuf just to the `case` branch which needs it:

    case FORMAT_KEYVALUE: {
        struct strbuf buf = STRBUF_INIT;
        quote_c_style(value, buf, NULL, 0);
        printf("%s=%s\n", key, buf->buf);
        strbuf_release(&buf);
        break;
     }

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

[Eric Sunshine]

On Sun, Oct 26, 2025 at 6:54 PM Lucas Seiki Oshiro
<lucasseikioshiro@gmail.com> wrote:
> Add a new flag `--all` to git-repo-info for requesting values for 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.
>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
> diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
> @@ -18,13 +18,13 @@ THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
> +`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.

I'm getting mixed signals from this patch. The documentation says that
it requests all *values*, but... (continued far below)

> diff --git a/builtin/repo.c b/builtin/repo.c
> @@ -124,6 +124,24 @@ static int print_fields(int argc, const char **argv,
> +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];

Why do we need to make a copy of the field record? Can't we just use a
const pointer?

    struct field *field = &repo_info_fields[i];

or:

   struct field *field = repo_info_fields + i;

> +               strbuf_reset(&valbuf);
> +               field.get_value(repo, &valbuf);
> +               print_field(format, field.key, &valbuf, &quotbuf);
> +       }
> +
> +       strbuf_release(&valbuf);
> +       strbuf_release(&quotbuf);
> +}
> @@ -153,11 +172,20 @@ static int repo_info(int argc, const char **argv, const char *prefix,
> +               OPT_BOOL(0, "all", &all_keys, N_("return all keys")),

(continued from above) ...this gives the impression that it's only
returning *keys*, but not necessarily the values associated with those
keys. Also, "return" is a bit unusual in this context; perhaps say
"request all keys/values" or "print all keys/values" or something.

>                 OPT_END()
>         };
>
>         argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
>
> +       if (all_keys) {
> +               if (argc)
> +                       die(_("--all and <key> cannot be used together"));
> +
> +               print_all_fields(repo, format);
> +               return 0;
> +       }
> +
>         return print_fields(argc, argv, repo, format);
>  }
>
> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
> @@ -4,6 +4,15 @@ test_description='test git repo-info'
> +# git-repo-info keys. It must contain the same keys listed in the const
> +# repo_info_fields, in lexicographical order.
> +REPO_INFO_KEYS='
> +       layout.bare
> +       layout.shallow
> +       object.format
> +       references.format
> +'

I'm not a fan of this since it is so brittle. However, I can't think
of a better alternative at the moment, and we can always revisit it
later if it becomes a maintenance burden.

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

[Eric Sunshine]

On Sun, Oct 26, 2025 at 8:22 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sun, Oct 26, 2025 at 6:54 PM Lucas Seiki Oshiro
> <lucasseikioshiro@gmail.com> wrote:
> > +       for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
> > +               struct field field = repo_info_fields[i];
>
> Why do we need to make a copy of the field record? Can't we just use a
> const pointer?
>
>     struct field *field = &repo_info_fields[i];
>
> or:
>
>    struct field *field = repo_info_fields + i;

Of course, I mean to have a `const` in there:

    const struct field *field = &repo_info_fields[i];

Re: [PATCH v3 1/2] repo: factor out field printing to dedicated function

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Or, even better, scope the strbuf just to the `case` branch which needs it:
>
>     case FORMAT_KEYVALUE: {
>         struct strbuf buf = STRBUF_INIT;
>         quote_c_style(value, buf, NULL, 0);
>         printf("%s=%s\n", key, buf->buf);
>         strbuf_release(&buf);
>         break;
>      }

Yuck.  

For something simple like this, quote_c_style() can take "FILE *"
instead of struct "strbuf *" so that you do not have to allocate;
especially without any need for i18n, perhaps

    printf("%s=", key);
    quote_c_style(value, NULL, stdout, 0);

is sufficient?

Re: [PATCH v3 1/2] repo: factor out field printing to dedicated function

[Eric Sunshine]

On Mon, Oct 27, 2025 at 10:56 AM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > Or, even better, scope the strbuf just to the `case` branch which needs it:
> >
> >     case FORMAT_KEYVALUE: {
> >         struct strbuf buf = STRBUF_INIT;
> >         quote_c_style(value, buf, NULL, 0);
> >         printf("%s=%s\n", key, buf->buf);
> >         strbuf_release(&buf);
> >         break;
> >      }
>
> For something simple like this, quote_c_style() can take "FILE *"
> instead of struct "strbuf *" so that you do not have to allocate;
> especially without any need for i18n, perhaps
>
>     printf("%s=", key);
>     quote_c_style(value, NULL, stdout, 0);

Thanks, that's even better. Nice and clean and easy to read.

(I didn't have the Git source code at hand, so didn't know that
`quote_c_style` could take `FILE *`.)

[PATCH v4 0/2] repo: add --all to git-repo-info

[Lucas Seiki Oshiro]

Hi!

Sorry for only sending this after some weeks. I've been busy finishing
my master's and I didn't have enough time to send another version. But
here it is.

This fourth version of this patch addresses the issues pointed by Eric
in the v3:

- I dropped the `strbuf quotebuf`, since it can be replaced by
  outputting `quote_c_style` directly to `stdout`;

- `print_field` now uses the string `value` instead of the
  `strbuf valbuf`;

- The variable `field` in `print_fields` was replaced by a pointer,
  since it didn't require to be copied;

- replace the help string by the suggested.

Lucas Seiki Oshiro (2):
  repo: factor out field printing to dedicated function
  repo: add --all to git-repo-info

 Documentation/git-repo.adoc |  6 ++--
 builtin/repo.c              | 62 +++++++++++++++++++++++++++----------
 t/t1900-repo.sh             | 21 +++++++++++++
 3 files changed, 69 insertions(+), 20 deletions(-)

Range-diff against v3:
1:  0db9aad2bc ! 1:  fce09770b8 repo: factor out field printing to dedicated function
    @@ builtin/repo.c: static get_value_fn *get_value_fn_for_key(const char *key)
      }
      
     +static void print_field(enum output_format format, const char *key,
    -+			struct strbuf *valbuf, struct strbuf *quotbuf)
    ++			const char *value)
     +{
    -+	strbuf_reset(quotbuf);
    -+
     +	switch (format) {
     +	case FORMAT_KEYVALUE:
    -+		quote_c_style(valbuf->buf, quotbuf, NULL, 0);
    -+		printf("%s=%s\n", key, quotbuf->buf);
    ++		printf("%s=", key);
    ++		quote_c_style(value, NULL, stdout, 0);
    ++		putchar('\n');
     +		break;
     +	case FORMAT_NUL_TERMINATED:
    -+		printf("%s\n%s%c", key, valbuf->buf, '\0');
    ++		printf("%s\n%s%c", key, value, '\0');
     +		break;
     +	default:
     +		BUG("not a valid output format: %d", format);
    @@ builtin/repo.c: static get_value_fn *get_value_fn_for_key(const char *key)
      static int print_fields(int argc, const char **argv,
      			struct repository *repo,
      			enum output_format format)
    + {
    + 	int ret = 0;
    + 	struct strbuf valbuf = STRBUF_INIT;
    +-	struct strbuf quotbuf = STRBUF_INIT;
    + 
    + 	for (int i = 0; i < argc; i++) {
    + 		get_value_fn *get_value;
     @@ builtin/repo.c: static int print_fields(int argc, const char **argv,
      		}
      
    @@ builtin/repo.c: static int print_fields(int argc, const char **argv,
     -		default:
     -			BUG("not a valid output format: %d", format);
     -		}
    -+		print_field(format, key, &valbuf, &quotbuf);
    ++		print_field(format, key, valbuf.buf);
      	}
      
      	strbuf_release(&valbuf);
    +-	strbuf_release(&quotbuf);
    + 	return ret;
    + }
    + 
2:  b6ecdc2c2f ! 2:  ccdad86123 repo: add --all to git-repo-info
    @@ Documentation/git-repo.adoc: git-repo - Retrieve information about the repositor
      [synopsis]
     -git repo info [--format=(keyvalue|nul)] [-z] [<key>...]
     +git repo info [--format=(keyvalue|nul)] [-z] [--all | <key>...]
    + git repo structure [--format=(table|keyvalue|nul)]
      
      DESCRIPTION
    - -----------
     @@ Documentation/git-repo.adoc: THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
      
      COMMANDS
    @@ Documentation/git-repo.adoc: THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHAN
     
      ## builtin/repo.c ##
     @@
    - #include "shallow.h"
    + #include "utf8.h"
      
      static const char *const repo_usage[] = {
     -	"git repo info [--format=(keyvalue|nul)] [-z] [<key>...]",
     +	"git repo info [--format=(keyvalue|nul)] [-z] [--all | <key>...]",
    + 	"git repo structure [--format=(table|keyvalue|nul)]",
      	NULL
      };
    - 
     @@ builtin/repo.c: static int print_fields(int argc, const char **argv,
      	return ret;
      }
    @@ builtin/repo.c: static int print_fields(int argc, const char **argv,
     +			     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];
    ++		const struct field *field = &repo_info_fields[i];
     +
     +		strbuf_reset(&valbuf);
    -+		field.get_value(repo, &valbuf);
    -+		print_field(format, field.key, &valbuf, &quotbuf);
    ++		field->get_value(repo, &valbuf);
    ++		print_field(format, field->key, valbuf.buf);
     +	}
     +
     +	strbuf_release(&valbuf);
    -+	strbuf_release(&quotbuf);
     +}
     +
      static int parse_format_cb(const struct option *opt,
      			   const char *arg, int unset UNUSED)
      {
    -@@ builtin/repo.c: static int repo_info(int argc, const char **argv, const char *prefix,
    - 		     struct repository *repo)
    +@@ builtin/repo.c: static int cmd_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"),
    -@@ builtin/repo.c: static int repo_info(int argc, const char **argv, const char *prefix,
    +@@ builtin/repo.c: static int cmd_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_BOOL(0, "all", &all_keys, N_("print all keys/values")),
      		OPT_END()
      	};
      
    - 	argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
    +@@ builtin/repo.c: static int cmd_repo_info(int argc, const char **argv, const char *prefix,
    + 	if (format != FORMAT_KEYVALUE && format != FORMAT_NUL_TERMINATED)
    + 		die(_("unsupported output format"));
      
     +	if (all_keys) {
     +		if (argc)
-- 
2.50.1 (Apple Git-155)

[PATCH v4 1/2] repo: factor out field printing to dedicated function

[Lucas Seiki Oshiro]

Move the field printing in git-repo-info to a new function called
`print_field`, allowing it to be called by functions other than
`print_fields`.

Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 builtin/repo.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/builtin/repo.c b/builtin/repo.c
index 9d4749f79b..f9fb418494 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -85,13 +85,29 @@ 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,
+			const char *value)
+{
+	switch (format) {
+	case FORMAT_KEYVALUE:
+		printf("%s=", key);
+		quote_c_style(value, NULL, stdout, 0);
+		putchar('\n');
+		break;
+	case FORMAT_NUL_TERMINATED:
+		printf("%s\n%s%c", key, value, '\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)
 {
 	int ret = 0;
 	struct strbuf valbuf = STRBUF_INIT;
-	struct strbuf quotbuf = STRBUF_INIT;
 
 	for (int i = 0; i < argc; i++) {
 		get_value_fn *get_value;
@@ -105,25 +121,11 @@ static int print_fields(int argc, const char **argv,
 		}
 
 		strbuf_reset(&valbuf);
-		strbuf_reset(&quotbuf);
-
 		get_value(repo, &valbuf);
-
-		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);
-		}
+		print_field(format, key, valbuf.buf);
 	}
 
 	strbuf_release(&valbuf);
-	strbuf_release(&quotbuf);
 	return ret;
 }
 
-- 
2.50.1 (Apple Git-155)

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

[Lucas Seiki Oshiro]

Add a new flag `--all` to git-repo-info for requesting values for 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.

Helped-by: Karthik Nayak <karthik.188@gmail.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 Documentation/git-repo.adoc |  6 +++---
 builtin/repo.c              | 28 +++++++++++++++++++++++++++-
 t/t1900-repo.sh             | 21 +++++++++++++++++++++
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index ce43cb19c8..70f0a6d2e4 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>...]
 git repo structure [--format=(table|keyvalue|nul)]
 
 DESCRIPTION
@@ -19,13 +19,13 @@ 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.
 +
 The output format can be chosen through the flag `--format`. Two formats are
 supported:
diff --git a/builtin/repo.c b/builtin/repo.c
index f9fb418494..22a9ecb3a1 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -15,7 +15,7 @@
 #include "utf8.h"
 
 static const char *const repo_usage[] = {
-	"git repo info [--format=(keyvalue|nul)] [-z] [<key>...]",
+	"git repo info [--format=(keyvalue|nul)] [-z] [--all | <key>...]",
 	"git repo structure [--format=(table|keyvalue|nul)]",
 	NULL
 };
@@ -129,6 +129,22 @@ 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;
+
+	for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
+		const struct field *field = &repo_info_fields[i];
+
+		strbuf_reset(&valbuf);
+		field->get_value(repo, &valbuf);
+		print_field(format, field->key, valbuf.buf);
+	}
+
+	strbuf_release(&valbuf);
+}
+
 static int parse_format_cb(const struct option *opt,
 			   const char *arg, int unset UNUSED)
 {
@@ -152,6 +168,7 @@ static int cmd_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"),
@@ -160,6 +177,7 @@ static int cmd_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_("print all keys/values")),
 		OPT_END()
 	};
 
@@ -167,6 +185,14 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix,
 	if (format != FORMAT_KEYVALUE && format != FORMAT_NUL_TERMINATED)
 		die(_("unsupported output format"));
 
+	if (all_keys) {
+		if (argc)
+			die(_("--all and <key> cannot be used together"));
+
+		print_all_fields(repo, format);
+		return 0;
+	}
+
 	return print_fields(argc, argv, repo, format);
 }
 
diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
index 2beba67889..51d55f11a5 100755
--- a/t/t1900-repo.sh
+++ b/t/t1900-repo.sh
@@ -4,6 +4,15 @@ test_description='test git repo-info'
 
 . ./test-lib.sh
 
+# git-repo-info keys. It must contain the same keys listed in the const
+# repo_info_fields, in lexicographical order.
+REPO_INFO_KEYS='
+	layout.bare
+	layout.shallow
+	object.format
+	references.format
+'
+
 # Test whether a key-value pair is correctly returned
 #
 # Usage: test_repo_info <label> <init command> <repo_name> <key> <expected value>
@@ -110,4 +119,16 @@ 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> aborts' '
+	echo "fatal: --all and <key> cannot be used together" >expect &&
+	test_must_fail git repo info --all object.format 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.50.1 (Apple Git-155)

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

[Junio C Hamano]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> This fourth version of this patch addresses the issues pointed by Eric
> in the v3:
>
> - I dropped the `strbuf quotebuf`, since it can be replaced by
>   outputting `quote_c_style` directly to `stdout`;
>
> - `print_field` now uses the string `value` instead of the
>   `strbuf valbuf`;
>
> - The variable `field` in `print_fields` was replaced by a pointer,
>   since it didn't require to be copied;
>
> - replace the help string by the suggested.

All changes relative to v3 look sensible to me, but I'll have to see
what is outside range-diff (i.e., what was done before v3 and
remains in this iteration), too.

Thanks.

Re: [PATCH v4 1/2] repo: factor out field printing to dedicated function

[Junio C Hamano]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> Move the field printing in git-repo-info to a new function called
> `print_field`, allowing it to be called by functions other than
> `print_fields`.

Missing:

    Also change its use of quote_c_style() helper to output directly to
    the standard output stream, instead of taking a result in a strbuf
    and then printing it outselves.

The patch text looks great.

> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
>  builtin/repo.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/builtin/repo.c b/builtin/repo.c
> index 9d4749f79b..f9fb418494 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -85,13 +85,29 @@ 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,
> +			const char *value)
> +{
> +	switch (format) {
> +	case FORMAT_KEYVALUE:
> +		printf("%s=", key);
> +		quote_c_style(value, NULL, stdout, 0);
> +		putchar('\n');
> +		break;
> +	case FORMAT_NUL_TERMINATED:
> +		printf("%s\n%s%c", key, value, '\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)
>  {
>  	int ret = 0;
>  	struct strbuf valbuf = STRBUF_INIT;
> -	struct strbuf quotbuf = STRBUF_INIT;
>  
>  	for (int i = 0; i < argc; i++) {
>  		get_value_fn *get_value;
> @@ -105,25 +121,11 @@ static int print_fields(int argc, const char **argv,
>  		}
>  
>  		strbuf_reset(&valbuf);
> -		strbuf_reset(&quotbuf);
> -
>  		get_value(repo, &valbuf);
> -
> -		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);
> -		}
> +		print_field(format, key, valbuf.buf);
>  	}
>  
>  	strbuf_release(&valbuf);
> -	strbuf_release(&quotbuf);
>  	return ret;
>  }

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

[Junio C Hamano]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> +static void print_all_fields(struct repository *repo,
> +			     enum output_format format)
> +{
> +	struct strbuf valbuf = STRBUF_INIT;
> +
> +	for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
> +		const struct field *field = &repo_info_fields[i];
> +
> +		strbuf_reset(&valbuf);
> +		field->get_value(repo, &valbuf);
> +		print_field(format, field->key, valbuf.buf);
> +	}
> +	strbuf_release(&valbuf);
> +}

I am not sure if "unsigned long i" is the type you want here.  I do
not mind, and actually I prefer, a simple platform natural "int i"
for something simple like this [*], but I know other people prefer to
use "size_t" to work with ARRAY_SIZE() these days.

    Side note: The reason they insist using size_t here is that
        "-Wsign-compare" makes the compiler complain.  But I would
        say that it only shows what a misguided feature
        -Wsign-compare warning is, especially given that the
        compiler perfectly well knows how big repo_info_fields[]
        array is and the iteration cannot do any harm if done with a
        signed integer smaller than size_t

Anyway.

We grab each field, ask it for its value, and then print it.  What a
straight-forward flow that is pleasant to read! ;-).

Compared to that, printing of individual keys given by the end-user
is much uglier, but it is not a fault of this two-patch series, so
my comment here is only as #leftoverbits for later clean-up.

The print_fields() function does this (modulo error checking for
missing key):

	for (int i = 0; i < argc; i++) {
		get_value_fn *get_value;
		const char *key = argv[i];

		get_value = get_value_fn_for_key(key);
		get_value(repo, &valbuf);

We should get rid of the get_value_fn_for_key() helper, and instead
add and use repo_info_field(const char *key) helepr.  That way, the
logic become exactly the same as the "get all" case.  The body of
the loop would read (modulo error checking for missing key):

		const struct field *field = repo_info_field(argv[i]);
	        field->get_value(repo, &valbuf);

whcih is much nicer, when the repo_info_fields[] gains more
attributes other than a callback function, we do not want to keep
adding get_this_attr_for_key() functions.

    Side note: By the way, it should be named repo_info_field[].
        Name arrays singular so that you can name its 0th element by
        saying dog[0], not dogs[0].  "dog[1] and dog[2] are friends"
        not "dogs[1] and dogs[2] are friends".  An exception is when
        most of the time you use the array as a single unit as a
        collection, passing it around in the call chain, and you
        rarely address each individual element (other than outside
        the implementation of the API).  I am OK to see such an
        array that is mostly used as a collection named plural (but
        of course, singular names are always fine).  Adding this to
        CodingGuidelines is perhaps a #leftoverbits material.

> @@ -167,6 +185,14 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix,
>  	if (format != FORMAT_KEYVALUE && format != FORMAT_NUL_TERMINATED)
>  		die(_("unsupported output format"));
>  
> +	if (all_keys) {
> +		if (argc)
> +			die(_("--all and <key> cannot be used together"));
> +
> +		print_all_fields(repo, format);
> +		return 0;
> +	}
>  	return print_fields(argc, argv, repo, format);

This would work, but the symmetry between a list of keys vs the
"--all" option is lost.

I'd rather see something like the following after a #leftoverbits
clean-up commit:

	if (all_keys && argc)
		die(_("--all and <key> cannot be used together"));

	if (all_keys)
		return print_all_fields(repo, format);
	else
		return print_fields(argc, argv, repo, format);

> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
> index 2beba67889..51d55f11a5 100755
> --- a/t/t1900-repo.sh
> +++ b/t/t1900-repo.sh
> @@ -4,6 +4,15 @@ test_description='test git repo-info'
>  
>  . ./test-lib.sh
>  
> +# git-repo-info keys. It must contain the same keys listed in the const
> +# repo_info_fields, in lexicographical order.
> +REPO_INFO_KEYS='
> +	layout.bare
> +	layout.shallow
> +	object.format
> +	references.format
> +'

Again, this would work for now, but maybe "git repo info --keys"
that emits these would be easier to manage.  This can be left to
#leftoverbits of course.

But then we have seem to have seen too many #leftoverbits material,
you might want to handle some or all of them in this series in a
reroll?  I am starting to become undecided.

With "repo info --keys", the user could even do

    git repo info $(git repo info --keys)

if they wanted to.

No, I am not suggesting to discard the "--all" option; only pointing
out that conceptually, "--all" can be explained in terms of
"--keys".

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

[Lucas Seiki Oshiro]

>> + for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
>> 
> I am not sure if "unsigned long i" is the type you want here.  I do
> not mind, and actually I prefer, a simple platform natural "int i"
> for something simple like this [*], but I know other people prefer to
> use "size_t" to work with ARRAY_SIZE() these days.

Yeah, I also thought it an unsigned long feels out of place, but I
was only following ARRAY_SIZE. Actually, I was trying to avoid a
warning. In this case we have very few `repo_info_field`s and any
int type would work here...

I'll replace it by size_t, then.

>    Side note: The reason they insist using size_t here is that
>        "-Wsign-compare" makes the compiler complain.  But I would
>        say that it only shows what a misguided feature
>        -Wsign-compare warning is, especially given that the
>        compiler perfectly well knows how big repo_info_fields[]
>        array is and the iteration cannot do any harm if done with a
>        signed integer smaller than size_t

Perhaps if ARRAY_SIZE(repo_info_fields) is bigger than the maximum
limit of the integer type, which would overflow and this for would
loop forever. But, obviously this wouldn't happen here.

> This would work, but the symmetry between a list of keys vs the
> "--all" option is lost.
> 
> I'd rather see something like the following after a #leftoverbits
> clean-up commit:
> 
> if (all_keys && argc)
> die(_("--all and <key> cannot be used together"));
> 
> if (all_keys)
> return print_all_fields(repo, format);
> else
> return print_fields(argc, argv, repo, format);

I'll change it in v5.

> Again, this would work for now, but maybe "git repo info --keys"
> that emits these would be easier to manage.  This can be left to
> #leftoverbits of course.

I can't see a use for it other than these tests. What about writing
a helper inside t/helpers for that?

> But then we have seem to have seen too many #leftoverbits material,
> you might want to handle some or all of them in this series in a
> reroll?  I am starting to become undecided.

I agree with all of them, but I think they were too much for this
series... I also think that after git-repo-structure being added to
repo.c I think that it deserves a patchset only for refactoring.

But I'll send a v5 containing the changes directly related to this
series.

Thanks again. I'll send a v5 soon.

[PATCH v5 0/2] repo: add --all to git-repo-info

[Lucas Seiki Oshiro]

Hi!

This v5 addresses the issues pointed by Junio in the previous versions. They
are two small changes:

- `print_all_fields` now has the same signature `print_fields`

- now it uses `size_t` instead of `unsigned long` in a `for` loop

Lucas Seiki Oshiro (2):
  repo: factor out field printing to dedicated function
  repo: add --all to git-repo-info

 Documentation/git-repo.adoc |  6 ++--
 builtin/repo.c              | 63 ++++++++++++++++++++++++++-----------
 t/t1900-repo.sh             | 21 +++++++++++++
 3 files changed, 69 insertions(+), 21 deletions(-)

Range-diff against v4:
1:  fce09770b8 ! 1:  c792715fdd repo: factor out field printing to dedicated function
    @@ Commit message
         `print_field`, allowing it to be called by functions other than
         `print_fields`.
     
    +    Also change its use of quote_c_style() helper to output directly to
    +    the standard output stream, instead of taking a result in a strbuf
    +    and then printing it outselves.
    +
         Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
     
      ## builtin/repo.c ##
2:  ccdad86123 ! 2:  b309d0e13c repo: add --all to git-repo-info
    @@ builtin/repo.c: static int print_fields(int argc, const char **argv,
      	return ret;
      }
      
    -+static void print_all_fields(struct repository *repo,
    -+			     enum output_format format)
    ++static int print_all_fields(struct repository *repo,
    ++			    enum output_format format)
     +{
     +	struct strbuf valbuf = STRBUF_INIT;
     +
    -+	for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
    ++	for (size_t i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
     +		const struct field *field = &repo_info_fields[i];
     +
     +		strbuf_reset(&valbuf);
    @@ builtin/repo.c: static int print_fields(int argc, const char **argv,
     +	}
     +
     +	strbuf_release(&valbuf);
    ++	return 0;
     +}
     +
      static int parse_format_cb(const struct option *opt,
    @@ builtin/repo.c: static int cmd_repo_info(int argc, const char **argv, const char
      	if (format != FORMAT_KEYVALUE && format != FORMAT_NUL_TERMINATED)
      		die(_("unsupported output format"));
      
    -+	if (all_keys) {
    -+		if (argc)
    -+			die(_("--all and <key> cannot be used together"));
    +-	return print_fields(argc, argv, repo, format);
    ++	if (all_keys && argc)
    ++		die(_("--all and <key> cannot be used together"));
     +
    -+		print_all_fields(repo, format);
    -+		return 0;
    -+	}
    -+
    - 	return print_fields(argc, argv, repo, format);
    ++	if (all_keys)
    ++		return print_all_fields(repo, format);
    ++	else
    ++		return print_fields(argc, argv, repo, format);
      }
      
    + struct ref_stats {
     
      ## t/t1900-repo.sh ##
     @@ t/t1900-repo.sh: test_description='test git repo-info'
-- 
2.50.1 (Apple Git-155)

[PATCH v5 1/2] repo: factor out field printing to dedicated function

[Lucas Seiki Oshiro]

Move the field printing in git-repo-info to a new function called
`print_field`, allowing it to be called by functions other than
`print_fields`.

Also change its use of quote_c_style() helper to output directly to
the standard output stream, instead of taking a result in a strbuf
and then printing it outselves.

Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 builtin/repo.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/builtin/repo.c b/builtin/repo.c
index 9d4749f79b..f9fb418494 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -85,13 +85,29 @@ 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,
+			const char *value)
+{
+	switch (format) {
+	case FORMAT_KEYVALUE:
+		printf("%s=", key);
+		quote_c_style(value, NULL, stdout, 0);
+		putchar('\n');
+		break;
+	case FORMAT_NUL_TERMINATED:
+		printf("%s\n%s%c", key, value, '\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)
 {
 	int ret = 0;
 	struct strbuf valbuf = STRBUF_INIT;
-	struct strbuf quotbuf = STRBUF_INIT;
 
 	for (int i = 0; i < argc; i++) {
 		get_value_fn *get_value;
@@ -105,25 +121,11 @@ static int print_fields(int argc, const char **argv,
 		}
 
 		strbuf_reset(&valbuf);
-		strbuf_reset(&quotbuf);
-
 		get_value(repo, &valbuf);
-
-		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);
-		}
+		print_field(format, key, valbuf.buf);
 	}
 
 	strbuf_release(&valbuf);
-	strbuf_release(&quotbuf);
 	return ret;
 }
 
-- 
2.50.1 (Apple Git-155)

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

[Lucas Seiki Oshiro]

Add a new flag `--all` to git-repo-info for requesting values for 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.

Helped-by: Karthik Nayak <karthik.188@gmail.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 Documentation/git-repo.adoc |  6 +++---
 builtin/repo.c              | 29 +++++++++++++++++++++++++++--
 t/t1900-repo.sh             | 21 +++++++++++++++++++++
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index ce43cb19c8..70f0a6d2e4 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>...]
 git repo structure [--format=(table|keyvalue|nul)]
 
 DESCRIPTION
@@ -19,13 +19,13 @@ 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.
 +
 The output format can be chosen through the flag `--format`. Two formats are
 supported:
diff --git a/builtin/repo.c b/builtin/repo.c
index f9fb418494..e30e2416d4 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -15,7 +15,7 @@
 #include "utf8.h"
 
 static const char *const repo_usage[] = {
-	"git repo info [--format=(keyvalue|nul)] [-z] [<key>...]",
+	"git repo info [--format=(keyvalue|nul)] [-z] [--all | <key>...]",
 	"git repo structure [--format=(table|keyvalue|nul)]",
 	NULL
 };
@@ -129,6 +129,23 @@ static int print_fields(int argc, const char **argv,
 	return ret;
 }
 
+static int print_all_fields(struct repository *repo,
+			    enum output_format format)
+{
+	struct strbuf valbuf = STRBUF_INIT;
+
+	for (size_t i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
+		const struct field *field = &repo_info_fields[i];
+
+		strbuf_reset(&valbuf);
+		field->get_value(repo, &valbuf);
+		print_field(format, field->key, valbuf.buf);
+	}
+
+	strbuf_release(&valbuf);
+	return 0;
+}
+
 static int parse_format_cb(const struct option *opt,
 			   const char *arg, int unset UNUSED)
 {
@@ -152,6 +169,7 @@ static int cmd_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"),
@@ -160,6 +178,7 @@ static int cmd_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_("print all keys/values")),
 		OPT_END()
 	};
 
@@ -167,7 +186,13 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix,
 	if (format != FORMAT_KEYVALUE && format != FORMAT_NUL_TERMINATED)
 		die(_("unsupported output format"));
 
-	return print_fields(argc, argv, repo, format);
+	if (all_keys && argc)
+		die(_("--all and <key> cannot be used together"));
+
+	if (all_keys)
+		return print_all_fields(repo, format);
+	else
+		return print_fields(argc, argv, repo, format);
 }
 
 struct ref_stats {
diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
index 2beba67889..51d55f11a5 100755
--- a/t/t1900-repo.sh
+++ b/t/t1900-repo.sh
@@ -4,6 +4,15 @@ test_description='test git repo-info'
 
 . ./test-lib.sh
 
+# git-repo-info keys. It must contain the same keys listed in the const
+# repo_info_fields, in lexicographical order.
+REPO_INFO_KEYS='
+	layout.bare
+	layout.shallow
+	object.format
+	references.format
+'
+
 # Test whether a key-value pair is correctly returned
 #
 # Usage: test_repo_info <label> <init command> <repo_name> <key> <expected value>
@@ -110,4 +119,16 @@ 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> aborts' '
+	echo "fatal: --all and <key> cannot be used together" >expect &&
+	test_must_fail git repo info --all object.format 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.50.1 (Apple Git-155)

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

[Junio C Hamano]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

>>> + for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
>>> 
>> I am not sure if "unsigned long i" is the type you want here.  I do
>> not mind, and actually I prefer, a simple platform natural "int i"
>> for something simple like this [*], but I know other people prefer to
>> use "size_t" to work with ARRAY_SIZE() these days.
>
> Yeah, I also thought it an unsigned long feels out of place, but I
> was only following ARRAY_SIZE. Actually, I was trying to avoid a
> warning. In this case we have very few `repo_info_field`s and any
> int type would work here...
>
> I'll replace it by size_t, then.
>
>>    Side note: The reason they insist using size_t here is that
>>        "-Wsign-compare" makes the compiler complain.  But I would
>>        say that it only shows what a misguided feature
>>        -Wsign-compare warning is, especially given that the
>>        compiler perfectly well knows how big repo_info_fields[]
>>        array is and the iteration cannot do any harm if done with a
>>        signed integer smaller than size_t
>
> Perhaps if ARRAY_SIZE(repo_info_fields) is bigger than the maximum
> limit of the integer type, which would overflow and this for would
> loop forever. But, obviously this wouldn't happen here.

Yes.  We can tell, and a compiler should be able to figure out, that
inside the loop nothing other than increment by one per iteration is
done to "i", and the ARRAY_SIZE(repo_info_fields) is a compile-time
constant that comfortably fits in platform natural "int", so we
know, and a compiler should know, that there is nothing to complain
about if "int i" is used there.

But the quality of implementation of -Wsign-compare may not be good
enough to figure it out.

As ARRAY_SIZE() essentially is a size_t divided by another size_t,
use of size_t is the safest solution that does not require any
braincycle to pick.

>> Again, this would work for now, but maybe "git repo info --keys"
>> that emits these would be easier to manage.  This can be left to
>> #leftoverbits of course.
>
> I can't see a use for it other than these tests. What about writing
> a helper inside t/helpers for that?

If you use "git repo info" only occasionally, wouldn't "git repo
info --keys", if supported, be a useful way to get a more focused
help than "git repo --help" where you have to scan the entire
document and try to find the list of keys that are supported from
there?

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

[Junio C Hamano]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> This v5 addresses the issues pointed by Junio in the previous versions. They
> are two small changes:
>
> - `print_all_fields` now has the same signature `print_fields`
>
> - now it uses `size_t` instead of `unsigned long` in a `for` loop
>
> Lucas Seiki Oshiro (2):
>   repo: factor out field printing to dedicated function
>   repo: add --all to git-repo-info
>
>  Documentation/git-repo.adoc |  6 ++--
>  builtin/repo.c              | 63 ++++++++++++++++++++++++++-----------
>  t/t1900-repo.sh             | 21 +++++++++++++
>  3 files changed, 69 insertions(+), 21 deletions(-)

Looking good.  Will replace.

Shall we mark the topic for 'next' now?

Thanks.

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

[Eric Sunshine]

On Mon, Nov 17, 2025 at 1:58 PM Junio C Hamano <gitster@pobox.com> wrote:
> Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:
> > +     if (all_keys) {
> > +             if (argc)
> > +                     die(_("--all and <key> cannot be used together"));
> > +
> > +             print_all_fields(repo, format);
> > +             return 0;
> > +     }
> >       return print_fields(argc, argv, repo, format);
>
> This would work, but the symmetry between a list of keys vs the
> "--all" option is lost.
>
> I'd rather see something like the following after a #leftoverbits
> clean-up commit:
>
>         if (all_keys && argc)
>                 die(_("--all and <key> cannot be used together"));
>
>         if (all_keys)
>                 return print_all_fields(repo, format);
>         else
>                 return print_fields(argc, argv, repo, format);

For what it's worth, I had the same reaction when reviewing the
previous version, and thought about proposing the same rewrite but
figured that such a comment might fall into the "too subjective"
category, thus omitted it from my review.

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

[Eric Sunshine]

On Tue, Nov 18, 2025 at 4:34 PM Junio C Hamano <gitster@pobox.com> wrote:
> Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:
> > This v5 addresses the issues pointed by Junio in the previous versions. They
> > are two small changes:
> >
> > - `print_all_fields` now has the same signature `print_fields`
> >
> > - now it uses `size_t` instead of `unsigned long` in a `for` loop
>
> Looking good.  Will replace.
>
> Shall we mark the topic for 'next' now?

I think all my review comments have been addressed.

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> For what it's worth, I had the same reaction when reviewing the
> previous version, and thought about proposing the same rewrite but
> figured that such a comment might fall into the "too subjective"
> category, thus omitted it from my review.

Heh, then that makes the subjectiveness a bit weaker? ;-)

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

[Lucas Seiki Oshiro]

> If you use "git repo info" only occasionally, wouldn't "git repo
> info --keys", if supported, be a useful way to get a more focused
> help than "git repo --help" where you have to scan the entire
> document and try to find the list of keys that are supported from
> there?

Hmmm, ok, makes sense. After sending the previous message,
I also thought about it being used for shell completions by
fish or zsh, for example.