Elijah Newren <newren@xxxxxxxxx> 于2023年1月20日周五 13:30写道：
>
> On Thu, Jan 19, 2023 at 9:34 AM ZheNing Hu via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
> >
> > From: ZheNing Hu <adlternative@xxxxxxxxx>
> >
> > Because sometimes we want to check if the files in the
> > index match the sparse specification, so introduce
> > "%(skipworktree)" atom to git ls-files `--format` option.
> > When we use this option, if the file match the sparse
> > specification, it will output "1", otherwise, output
> > empty string "".
>
> Why is that output format useful? It seems like it'll just lead to
> bugs, or someone re-implementing the same field with a different name
> to make it useful in the future. In particular, if there are multiple
> boolean fields and someone specifies e.g.
> git ls-files --format="%(path) %(skipworktree) %(intentToAdd)"
> and both boolean fields are displayed the same way (either a "1" or a
> blank string), and we see something like:
> foo.c 1
> bar.c 1
> Then how do we know if foo.c and bar.c are SKIP_WORKTREE or
> INTENT_TO_ADD? The "1" could have come from either field.
>
I understand your confusion here. If we need to combine these
boolean values in --format with %(if) %(else) of ref-filter in the future,
we can only do this strange design. Output like "true"/"false" or "1"/"0"
would be better without considering %(if), %(else).
> > Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
> > ---
> > Documentation/git-ls-files.txt | 5 +++++
> > builtin/ls-files.c | 3 +++
> > t/t3013-ls-files-format.sh | 23 +++++++++++++++++++++++
> > 3 files changed, 31 insertions(+)
> >
> > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> > index 440043cdb8e..2540b404808 100644
> > --- a/Documentation/git-ls-files.txt
> > +++ b/Documentation/git-ls-files.txt
> > @@ -260,6 +260,11 @@ eolattr::
> > that applies to the path.
> > path::
> > The pathname of the file which is recorded in the index.
> > +skipworktree::
> > + If the file in the index marked with SKIP_WORKTREE bit.
> > + It means the file do not match the sparse specification.
> > + See link:technical/sparse-checkout.txt[sparse-checkout]
> > + for more information.
>
> minor nits: Missing an "is", and "do" should be "does".
>
> I'm curious whether the second sentence is even necessary; we've
> already got the link to the more technical docs. Perhaps just:
>
> skipworktree::
> Whether the file in the index has the SKIP_WORKTREE bit set.
> See link:technical/sparse-checkout.txt[sparse-checkout]
> for more information.
>
Agree.
> > EXCLUDE PATTERNS
> > ----------------
> > diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> > index a03b559ecaa..bbff868ae6b 100644
> > --- a/builtin/ls-files.c
> > +++ b/builtin/ls-files.c
> > @@ -280,6 +280,9 @@ static size_t expand_show_index(struct strbuf *sb, const char *start,
> > data->pathname));
> > else if (skip_prefix(start, "(path)", &p))
> > write_name_to_buf(sb, data->pathname);
> > + else if (skip_prefix(start, "(skipworktree)", &p))
> > + strbuf_addstr(sb, ce_skip_worktree(data->ce) ?
> > + "1" : "");
>
> As I mentioned in response to the commit message, I don't understand
> why having an empty string would be desirable.
>
> > else
> > die(_("bad ls-files format: %%%.*s"), (int)len, start);
> >
> > diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh
> > index efb7450bf1e..cd35dba5930 100755
> > --- a/t/t3013-ls-files-format.sh
> > +++ b/t/t3013-ls-files-format.sh
> > @@ -92,4 +92,27 @@ test_expect_success 'git ls-files --format with --debug' '
> > test_cmp expect actual
> > '
> >
> > +test_expect_success 'git ls-files --format with skipworktree' '
> > + test_when_finished "git sparse-checkout disable" &&
> > + mkdir dir1 dir2 &&
> > + echo "file1" >dir1/file1.txt &&
> > + echo "file2" >dir2/file2.txt &&
> > + git add dir1 dir2 &&
> > + git commit -m skipworktree &&
> > + git sparse-checkout set dir1 &&
> > + git ls-files --format="%(path)%(skipworktree)" >actual &&
> > + cat >expect <<-\EOF &&
> > + dir1/file1.txt
> > + dir2/file2.txt1
> > + o1.txt
> > + o2.txt
> > + o3.txt
> > + o4.txt
> > + o5.txt
> > + o6.txt
> > + o7.txt
> > + EOF
> > + test_cmp expect actual
> > +'
>
> I find this test hard to read; it's just too easy to miss
> "dir2/file2.txt1" vs "dir2/file2.txt". I'd suggest at least adding a
> space, and likely having the skipworktree attribute come first in the
> format string. It might also be useful to add "dir*" on the ls-files
> command to limit which paths are shown, just because there's an awful
> lot of files in the root directory and no variance between them, and
> it's easier to notice the binary difference between two items than
> having a full 9 and figuring out which are relevant.
Good idea.
I deliberately removed the space between %(path) and
%(skipworktree) before, because according to the current design,
the "" output by %(skipworktree) is empty, which leads to an extra
space at the end of the output line, which will break github's
"whitespace" ci tests. Maybe swapping the location of %(path) and
%(skipworktree) will fix this.
