From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 20 Jan 2026 06:05:45 GMT
Subject: Re: [PATCH v4 2/2] repo: add new flag --keys to git-repo-info
Message-ID: <aW8bOfVLcG58t-Uk@pks.im>
In-Reply-To: <20260119210932.68893-3-lucasseikioshiro@gmail.com>

On Mon, Jan 19, 2026 at 05:20:20PM -0300, Lucas Seiki Oshiro wrote:
> diff --git a/builtin/repo.c b/builtin/repo.c
> index 306d3fa2df..2f698c5253 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -29,6 +30,7 @@ enum output_format {
>  	FORMAT_TABLE,
>  	FORMAT_KEYVALUE,
>  	FORMAT_NUL_TERMINATED,
> +	FORMAT_LINES
>  };
>  
>  struct field {

Tiny nit: we also tend to terminate the last enum value with a comma.
The reason here is that it makes it easier to add new values going
forward while only having to change one line.

> @@ -149,6 +151,32 @@ static int print_all_fields(struct repository *repo,
>  	return 0;
>  }
>  
> +static int print_keys(enum output_format format)
> +{
> +	char sep;
> +
> +	if (format == FORMAT_DEFAULT)
> +		format = FORMAT_LINES;
> +
> +	switch (format) {
> +	case FORMAT_LINES:
> +		sep = '\n';
> +		break;
> +	case FORMAT_NUL_TERMINATED:
> +		sep = '\0';
> +		break;
> +	default:
> +		die(_("--keys can only be used with --format=default or --format=nul"));

This error message isn't true anymore, as we also support
"--format=lines" now.

> @@ -162,6 +190,8 @@ static int parse_format_cb(const struct option *opt,
>  		*format = FORMAT_KEYVALUE;
>  	else if (!strcmp(arg, "table"))
>  		*format = FORMAT_TABLE;
> +	else if (!strcmp(arg, "lines"))
> +		*format = FORMAT_LINES;
>  	else if (!strcmp(arg, "default"))
>  		*format = FORMAT_DEFAULT;
>  	else

You also have to adapt `cmd_repo_structure()` to handle this new vaule.
Otherwise it would `BUG()`. I guess the most reasonable change here
would be to treat "lines" and "keyvalue" as equivalent?

Thanks!

Patrick