Re: [PATCH v9 4/4] cat-file: add --batch-command mode

John Cai [off-list ref] writes:

quoted

quoted

+		if (!strcmp(cmd->name, "flush")) {
+			dispatch_calls(opt, output, data, queued_cmd, nr);
+			free_cmds(queued_cmd, nr);
+			nr = 0;

It'd be nice if free_cmds() zeroed nr for us rather than having to remember to do it separately as the two are intimately linked.

This does feel cleaner. Before there was a version where I did this inside of
dispatch_calls and there was feedback that this wasn't clean. But now that
free_cmds prepares the queued_cmd array for reuse, then it may make sense to do
it inside. Though honestly from the back and forth around this, I'm not too sure
what the best thing to do stylistically would be.

I am not sure about style, but at the semantic level, free_cmds()
that "frees" the queued_cmd by releasing the resources it holds and
resets its counter to zero would be a more complete "does one thing
and one thing well" helper function.

quoted

quoted

 +test_expect_success '--batch-command --buffer with flush for blob info' '
+	echo "$hello_sha1 blob $hello_size" >expect &&
+	test_write_lines "info $hello_sha1" "flush" | \

You don't need a '\' after a '|', however it might be better to use the style from the tests above where the '|' is on the beginning of the next line.

Please don't do

	producer \
	| consumer

instead, write

	producer |
	consumer

With two fewer bytes, and is far more common, judging from the
output of

    $ git grep -e '^[   ]*| [A-Za-z]' t

i.e. indent with whitespace or tab, pipe, space and alpha (i.e. the
beginning of the command, possibly a single-shot environment
assignment).