Subject: Re: [PATCH 3/4] rev-list: support delimiting objects with NUL bytes

From: Junio C Hamano <[email-masked]>
Date: Mon, 10 Mar 2025 13:59:55 -0700

Justin Tobler <[email-masked]> writes:

> +-z::
> +	Instead of being newline-delimited, each outputted object is delimited
> +	with two NUL bytes in the following form:
> ++
> +-----------------------------------------------------------------------
> +<OID> NUL NUL
> +-----------------------------------------------------------------------
> ++
> +When the `--objects` option is also present, available object name information
> +is printed in the following form without any truncation for object names
> +containing newline characters:
> ++
> +-----------------------------------------------------------------------
> +<OID> [NUL <object-name>] NUL NUL
> +-----------------------------------------------------------------------
> ++
> +This option is only compatible with `--objects`.
>  endif::git-rev-list[]

As I said, I do not think we strongly want double-NUL and would
prefer to do away with a single NUL if possible.

> +static int nul_delim;
>  static int show_disk_usage;
>  static off_t total_disk_usage;
>  static int human_readable;
>  
> +static void print_object_term(int nul_delim)
> +{
> +	char line_sep = '\n';
> +
> +	if (nul_delim)
> +		line_sep = '\0';
> +
> +	putchar(line_sep);
> +	if (nul_delim)
> +		putchar(line_sep);
> +}

This looks, to put it mildly, strange.  The concept of the line
delimiter byte (which can take any single byte) is wider than having
a NUL as the line delimiter byte.  Why would we even want both?

IOW, wouldn't it make more sense to have line_delim as the global
(or per-invocation parameter to this function) and have
print_object_term() just use it?  If you want to make it behave
differently only when line-delimter is NUL (which I do not
recommend), you can switch on the value of line_delimiter being NUL.
So I do not see a merit in having two separate variables (except for
confusing future readers).