Re: [PATCH v7] ls-files: introduce "--format" option

[ZheNing Hu <adlternative@gmail.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2022年7月18日周一 16:29写道：
>
>
> On Wed, Jul 13 2022, ZheNing Hu via GitGitGadget wrote:
>
> > +test_expect_success 'setup' '
> > +     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 &&
>
> If you want to do this sort of thing in general this pattern is better:
>
>         x="a b c" &&
>         printf "%s\n" $x
>         printf "%s\r\n" $x
>

Let see what's these cmd output:

x="a b c" &&
printf "%s\n" $x &&
printf "%s\r\n" $x

a b c
a b c

I guess what we expect is:

a
b
c
a
b
c

test_write_lines() can do this:

test_write_lines a b c
test_write_lines a b c

yeah, maybe printf do this too:

# x="a b c" we don't use a variable
printf "%s\n" a b c &&
printf "%s\r\n" a b c

> I.e. you can use printf's auto-repeating, or test_write_lines[1]. But in
> this case I tried:
>
>         diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh
>         index e62bce70f3b..e4c3a788acb 100755
>         --- a/t/t3013-ls-files-format.sh
>         +++ b/t/t3013-ls-files-format.sh
>         @@ -13,9 +13,11 @@ do
>          done
>
>          test_expect_success 'setup' '
>         -       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 &&
>         +       lines="LO LINETWO LINETHREE" &&
>         +       test_write_lines $lines >o1.txt &&
>         +       # Even this passes!
>         +       #>o1.txt &&
>         +
>                 ln -s o3.txt o4.txt &&
>                 git add "*.txt" &&
>                 git add --chmod +x o1.txt &&
>
> I.e. all tests pass if we don't write o2.txt and o3.txt, and continue to
> pass if you uncomment that and make o1.txt an empty file.
>
> So is this some incomplete test setup code that was never used & we
> could drop?

No, o2.txt, o3.txt just for test some file with different
eolinfo/eolattr, so if we just
keep o1,txt and no o2.txt, o3.txt, it can certainly work with this test case:
'git ls-files --format v.s. --eol'. Other cases don't really need
o2.txt, o3.txt.

By the way: this part of code was copied from t/t0025-crlf-renormalize.sh.
it want to test for three kind file, end with "\n", "\r\n", or mix
with "\n", "\r\n".

>
>
> > +     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 &&
>
> Do:
>
>         oid=$(git hash-object ..) &&
>         git update-index ... "$(oid)"
>
> Otherwise we hide the exit code of "git-hash-object", e.g. if it returns
> the hash and then segfaults.
>

Thanks. I will keep this in my mind.

> > +     git commit -m base
> > +'
> > +
> > +test_expect_success 'git ls-files --format objectmode v.s. -s' '
> > +     git ls-files -s | awk "{print \$1}" >expect &&
>
> Same in this case and below, i.e. let's not hide "git" on the lhs of a
> pipe. So:
>
>         git ls-files >files &&
>         awk ... <files >expect
>
> In this case all your awk-ing can be replaced with (continued)...
>
> > +     git ls-files --format="%(objectmode)" >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format objectname v.s. -s' '
> > +     git ls-files -s | awk "{print \$2}" >expect &&
>
> ...
>
>         cut -d" " -f2
>
> ...
>
> > +     git ls-files --format="%(objectname)" >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format v.s. --eol' '
> > +     git ls-files --eol >tmp &&
> > +     sed -e "s/      / /g" -e "s/  */ /g" tmp >expect 2>err &&
> > +     test_must_be_empty err &&
> > +     git ls-files --format="i/%(eolinfo:index) w/%(eolinfo:worktree) attr/%(eolattr) %(path)" >actual 2>err &&
> > +     test_must_be_empty err &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format path v.s. -s' '
> > +     git ls-files -s | awk "{print \$4}" >expect &&
>
> ...
>
>         cut -f2
>
> I.e. instead of the 4th whitespace field ask for the 2nd \t-delimited
> field. There's nothing wrong with using awk per-se, but let's use the
> simpler "cut" for such a simple use-case.
>
> > +     git ls-files --format="%(path)" >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format with -m' '
> > +     echo change >o1.txt &&
> > +     cat >expect <<-\EOF &&
> > +     o1.txt
> > +     o5.txt
> > +     EOF
> > +     git ls-files --format="%(path)" -m >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format with -d' '
> > +     echo o6 >o6.txt &&
> > +     git add o6.txt &&
> > +     rm o6.txt &&
> > +     cat >expect <<-\EOF &&
> > +     o5.txt
> > +     o6.txt
> > +     EOF
> > +     git ls-files --format="%(path)" -d >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format imitate --stage' '
> > +     git ls-files --stage >expect &&
> > +     git ls-files --format="%(objectmode) %(objectname) %(stage)%x09%(path)" >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format with --debug' '
> > +     git ls-files --debug >expect &&
> > +     git ls-files --format="%(path)" --debug >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_done
>
> The rest of this (and especially the C code) all looks good to me at
> this point, thanks!
>
> 1. Aside: I've found the test_write_lines helper to be rather strange
>    for us to carry. I.e. most helpers provide a briefer or less
>    buggy/tricky way to do something, but in that case:
>
>         test_write_lines
>         printf "%s\n"
>
>    So we have it to write something in a more verbose way than we need,
>    as we can see experimentally from all tests passing with:
>
>         perl -pi -e 's[test_write_lines ][printf "%s\\n" ]g' t/t[0-9]*.sh
>
>    It seems to me that per
>    https://lore.kernel.org/git/xmqqioqu5fr3.fsf@gitster.dls.corp.google.com/
>    and
>    https://lore.kernel.org/git/1398255277-26303-2-git-send-email-mst@redhat.com/
>    it was suggested without knowing that we could use printf to do the
>    same.
>
>    The implementation that landed in ac9afcc31cd (test: add
>    test_write_lines helper, 2014-04-27) was fixed up to use printf,
>    without re-visiting why we were carrying a helper when an even
>    shorter printf would do...

I guess it's just for letting contributers use the same way output multiple
line, otherwise contributers (like me) want to use echo or other commands
sometimes?

But in my test case here, I need a file mixed with "r\n", "\n" instread of
only "\n", so I think maybe we should keep it.

ZheNing Hu