Re: [PATCH] repo: add new flag --keys to git-repo-info

[Junio C Hamano <gitster@pobox.com>]

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

> Currently, if the user wants to find what are the available keys,
> they need to either check the documentation or to ask to all the
> key-value pairs by using --all.
>
> Add a new flag --keys for listing only the available keys without
> listing the values.

We do not need to say "Currently," but other than that the above is
very well written.  Easy to grok and to the point.

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

So "git repo info --keys --all" or "git repo info --keys --format=..."
is not supported.  Does the implementation behave sensibly when given
such nonsense commands?  Let's see.

> @@ -170,6 +181,7 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix,
>  {
>  	enum output_format format = FORMAT_KEYVALUE;
>  	int all_keys = 0;
> +	int show_keys = 0;
>  	struct option options[] = {
>  		OPT_CALLBACK_F(0, "format", &format, N_("format"),
>  			       N_("output format"),
> @@ -179,10 +191,15 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix,
>  			       PARSE_OPT_NONEG | PARSE_OPT_NOARG,
>  			       parse_format_cb),
>  		OPT_BOOL(0, "all", &all_keys, N_("print all keys/values")),
> +		OPT_BOOL(0, "keys", &show_keys, N_("show keys")),
>  		OPT_END()
>  	};
>  
>  	argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
> +
> +	if (show_keys)
> +		return print_keys();
> +
>  	if (format != FORMAT_KEYVALUE && format != FORMAT_NUL_TERMINATED)
>  		die(_("unsupported output format"));

OK, so it is:

    "git repo info --all --keys" and "git repo info --keys layout.bare"
    both behave as if "git repo --keys" was given, ignoring
    everything else.

Shouldn't "--keys" be explicitly marked incompatible with "--all"
and remaining keys in argc/argv[]?

While there is no strong reason why anybody must use NUL-terminated
output format, simply because repo_info_fields[] contains no tokens
with strange byte values, but just as principle, shouldn't

    "git repo info --keys -z"

do what is naturally expected?

Perhaps

	if (format != ...)
		die(_("unsupported output format"));

	if (show_keys && (all_keys || argc))
		die(_("--keys cannot be used with a <key> or --all"));

	if (show_keys)
		return print_keys(output_format);

with a trivial update to print_keys() to support NUL-terminated
records, instead of puts()?