On Tue, Feb 08 2022, Teng Long wrote:
> + if (*start != '(')
> + die(_("bad ls-tree format: as '%s'"), start);
My typo surely, but I think I menat "as of" not just "as" there:
$ ./git ls-tree --format="%[blah)" -r HEAD
fatal: bad ls-tree format: as of '[blah)'
> +
> + end = strchr(start + 1, ')');
> + if (!end)
> + die(_("bad ls-tree format: element '%s' does not end in ')'"), start);
Or actually:
$ ./git ls-tree --format="%(blah]" -r HEAD
fatal: bad ls-tree format: element '(blah]' does not end in ')'
We could rather say for the first one:
$ ./git ls-tree --format="%[blah)" -r HEAD
fatal: bad ls-tree format: element '[blah)' does not start with '('
> [...]
> + errlen = (unsigned long)len;
> + die(_("bad ls-tree format: %%%.*s"), errlen, start);
I wondered why that %% is there (and I probably wrote this in the first
place, I didn't check:). But it makes sense, because strbuf_expand()
skips past the % for us, and we'd like to say e.g. %(foobar) here, not
(foobar) or whatever.
> new file mode 100755
> index 0000000000..e08c83dc47
> --- /dev/null
> +++ b/t/t3104-ls-tree-format.sh
> @@ -0,0 +1,81 @@
> +#!/bin/sh
> +
> +test_description='ls-tree --format'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
I notice now after commenting on your 13/13 that you should add
TEST_PASSES_SANITIZE_LEAK=true to it (assuming it doesn't leak, which I
don't think it does, but test with SANITIZE=leak first!)
> +test_ls_tree_format () {
> + format=$1 &&
> + opts=$2 &&
> + fmtopts=$3 &&
> + shift 2 &&
> + git ls-tree $opts -r HEAD >expect.raw &&
> + sed "s/^/> /" >expect <expect.raw &&
> + git ls-tree --format="> $format" -r $fmtopts HEAD >actual &&
> + test_cmp expect actual
> +}
I also forgot I wrote this, but also per my comment on 13/13 you can
just add your tests added in 13/13 to this file, then we'll assert that
-r etc. work the same for both.
