Re: [PATCH v5 1/2] repo: rename "keyvalue" to "lines"

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jan 23, 2026 at 01:34:53PM -0300, Lucas Seiki Oshiro wrote:
> The output format name "keyvalue" isn't so descriptive. Rename it to
> "lines", since it describes better the syntax of the output format and
> it isn't tied to key-value pairs.

I think this commit message is a bit hand-wavy. Most importantly, the
reader might wonder _why_ it isn't so descriptive, and why these
concerns are strong enough to change the accepted value now.

I would've written something like the following:

    Both subcommands in git-repo(1) accept the "keyvalue" format. This
    format is newline-delimited, where the key is separated from the
    value with an equals sign.

    The name of this option is suboptimal though, as it is both too
    limiting while at the same time not really indicating what it
    actually does:

      - There is no mention of the format being newline-delimited, which
        is the key differentiator to the "nul" format.

      - Both "nul" and "keyvalue" have a key and a value, so the latter
        is not exactly giving any hint what makes it so special.

      - "keyvalue" requires there to be, well, a key and a value, but we
        want to add additional output that is only going to be newline
        delimited.

    Taken together, "keyvalue" is kind of a bad name for this output
    format.

    Luckily, the git-repo(1) command is still rather new and marked as
    experimental, so things aren't cast into stone yet. Rename the
    format to "lines" instead to better indicate that the major
    difference is that we'll get newline-delimited output. This new name
    will also be a better fit for a subsequent extension in git-repo(1).

Please feel free to use this message or parts of it if you plan to
reroll.

I was also briefly wondering whether it would make sense to call the new
format "newlines" instead of "lines", but I'm not feeling strongly about
this in any way.

> diff --git a/builtin/repo.c b/builtin/repo.c
> index 0ea045abc1..4031612bc8 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -26,7 +26,7 @@ typedef int get_value_fn(struct repository *repo, struct strbuf *buf);
>  
>  enum output_format {
>  	FORMAT_TABLE,
> -	FORMAT_KEYVALUE,
> +	FORMAT_LINES,
>  	FORMAT_NUL_TERMINATED,
>  };

This feels a bit unbalanced to me, as `FORMAT_LINES` and
`FORMAT_NUL_TERMINATED` look so different from one another. Maybe it
would be better to call it `FORMAT_NEWLINE_TERMINATED`?

Patrick