Re: [PATCH 3/4] rev-list: support delimiting objects with NUL bytes
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Mar 12, 2025, 21:39 UTC
- Message-ID
- <rrc53bb5433gkdoboubhjrfk4pr6uaykiugiqghwbyhwtigaws@dyzt2buwbasl>
- In-Reply-To
- <xmqqmsds61pw.fsf@gitster.g>
On 25/03/10 01:59PM, Junio C Hamano wrote:
Show 20 quoted lines
> > +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?This is a fair point.
There are scerarios where the printed object format varies more than just the delimiter used. For example, in the normal output mode printed object paths are truncated if they contain a newline. In the NUL-delimited mode we want to print the complete path. Similarly, missing object paths are c-quoted if they contain SP or LF characters. These should also be printed as-is when in the NUL-delimited mode.
Due to branching behavior, I initially thought it would be easier to follow if there was separate variable that signaled printing behavior, but as you mentioned, we could also just use the global to hold the line terminator being used and check if it is set to NUL when there is conditional behavior.
Show 7 quoted lines
> 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).
In the next version, I'll use a global to hold the configured line delimiter. I think it also makes sense to have a separate global variable for the metadata delimiter since in normal mode, it is usually a SP character. When the -z option is detected, we can set both of these to a NUL byte.
-Justin