"ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
It's been quite many iterations, so I'll just comment on the range-diff.
> -+ usage_msg_opt("--format cannot used with -s, -o, -k, -t"
> ++ usage_msg_opt("--format cannot used with -s, -o, -k, -t, "
> + "--resolve-undo, --deduplicate, --eol",
> + ls_files_usage, builtin_ls_files_options);
Looks good.
> @@ t/t3013-ls-files-format.sh (new)
> + printf "LINEONE\nLINETWO\nLINETHREE\n" >o1.txt &&
> + printf "LINEONE\r\nLINETWO\r\nLINETHREE\r\n" >o2.txt &&
> + printf "LINEONE\r\nLINETWO\nLINETHREE\n" >o3.txt &&
> -+ ln -s o3.txt o4.txt &&
> -+ git add "*.txt" &&
> -+ git add --chmod +x o1.txt &&
> -+ git update-index --add --cacheinfo 160000 $(git hash-object o1.txt) o5.txt &&
> ++ git add . &&
We may want to be a bit more strict (like "o?.txt") but because we
know this is the first 'setup' step, let's let it pass.
> ++ oid=$(git hash-object o1.txt) &&
> ++ git update-index --add --cacheinfo 120000 $oid o4.txt &&
> ++ git update-index --add --cacheinfo 160000 $oid o5.txt &&
> ++ git update-index --add --cacheinfo 100755 $oid o6.txt &&
It is a bit inconvenient that --cacheinfo takes only fully-spelled
raw object name that we need to use $oid like this (otherwise we
would be able to write ":o1.txt" instead), but (1) it is not a fault
of this patch, and (2) update-index is a plumbing command meant for
scripts, so it is not too big a problem.
> + git commit -m base
> +'
> +
> +test_expect_success 'git ls-files --format objectmode v.s. -s' '
> -+ git ls-files -s | awk "{print \$1}" >expect &&
> ++ git ls-files -s >files &&
> ++ cut -d" " -f1 files >expect &&
Either "awk" or "cut" is fine and flipping between them is a bit
distracting. Cutting the pipe into two is a good move.
But is this testing the right thing?
> +test_expect_success 'git ls-files --format objectmode v.s. -s' '
> + git ls-files -s >files &&
> + cut -d" " -f1 files >expect &&
> + git ls-files --format="%(objectmode)" >actual &&
> + test_cmp expect actual
> +'
It only looks at the first column of the "-s" output, and we are
implicitly assuming that the order of output does not change between
the "-s" output and "--format=<format>" output. I wonder if it is
more useful and less error prone to come up with a format string
that 100% reproduces the "ls-files -s" output and compare the two,
e.g.
format="%(objectmode) %(objectname) %(stage) %(path)" &&
git ls-files -s >expect &&
git ls-files --format="$format" >actual &&
test_cmp expect actual
I do not know if the $format I wrote without looking at the doc is
correct, but you get the idea.
Thanks.
