On 25/03/10 01:59PM, Junio C Hamano wrote:
> > +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.
> 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
