All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: Phillip Wood <phillip.wood123@gmail.com>,
	git@vger.kernel.org, Taylor Blau <me@ttaylorr.com>,
	Toon Claes <toon@iotcl.com>
Subject: Re: [PATCH 5/5] cat-file: Introduce new option to delimit output with NUL characters
Date: Tue, 6 Jun 2023 06:52:08 +0200	[thread overview]
Message-ID: <ZH67eBAtFxo95aBL@ncase> (raw)
In-Reply-To: <xmqq35355utz.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 2207 bytes --]

On Tue, Jun 06, 2023 at 08:54:16AM +0900, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> >> Instead, introduce a new option `-Z` that switches to NUL-delimited
> >> input and output. The old `-z` option is marked as deprecated with a
> >> hint that its output may become unparsable.
> >
> > The commit message explains the problem well, I agree adding a new
> > option is the cleanest solution.
> >
> >...
> >>   @@ -246,6 +246,12 @@ respectively print:
> >>   -z::
> >>   	Only meaningful with `--batch`, `--batch-check`, or
> >>   	`--batch-command`; input is NUL-delimited instead of
> >> +	newline-delimited. This option is deprecated in favor of
> >> +	`-Z` as the output can otherwise be ambiguous.
> >> +
> >> +-Z::
> >> +	Only meaningful with `--batch`, `--batch-check`, or
> >> +	`--batch-command`; input and output is NUL-delimited instead of
> >>   	newline-delimited.
> >
> > The documentation changes look good. I wonder if we should put the
> > documentation for "-Z" above "-z" so users see the preferred option
> > first.
> 
> Hmph, I expected "-z" and "-Z" to be orthogonal, the former
> controlling how input records are delimited, the latter controlling
> how output records are delimited, as it usually is a good idea to
> keep things that could be orthogonal to be orthogonal to avoid
> unnecessarily robbing users flexibility.  "-Z is a new way that is
> preferred over -z" was something I did not expect, actually.
> 
> I am not outright rejecting such a deliberately limiting design, but
> I'll have to think about it a bit.

Well, the way I see it we shouldn't have ever decoupled the input and
output format, and I consider it a bug that `-z` did as it makes it
unusable with arbitrary input. So I'd rather be helping the user of
these modes to do the right thing and NUL-delimit both input and output
than running into these edge cases later down the road.

That being said I'd be fine to change this series to mean "-Z changes
stdout" if you insist. In that case we should be pointing out in our
documentation that "You should never use `-z` without `-Z` when you
process arbitrary input".

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2023-06-06  4:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-02 13:02 [PATCH 0/5] cat-file: introduce NUL-terminated output format Patrick Steinhardt
2023-06-02 13:02 ` [PATCH 1/5] t1006: don't strip timestamps from expected results Patrick Steinhardt
2023-06-02 13:02 ` [PATCH 2/5] t1006: modernize test style to use `test_cmp` Patrick Steinhardt
2023-06-02 13:02 ` [PATCH 3/5] strbuf: provide CRLF-aware helper to read until a specified delimiter Patrick Steinhardt
2023-06-02 13:02 ` [PATCH 4/5] cat-file: simplify reading from standard input Patrick Steinhardt
2023-06-02 13:02 ` [PATCH 5/5] cat-file: Introduce new option to delimit output with NUL characters Patrick Steinhardt
2023-06-05 15:47   ` Phillip Wood
2023-06-05 23:54     ` Junio C Hamano
2023-06-06  4:52       ` Patrick Steinhardt [this message]
2023-06-06  5:22         ` Junio C Hamano
2023-06-06  5:31           ` Patrick Steinhardt
2023-06-12 19:12             ` Junio C Hamano
2023-06-06  5:00     ` Patrick Steinhardt
2023-06-06  1:23   ` Junio C Hamano
2023-06-03  1:44 ` [PATCH 0/5] cat-file: introduce NUL-terminated output format Junio C Hamano
2023-06-06  5:19 ` [PATCH v2 0/5] catfile: " Patrick Steinhardt
2023-06-06  5:19   ` [PATCH v2 1/5] t1006: don't strip timestamps from expected results Patrick Steinhardt
2023-06-06  5:19   ` [PATCH v2 2/5] t1006: modernize test style to use `test_cmp` Patrick Steinhardt
2023-06-06  5:19   ` [PATCH v2 3/5] strbuf: provide CRLF-aware helper to read until a specified delimiter Patrick Steinhardt
2023-06-06  5:19   ` [PATCH v2 4/5] cat-file: simplify reading from standard input Patrick Steinhardt
2023-06-06  5:19   ` [PATCH v2 5/5] cat-file: introduce option to delimit input and output with NUL Patrick Steinhardt
2023-06-12 20:43   ` [PATCH v2 0/5] catfile: introduce NUL-terminated output format Junio C Hamano

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=ZH67eBAtFxo95aBL@ncase \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood123@gmail.com \
    --cc=toon@iotcl.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.