From: Glen Choo <chooglen@google.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH 8/8] fetch: introduce machine-parseable "porcelain" output format
Date: Fri, 28 Apr 2023 10:20:02 -0700 [thread overview]
Message-ID: <kl6ledo33ovx.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <ZEuJFOBCdnCLdGc5@ncase>
Patrick Steinhardt <ps@pks.im> writes:
>> I'm happy with either approach as long as we don't have to bikeshed
>> about the "perfect" porcelain output :)
>
> Agreed, and that's why I'm currently defending the "good enough" format.
> It should likely work for most usecases that exist out there. The target
> audience is going to be quite small here as this is not a user-directed
> feature.
Okay, I agree that this is "good enough" for most cases and we can
extend it if needed.
>> My full thoughts on this are in
>>
>> https://lore.kernel.org/git/kl6lildhlz3i.fsf@chooglen-macbookpro.roam.corp.google.com
>>
>> but the short version is that I'm not sure if I expect something as
>> innocuous-looking as --output-format would imply other, machine-friendly
>> things (like stdout instead of stderr), and using --porcelain might make
>> option precedence clearer in some situtations (like if -z is given).
>
> I'm not even sure that `-z` makes sense in this context. If we see cases
> where not using `-z` can cause the machine-readable interface to become
> unparseable then this is a bug in the output format, if you ask me.
> Mostly because the whole intent of it is to be machine-parseable. So if
> we output data that can e.g. contain newlines, then we must not use
> newlines as part of the output format or alternatively escape them. Why
> let the author of the script shoot themselves into the foot?
Agreed. I think the current output format is resilient enough, just that
"-z" is something we choose to support for plumbing anyway.
> Anyway, I'm digressing. It's hard for me to decide what to do right now.
> The thread with Junio and Jacob points into the direction of keeping the
> `--output-format=` interface, while this thread points into the other
> direction. I'm naturally more inclined to keep `--output-format=`,
> mostly because I personally feel like it's the more obvious interface.
> But I also see your point, so it's not really a choice of right-or-wrong
> here, but rather of style.
Yes. All sides seem to understand the tradeoffs, but we value them
differently.
I'd personally prefer to err on the side of consistency, because even if
the existing behavior is less-than-ideal since:
1) This makes it possible for us to fix it in a consistent, well-thought
out way, so we don't have to decide the future for the whole project
in a git-fetch series.
2) At least it still behaves how _some_ users have come to expect it.
As for which is more consistent, a grep for --porcelain over
Documentation/* shows:
- git-push
- git-status
- git-worktree
- git-blame
- git-commit
Most of them already output to stdout. The lone exception is git-push,
which does exactly this "use stdout for the main output, but keep using
stderr for debugging output" behavior you added, so using --porcelain
seems somewhat consistent.
On the other hand, I couldn't find any other option that switches from
stderr to stdout (we sometimes say --stdout to say "use stdout instead
of a file", but that's different), so if we added this behavior to "git
fetch --[output-]format", this might be the first.
(Sidenote: most instances of --format are for templated-style format,
but "git-replace" does accept an enum for it. I don't think this should
weigh heavily in our decision though.)
For the other, non git-push commands, --porcelain is treated like an
output format and respects "last one wins" regular option precedence,
e.g. in the case of "git status --porcelain --short". I find this
behavior somewhat confusing (I'd expect --porcelain to win, or at least
be incompatible), but at least we can consistently change this in the
future [*].
That said, I'm willing to accept that I'm biased toward my own ideas. If
others don't find "--format=porcelain implies stdout" confusing, I can
accept that decision.
[*] In all likelihood, we probably can't make the change to --porcelain
for backwards-compatibility reasons, but we could introduce a synonym
with the behavior we want ("--machine"?) and have OPT_PARSE_MACHINE
handle both --porcelain and --machine.
next prev parent reply other threads:[~2023-04-28 17:20 UTC|newest]
Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-19 12:31 [PATCH 0/8] fetch: introduce machine-parseable output Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 1/8] fetch: split out tests for output format Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 2/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 3/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-04-26 19:20 ` Jacob Keller
2023-04-27 10:58 ` Patrick Steinhardt
2023-04-26 19:21 ` Jacob Keller
2023-04-27 10:58 ` Patrick Steinhardt
2023-04-26 19:25 ` Glen Choo
2023-04-27 10:58 ` Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 4/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 5/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 6/8] fetch: move option related variables " Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 7/8] fetch: introduce new `--output-format` option Patrick Steinhardt
2023-04-26 19:40 ` Glen Choo
2023-04-27 10:58 ` Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-04-26 19:52 ` Glen Choo
2023-04-27 10:58 ` Patrick Steinhardt
2023-04-27 23:20 ` Glen Choo
2023-04-28 8:51 ` Patrick Steinhardt
2023-04-28 17:20 ` Glen Choo [this message]
2023-05-02 20:55 ` Felipe Contreras
2023-04-24 20:17 ` [PATCH 0/8] fetch: introduce machine-parseable output Felipe Contreras
2023-04-25 9:58 ` Patrick Steinhardt
2023-04-26 19:14 ` Jacob Keller
2023-04-26 20:23 ` Junio C Hamano
2023-04-26 20:30 ` Jacob Keller
2023-04-27 10:58 ` Patrick Steinhardt
2023-04-27 19:46 ` Jacob Keller
2023-04-27 22:49 ` Glen Choo
2023-04-26 20:24 ` Junio C Hamano
2023-04-26 18:54 ` Glen Choo
2023-04-26 21:14 ` Glen Choo
2023-04-26 19:17 ` Jacob Keller
2023-04-27 11:13 ` [PATCH v2 " Patrick Steinhardt
2023-04-27 11:13 ` [PATCH v2 1/8] fetch: split out tests for output format Patrick Steinhardt
2023-04-29 17:34 ` SZEDER Gábor
2023-05-03 11:21 ` Patrick Steinhardt
2023-04-27 11:13 ` [PATCH v2 2/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-04-27 11:13 ` [PATCH v2 3/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-04-27 17:26 ` Glen Choo
2023-04-27 19:49 ` Jacob Keller
2023-04-27 11:13 ` [PATCH v2 4/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-04-27 11:13 ` [PATCH v2 5/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-04-27 11:13 ` [PATCH v2 6/8] fetch: move option related variables " Patrick Steinhardt
2023-04-27 21:52 ` Junio C Hamano
2023-04-27 11:13 ` [PATCH v2 7/8] fetch: introduce new `--output-format` option Patrick Steinhardt
2023-04-27 22:01 ` Junio C Hamano
2023-04-28 22:03 ` Glen Choo
2023-05-03 9:12 ` Patrick Steinhardt
2023-04-28 22:31 ` Glen Choo
2023-05-03 9:43 ` Patrick Steinhardt
2023-05-03 11:36 ` Patrick Steinhardt
2023-04-27 11:13 ` [PATCH v2 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-04-27 19:52 ` Jacob Keller
2023-04-28 22:42 ` Glen Choo
2023-05-03 11:34 ` [PATCH v3 0/8] fetch: introduce machine-parseable output Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 1/8] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-08 22:51 ` Glen Choo
2023-05-09 12:41 ` Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 2/8] fetch: split out tests for output format Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 3/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 4/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 5/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 6/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 7/8] fetch: move option related variables " Patrick Steinhardt
2023-05-03 11:34 ` [PATCH v3 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-08 23:42 ` Glen Choo
2023-05-09 12:41 ` Patrick Steinhardt
2023-05-09 0:03 ` Glen Choo
2023-05-03 16:48 ` [PATCH v3 0/8] fetch: introduce machine-parseable output Junio C Hamano
2023-05-03 16:53 ` Junio C Hamano
2023-05-04 7:57 ` Patrick Steinhardt
2023-05-09 0:06 ` Glen Choo
2023-05-09 12:42 ` Patrick Steinhardt
2023-05-09 13:01 ` [PATCH v4 " Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 1/8] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-09 17:49 ` Junio C Hamano
2023-05-09 18:27 ` Glen Choo
2023-05-10 12:34 ` Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 2/8] fetch: split out tests for output format Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 3/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-09 17:58 ` Junio C Hamano
2023-05-10 12:34 ` Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 4/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-05-09 19:28 ` Junio C Hamano
2023-05-10 12:34 ` Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 5/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-09 20:19 ` Junio C Hamano
2023-05-10 12:35 ` Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 6/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-05-09 20:35 ` Junio C Hamano
2023-05-09 22:30 ` Glen Choo
2023-05-10 12:35 ` Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 7/8] fetch: move option related variables " Patrick Steinhardt
2023-05-09 13:02 ` [PATCH v4 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-09 20:43 ` Junio C Hamano
2023-05-10 12:35 ` Patrick Steinhardt
2023-05-10 12:33 ` [PATCH v5 0/9] fetch: introduce machine-parseable output Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 1/9] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 2/9] fetch: split out tests for output format Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 3/9] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 4/9] fetch: print left-hand side when fetching HEAD:foo Patrick Steinhardt
2023-05-12 0:16 ` Glen Choo
2023-05-13 16:59 ` Jeff King
2023-05-15 5:15 ` Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 5/9] fetch: refactor calculation of the display table width Patrick Steinhardt
2023-05-12 0:49 ` Glen Choo
2023-05-10 12:34 ` [PATCH v5 6/9] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 7/9] fetch: lift up parsing of "fetch.output" config variable Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 8/9] fetch: move option related variables into main function Patrick Steinhardt
2023-05-10 12:34 ` [PATCH v5 9/9] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-12 1:02 ` Glen Choo
2023-05-10 18:05 ` [PATCH v5 0/9] fetch: introduce machine-parseable output Junio C Hamano
2023-05-11 11:05 ` Patrick Steinhardt
2023-05-11 16:53 ` Junio C Hamano
2023-05-11 17:24 ` Felipe Contreras
2023-05-12 1:09 ` Glen Choo
2023-05-12 7:16 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=kl6ledo33ovx.fsf@chooglen-macbookpro.roam.corp.google.com \
--to=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).