All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Taylor Blau <me@ttaylorr.com>, Toon Claes <toon@iotcl.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/5] catfile: introduce NUL-terminated output format
Date: Tue, 6 Jun 2023 07:19:24 +0200	[thread overview]
Message-ID: <cover.1686028409.git.ps@pks.im> (raw)
In-Reply-To: <cover.1685710884.git.ps@pks.im>

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

Hi,

this is the second version of my patch series that introduces a new NUL
terminated output format to git-cat-file(1) in order to make its output
unambiguously parsable in the case where the input contains newlines.

Changes compared to v1:

    - Improved the commit subject of v5 to mention that the new option
      changes both input and output to be NUL delimited.

    - Extended the commit message to make a better case for why `-Z`
      changes both input and output format to be NUL terminated, instead
      of having `-z` change the input and `-Z` change the output format.

    - The `-Z` option is now sorted before `-z` in git-cat-file(1).

    - The `-z` option is now deprecated "even more", where it is hidden
      from the synopsis as well as from the `-h` output.

    - A small change to pass the delimiter to `batch_write()` directly
      instead of storing it in a temporary array first.

Thanks for your feedback, Junio and Phillip!

Patrick

Patrick Steinhardt (5):
  t1006: don't strip timestamps from expected results
  t1006: modernize test style to use `test_cmp`
  strbuf: provide CRLF-aware helper to read until a specified delimiter
  cat-file: simplify reading from standard input
  cat-file: introduce option to delimit input and output with NUL

 Documentation/git-cat-file.txt |  15 +-
 builtin/cat-file.c             |  85 +++++------
 strbuf.c                       |  11 +-
 strbuf.h                       |  12 ++
 t/t1006-cat-file.sh            | 249 +++++++++++++++++++++------------
 5 files changed, 233 insertions(+), 139 deletions(-)

Range-diff against v1:
1:  5c8b4a1d70 = 1:  5c8b4a1d70 t1006: don't strip timestamps from expected results
2:  251fc2a387 = 2:  251fc2a387 t1006: modernize test style to use `test_cmp`
3:  8127eeac97 = 3:  8127eeac97 strbuf: provide CRLF-aware helper to read until a specified delimiter
4:  e7cba8dc4c = 4:  e7cba8dc4c cat-file: simplify reading from standard input
5:  07a7c34615 ! 5:  79ed618c84 cat-file: Introduce new option to delimit output with NUL characters
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    cat-file: Introduce new option to delimit output with NUL characters
    +    cat-file: introduce option to delimit input and output with NUL
     
         In db9d67f2e9 (builtin/cat-file.c: support NUL-delimited input with
         `-z`, 2022-07-22), we have introduced a new mode to read the input via
    @@ Commit message
         given that revisions containing newlines are quite exotic.
     
         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.
    +    input and output. While this new option could arguably only switch the
    +    output format to be NUL-delimited, the consequence would be that users
    +    have to always specify both `-z` and `-Z` when the input may contain
    +    newlines. On the other hand, if the user knows that there never will be
    +    newlines in the input, they don't have to use either of those options.
    +    There is thus no usecase that would warrant treating input and output
    +    format separately, which is why we instead opt to "do the right thing"
    +    and have `-Z` mean to NUL-terminate both formats.
    +
    +    The old `-z` option is marked as deprecated with a hint that its output
    +    may become unparsable. It is thus hidden both from the synopsis as well
    +    as the command's help output.
     
         Co-authored-by: Toon Claes <toon@iotcl.com>
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
    @@ Documentation/git-cat-file.txt: SYNOPSIS
      'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]
      	     [--buffer] [--follow-symlinks] [--unordered]
     -	     [--textconv | --filters] [-z]
    -+	     [--textconv | --filters] [-z] [-Z]
    ++	     [--textconv | --filters] [-Z]
      'git cat-file' (--textconv | --filters)
      	     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]
      
     @@ Documentation/git-cat-file.txt: 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.
    -+
    + 	/etc/passwd
    + --
    + 
     +-Z::
     +	Only meaningful with `--batch`, `--batch-check`, or
     +	`--batch-command`; input and output is NUL-delimited instead of
    - 	newline-delimited.
    ++	newline-delimited.
    ++
    + -z::
    + 	Only meaningful with `--batch`, `--batch-check`, or
    + 	`--batch-command`; input is NUL-delimited instead of
    +-	newline-delimited.
    ++	newline-delimited. This option is deprecated in favor of
    ++	`-Z` as the output can otherwise be ambiguous.
      
      
    + OUTPUT
     @@ Documentation/git-cat-file.txt: notdir SP <size> LF
      is printed when, during symlink resolution, a file is used as a
      directory name.
    @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,
      	batch_write(opt, scratch->buf, scratch->len);
      
      	if (opt->batch_mode == BATCH_MODE_CONTENTS) {
    -+		char buf[] = {opt->output_delim};
      		print_object_or_die(opt, data);
     -		batch_write(opt, "\n", 1);
    -+		batch_write(opt, buf, 1);
    ++		batch_write(opt, &opt->output_delim, 1);
      	}
      }
      
    @@ builtin/cat-file.c: int cmd_cat_file(int argc, const char **argv, const char *pr
      		N_("git cat-file (--batch | --batch-check | --batch-command) [--batch-all-objects]\n"
      		   "             [--buffer] [--follow-symlinks] [--unordered]\n"
     -		   "             [--textconv | --filters] [-z]"),
    -+		   "             [--textconv | --filters] [-z] [-Z]"),
    ++		   "             [--textconv | --filters] [-Z]"),
      		N_("git cat-file (--textconv | --filters)\n"
      		   "             [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]"),
      		NULL
     @@ builtin/cat-file.c: int cmd_cat_file(int argc, const char **argv, const char *prefix)
    + 			N_("like --batch, but don't emit <contents>"),
      			PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
      			batch_option_callback),
    - 		OPT_BOOL('z', NULL, &input_nul_terminated, N_("stdin is NUL-terminated")),
    +-		OPT_BOOL('z', NULL, &input_nul_terminated, N_("stdin is NUL-terminated")),
    ++		OPT_BOOL_F('z', NULL, &input_nul_terminated, N_("stdin is NUL-terminated"),
    ++			PARSE_OPT_HIDDEN),
     +		OPT_BOOL('Z', NULL, &nul_terminated, N_("stdin and stdout is NUL-terminated")),
      		OPT_CALLBACK_F(0, "batch-command", &batch, N_("format"),
      			N_("read commands from stdin"),
-- 
2.41.0


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

  parent reply	other threads:[~2023-06-06  5:19 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
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 ` Patrick Steinhardt [this message]
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=cover.1686028409.git.ps@pks.im \
    --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.