Re: [PATCH v5 1/2] repo: rename "keyvalue" to "lines"
From: Patrick Steinhardt <hidden>
Date: 2026-01-27 06:58:42
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.
quoted hunk ↗ jump to hunk
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