Re: [PATCH 8/8] fetch: introduce machine-parseable "porcelain" output format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Patrick Steinhardt <ps@xxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>> 
>> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux