ls-files: add %(skipworktree) atom to format option

From: ZheNing Hu <[email protected]>

Because sometimes we want to check if the files in the index match the sparse specification by using git ls-files -t, but -t option have semi-deprecated,

So introduce "%(skipworktree)" atom to git ls-files --format option.

When we use this option, if the file
match the sparse specification and removed from working tree...

...it will output "yes", othewise, output "no".

Signed-off-by ZheNing Hu <[email protected]>

Documentation/git-ls-files.txt +0 -0

index 440043cdb8e..0e50307121d 100644

Documentation/git-ls-files.txt +0 -0

 EXCLUDE PATTERNS
 ----------------

builtin/ls-files.c +3 -0

index a03b559ecaa..d1a27f28f01 100644
@@ -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) ?
+                             "true" : "false");
        else
                die(_("bad ls-files format: %%%.*s"), (int)len, start);
 

t/t3013-ls-files-format.sh +0 -0

index efb7450bf1e..ac8b865c275 100755

t/t3013-ls-files-format.sh +10 -0

+       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 false
+       dir2/file2.txt true

From: ZheNing Hu <[email protected]>

Because sometimes we want to check if the files in the index match the sparse specification by using git ls-files -t, but -t option have semi-deprecated,

So introduce "%(skipworktree)" atom to git ls-files --format option. When we use this option, if the file match the sparse specification and removed from working tree, it will output "yes", othewise, output "no".

Signed-off-by ZheNing Hu <[email protected]>

---
ls-files: add %(skipworktree) atom to format option

git ls-files -t is semi-deprecated, but in face we need to use -t option to check if a file in the index match the sparse specification.

So I think this feature can be migrated to git ls-files --format, add a %(skipworktree) atom to indicate whether the file in the index match the sparse specification and is removed from the working tree.

v1: add %(skipworktree) atom to git ls-files format option.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1458%2Fadlternative%2Fzh%2Fls-file-format-skipworktree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1458/adlternative/zh/ls-file-format-skipworktree-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1458

 Documentation/git-ls-files.txt |  6 ++++++
 builtin/ls-files.c             |  3 +++
 t/t3013-ls-files-format.sh     | 22 ++++++++++++++++++++++
 3 files changed, 31 insertions(+)

Documentation/git-ls-files.txt +0 -0

index 440043cdb8e..0e50307121d 100644

Given my above comments, I personally don't buy this as justification for adding a new way of reporting on the skip-worktree bit. It may still make sense to add this feature or something like it, but I personally think it deserves separate justification from "`ls-files

-t` is semi-deprecated".
...
To be honest, I don't yet see any compelling reason to use this new option. Even if this patch is accepted, I'd just continue using "git ls-files -t" (both directly and in scripts) in preference to this. However, you have inspired me to try to fix up the ls-files documentation and remove the "semi-deprecated" label for the -t option.

Quoted 4 lines · Elijah Newren

Given my above comments, I personally don't buy this as justification for adding a new way of reporting on the skip-worktree bit. It may still make sense to add this feature or something like it, but I personally think it deserves separate justification from "`ls-files

Quoted 8 lines

-t` is semi-deprecated".
...
To be honest, I don't yet see any compelling reason to use this new option. Even if this patch is accepted, I'd just continue using "git ls-files -t" (both directly and in scripts) in preference to this. However, you have inspired me to try to fix up the ls-files documentation and remove the "semi-deprecated" label for the -t option.

Thanks. I think that would be the better way forward between the two (i.e. adding this one-shot new feature vs resurrecting -t).

Quoted 6 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>

Because sometimes we want to check if the files in the index match the sparse specification by using git ls-files -t, but -t option have semi-deprecated,

Where semi-deprecated was explicitly "suggest other functionality in preference, but do not ever remove"[1]. The "suggest other functionality in preference" for "ls-files -t" came about because people got confused about staged files which have (a) no unstaged changes, vs. (b) unstaged content changes, vs. (c) unstaged deletion of the file. Such users accidentally presumed that "H" (defined simply as "cached") should only refer to class (a) when it refers to the fact that the file is tracked (and not conflicted) and thus refers to any of (a), (b), and (c)[2]. (I wonder if changing the definition of "H" in the manual from "cached" to "tracked-and-not-conflicted" would fix this confusion.) In contrast, comparing tracked vs.

not-tracked-because-skip-worktree files, the distinction between "H" and "S" makes lots of sense and naturally you want "H" to represent all 3 of (a), (b), and (c). So, for the skip-worktree bit usecase, "ls-files -t" doesn't cause the same confusion. (Perhaps the fact that we have a tri-state of "M" (unmerged) vs "S" (skip-worktree) vs. "H" (all other tracked files) could cause minor confusion, but in practice the possibility of "M" just hasn't seemed to have caused issues for sparse-checkout users or scripts.)

Further, since sparse-checkouts and monorepos really started taking off 4-5 years ago, "git ls-files -t" has been used *heavily* (but mostly by low-level script things rather than user-facing UI). If we wanted to come up with a better place to report on the skip-worktree bit and have scripts rely on that, we probably should have made such a change back then...if not another 8-9 years earlier. At this point, "ls-files -t" simply cannot be removed, even if we wanted to.

[1] As per this quote from 5bc0e247c4 ("Document ls-files -t as semi-obsolete.", 2010-07-28):
"git ls-files -t" is [...] badly documented, hence we point the
users to superior alternatives.
The feature is marked as "semi-obsolete" but not "scheduled for removal" since it's a plumbing command, scripts might use it, and Git testsuite already uses it to test the state of the index.
[2] https://lore.kernel.org/git/[email protected]/
(a.k.a. gmane:126516 from the commit message referenced above)

Quoted 2 lines

So introduce "%(skipworktree)" atom to git ls-files --format option.

Given my above comments, I personally don't buy this as justification for adding a new way of reporting on the skip-worktree bit. It may still make sense to add this feature or something like it, but I personally think it deserves separate justification from "ls-files -t is semi-deprecated".

(Others, of course, may disagree with me, but if this is the only justification for this change, then I'm more likely to want to fix the ls-files manual to remove the "semi-deprecated" notice and fix the definition of "H" to be less misleading than to make a change like this.)

Quoted 3 lines

When we use this option, if the file
match the sparse specification and removed from working tree...

The "and removed from working tree" portion of this sentence is superfluous. (And actually makes it harder to understand, I had to try to think through a bunch of cases to try to figure out why you might be trying to add some extra qualifier.)

Quoted 1 line

...it will output "yes", othewise, output "no".

typo in "otherwise".

Also, your commit message claims output different from what your code below implements and what your testcase shows. ("yes"/"no" vs. "true"/"false")

Quoted 1 line

Signed-off-by ZheNing Hu <[email protected]>

[...]

Quoted 9 lines

Documentation/git-ls-files.txt +1 -0

index 440043cdb8e..0e50307121d 100644
@@ -260,6 +260,12 @@ eolattr::
        that applies to the path.
 path::
        The pathname of the file which is recorded in the index.
+skipworktree::

Documentation/git-ls-files.txt eolattr:: · line 268

Quoted 5 lines

Documentation/git-ls-files.txt +5 -0

+       If the file in the index marked with SKIP_WORKTREE bit.
+       It means the file do not match the sparse specification
+       and removed from working tree.
+       See link:technical/sparse-checkout.txt[sparse-checkout]
+       for more information.

Should the actual wording be included here? (i.e. "yes"/"no", "true"/"false", or whatever you end up using)?

Quoted 24 lines

Documentation/git-ls-files.txt +0 -0

 EXCLUDE PATTERNS
 ----------------

builtin/ls-files.c +3 -0

index a03b559ecaa..d1a27f28f01 100644
@@ -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) ?
+                             "true" : "false");
        else
                die(_("bad ls-files format: %%%.*s"), (int)len, start);
 

t/t3013-ls-files-format.sh +0 -0

index efb7450bf1e..ac8b865c275 100755
@@ -92,4 +92,26 @@ test_expect_success 'git ls-files --format with --debug' '
        test_cmp expect actual
 '
 

t/t3013-ls-files-format.sh test_expect_success 'git ls-files --format with --debug' ' · line 95

Quoted 1 line

t/t3013-ls-files-format.sh +1 -0

+test_expect_success 'git ls-files --format with skipworktree' '

Should probably add a

    test_when_finished "git sparse-checkout disable" &&

at the beginning of this test, otherwise you are potentially causing confusion to future developers who try to add additional testcases to this file.

Quoted 10 lines

t/t3013-ls-files-format.sh +10 -0

+       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 false
+       dir2/file2.txt true

t/t3013-ls-files-format.sh test_expect_success 'git ls-files --format with --debug' ' · line 115

Quoted 10 lines

t/t3013-ls-files-format.sh +10 -0

+       o1.txt false
+       o2.txt false
+       o3.txt false
+       o4.txt false
+       o5.txt false
+       o6.txt false
+       o7.txt false
+       EOF
+       test_cmp expect actual
+'

To be honest, I don't yet see any compelling reason to use this new option. Even if this patch is accepted, I'd just continue using "git ls-files -t" (both directly and in scripts) in preference to this. However, you have inspired me to try to fix up the ls-files documentation and remove the "semi-deprecated" label for the -t option. Patches over here:
https://github.com/gitgitgadget/git/pull/1463 ; I'll submit them in the next few days.

Quoted 6 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>

Because sometimes we want to check if the files in the index match the sparse specification by using git ls-files -t, but -t option have semi-deprecated,

Quoted 4 lines

So introduce "%(skipworktree)" atom to git ls-files --format option. When we use this option, if the file match the sparse specification and removed from working tree, it will output "yes", othewise, output "no".

Your code says "true" and "false", not "yes" or "no". Which is it ?:)

More generally it's unfortanute that we don't support the ref-filter.c formats more generally, then we could just make this 1 or the empty string, and you'd do:

        %(if)%(skipworktree)%(then)true%(else)false%(end)

Now, I don't think your change needs to be blocked by generalizing that if/else stuff in ref-filter, that would probably be some thousand-line series if we're lucky, even though it's a good eventual goal.

But I feel strongly that you should not pick "true", or "false", or "yes" or "no", but rather "1" or "", here, as doing so will be future-proof when it comes to this format being compatible with using the ref-filter.c conditional support.

Quoted 1 line

Quoted 27 lines

Signed-off-by ZheNing Hu <[email protected]>

---
ls-files: add %(skipworktree) atom to format option

git ls-files -t is semi-deprecated, but in face we need to use -t option to check if a file in the index match the sparse specification.

So I think this feature can be migrated to git ls-files --format, add a %(skipworktree) atom to indicate whether the file in the index match the sparse specification and is removed from the working tree.

v1: add %(skipworktree) atom to git ls-files format option.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1458%2Fadlternative%2Fzh%2Fls-file-format-skipworktree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1458/adlternative/zh/ls-file-format-skipworktree-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1458

 Documentation/git-ls-files.txt |  6 ++++++
 builtin/ls-files.c             |  3 +++
 t/t3013-ls-files-format.sh     | 22 ++++++++++++++++++++++
 3 files changed, 31 insertions(+)

Documentation/git-ls-files.txt +0 -0

index 440043cdb8e..0e50307121d 100644

Documentation/git-ls-files.txt eolattr:: · line 268

Quoted 9 lines

Documentation/git-ls-files.txt +6 -0

@@ -260,6 +260,12 @@ 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
+     and removed from working tree.
+     See link:technical/sparse-checkout.txt[sparse-checkout]
+     for more information.

I likewise think this won't need to be blocked, but this is a sign that we should probably move these to the main doc namespace. See 1e2320161d2 (docs: move http-protocol docs to man section 5, 2022-08-04) (and commits before that) for prior art.

But what we should fix here is that this ling is wrong, as you can see in that commit we need to link to the HTML docs for these (confusing as that is).

Or (and I didn't check) if we never generate the *.html either for this particular one this link will always be broken. I.e. we won't install this, nor the HTML docs.

Quoted 16 lines · Junio C Hamano

Quoted 12 lines · Elijah Newren

Given my above comments, I personally don't buy this as justification for adding a new way of reporting on the skip-worktree bit. It may still make sense to add this feature or something like it, but I personally think it deserves separate justification from "ls-files -t is semi-deprecated".
...
To be honest, I don't yet see any compelling reason to use this new option. Even if this patch is accepted, I'd just continue using "git ls-files -t" (both directly and in scripts) in preference to this. However, you have inspired me to try to fix up the ls-files documentation and remove the "semi-deprecated" label for the -t option.

Quoted 2 lines

Thanks. I think that would be the better way forward between the two (i.e. adding this one-shot new feature vs resurrecting -t).

Submitted over here:
https://lore.kernel.org/git/[email protected]/

Quoted 4 lines · Elijah Newren <[email protected]> 于2023年1月12日周四 18:00写道：

Quoted 1 line · ZheNing Hu via GitGitGadget

Quoted 5 lines

The "and removed from working tree" portion of this sentence is superfluous. (And actually makes it harder to understand, I had to try to think through a bunch of cases to try to figure out why you might be trying to add some extra qualifier.)

I just quoted the definition of SKIP_WORKTREE from Documentation/technical/sparse-checkout.txt:

SKIP_WORKTREE: When tracked files do not match the sparse specification and
are removed from the working tree, the file in the index is marked with a SKIP_WORKTREE bit.

To be honest, right now I think %(skipworktree) just refines the --format option's ability to read the index entry SKIP_WORKTREE flag bits. It is probably still worth keeping.

From: ZheNing Hu <[email protected]>

There is a linking error when other documents want to refer to technical/sparse-checkout.txt, Also, there is something wrong with the format of the paragraphs in sparse-checkout.txt, which makes acsiidoc compile errors.

So fix the format of sparse-checkout.txt, and link it in the Makefile to correct that.

Signed-off-by ZheNing Hu <[email protected]>

---
 Documentation/Makefile                      |  1 +
 Documentation/technical/sparse-checkout.txt | 43 ++++++++++++++-------
 2 files changed, 30 insertions(+), 14 deletions(-)

Documentation/Makefile +1 -0

index 9c67c3a1c50..7540da27b8c 100644
@@ -121,6 +121,7 @@ TECH_DOCS += technical/reftable
 TECH_DOCS += technical/scalar
 TECH_DOCS += technical/send-pack-pipeline
 TECH_DOCS += technical/shallow
+TECH_DOCS += technical/sparse-checkout
 TECH_DOCS += technical/trivial-merge
 SP_ARTICLES += $(TECH_DOCS)
 SP_ARTICLES += technical/api-index

Documentation/technical/sparse-checkout.txt +23 -10

index fa0d01cbda4..52e86764a6c 100644
@@ -1,3 +1,6 @@
+Sparse Checkout Design Notes
+============================
+
 Table of contents:
 
   * Terminology

@@ -14,7 +17,8 @@ Table of contents:
   * Reference Emails
 
 
-=== Terminology ===
+Terminology
+-----------
 
 cone mode: one of two modes for specifying the desired subset of files
        in a sparse-checkout.  In cone-mode, the user specifies

@@ -92,7 +96,8 @@ vivifying: When a command restores a tracked file to the working tree (and
        file), this is referred to as "vivifying" the file.
 
 
-=== Purpose of sparse-checkouts ===
+Purpose of sparse-checkouts
+---------------------------
 
 sparse-checkouts exist to allow users to work with a subset of their
 files.

@@ -255,7 +260,8 @@ will perceive the checkout as dense, and commands should thus behave as if
 all files are present.
 
 
-=== Usecases of primary concern ===
+Usecases of primary concern
+---------------------------
 
 Most of the rest of this document will focus on Behavior A and Behavior
 B.  Some notes about the other two cases and why we are not focusing on

@@ -300,7 +306,8 @@ Behavior C do not assume they are part of the Behavior B camp and propose
 patches that break things for the real Behavior B folks.
 
 
-=== Oversimplified mental models ===
+Oversimplified mental models
+----------------------------
 
 An oversimplification of the differences in the above behaviors is:
 

@@ -313,7 +320,8 @@ An oversimplification of the differences in the above behaviors is:
              they can later lazily be populated instead.
 
 
-=== Desired behavior ===
+Desired behavior
+----------------
 
 As noted previously, despite the simple idea of just working with a subset
 of files, there are a range of different behavioral changes that need to be

@@ -544,7 +552,8 @@ understanding these differences can be beneficial.
   * gitk?
 
 
-=== Behavior classes ===
+Behavior classes
+----------------
 
 From the above there are a few classes of behavior:
 

@@ -611,7 +620,8 @@ From the above there are a few classes of behavior:
     specification.
 
 
-=== Subcommand-dependent defaults ===
+Subcommand-dependent defaults
+-----------------------------
 
 Note that we have different defaults depending on the command for the
 desired behavior :

@@ -751,7 +761,8 @@ desired behavior :
     implemented.
 
 
-=== Sparse specification vs. sparsity patterns ===
+Sparse specification vs. sparsity patterns
+------------------------------------------
 
 In a well-behaved situation, the sparse specification is given directly
 by the $GIT_DIR/info/sparse-checkout file.  However, it can transiently

@@ -823,7 +834,8 @@ under behavior B index operations are lumped with history and tend to
 operate full-tree.
 
 
-=== Implementation Questions ===
+Implementation Questions
+------------------------
 
   * Do the options --scope={sparse,all} sound good to others?  Are there better
     options?

@@ -894,7 +906,8 @@ operate full-tree.
     is seamless for them.
 
 
-=== Implementation Goals/Plans ===
+Implementation Goals/Plans
+--------------------------
 
  * Get buy-in on this document in general.
 

Documentation/technical/sparse-checkout.txt +2 -2

 1. am and apply:
 

@@ -1052,7 +1066,8 @@ been working on it.
     https://lore.kernel.org/git/CABPp-BEkJQoKZsQGCYioyga_uoDQ6iBeW+FKr8JhyuuTMK1RDw@mail.gmail.com/
 
 
-=== Reference Emails ===
+Reference Emails
+----------------
 
 Emails that detail various bugs we've had in sparse-checkout:
 
--

gitgitgadget

From: ZheNing Hu <[email protected]>

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 "".

Signed-off-by ZheNing Hu <[email protected]>

---
 Documentation/git-ls-files.txt |  5 +++++
 builtin/ls-files.c             |  3 +++
 t/t3013-ls-files-format.sh     | 23 +++++++++++++++++++++++
 3 files changed, 31 insertions(+)

Documentation/git-ls-files.txt +0 -0

index 440043cdb8e..2540b404808 100644

builtin/ls-files.c +0 -0

        else
                die(_("bad ls-files format: %%%.*s"), (int)len, start);
 

t/t3013-ls-files-format.sh +12 -0

index efb7450bf1e..cd35dba5930 100755
@@ -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

Quoted 4 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>
So fix the format of sparse-checkout.txt, and link it in the Makefile to correct that.

Documentation/technical/sparse-checkout.txt operate full-tree. · line 945

Quoted 4 lines

-0. Behavior A is not well supported in Git. (Behavior B didn't used to

Documentation/technical/sparse-checkout.txt +2 -1

-   be either, but was the easier of the two to implement.)
+Behavior A is not well supported in Git.  (Behavior B didn't used to
+be either, but was the easier of the two to implement.)

Why did you remove the numbering on this, though? If asciidoc doesn't like starting with 0 (the only guess I can think of for why you'd change this), shouldn't the series be renumbered starting at 1 rather than removing this from the list?

Quoted 9 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>

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.

Quoted 4 lines

This list used to be a lot longer (see e.g. [1,2,3,4,5,6,7,8,9]), but we've been working on it.

Documentation/technical/sparse-checkout.txt operate full-tree. · line 945

Quoted 4 lines

-0. Behavior A is not well supported in Git. (Behavior B didn't used to

Documentation/technical/sparse-checkout.txt +2 -1

-   be either, but was the easier of the two to implement.)
+Behavior A is not well supported in Git.  (Behavior B didn't used to
+be either, but was the easier of the two to implement.)

Why did you remove the numbering on this, though? If asciidoc doesn't like starting with 0 (the only guess I can think of for why you'd change this), shouldn't the series be renumbered starting at 1 rather than removing this from the list?

Quoted 14 lines

1. am and apply:

Documentation/technical/sparse-checkout.txt +2 -2

@@ -1052,7 +1066,8 @@ been working on it.
     https://lore.kernel.org/git/CABPp-BEkJQoKZsQGCYioyga_uoDQ6iBeW+FKr8JhyuuTMK1RDw@mail.gmail.com/
 
 
-=== Reference Emails ===
+Reference Emails
+----------------
 
 Emails that detail various bugs we've had in sparse-checkout:
 
--

gitgitgadget

Quoted 8 lines · Elijah Newren

Quoted 4 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>
So fix the format of sparse-checkout.txt, and link it in the Makefile to correct that.

Quoted 9 lines

Documentation/technical/sparse-checkout.txt operate full-tree. · line 945

Quoted 4 lines

-0. Behavior A is not well supported in Git. (Behavior B didn't used to

Documentation/technical/sparse-checkout.txt +2 -1

-   be either, but was the easier of the two to implement.)
+Behavior A is not well supported in Git.  (Behavior B didn't used to
+be either, but was the easier of the two to implement.)

Why did you remove the numbering on this, though? If asciidoc doesn't like starting with 0 (the only guess I can think of for why you'd change this), shouldn't the series be renumbered starting at 1 rather than removing this from the list?

It looks like the zero causes both asciidoc and Asciidoctor to emit warnings (one per item, since each item's number is off by one). They also helpfully relabel everything starting at 1.

I agree that there's a better fix here than dropping the 0. Either renumber everything or, probably better, just use "." for each item rather than "1.", "2." and so on. The right numbers will be inserted automatically. This also means that if an item is ever added earlier in the list, we won't need to update all the numbers below that point.

(The numbers being generated automatically means we can't refer to them ("see item 2") without potentially getting out of sync, but that is true regardless if the numbers are corrected for us, as now, or if we just use ".".)

The contents of these list items could be prettified in various ways, but I'm not sure what the status and goal is for these technical/ documents. Avoiding warnings in the build process, as ZheNing aimed for, seems like a good idea regardless.

Martin

Quoted 9 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>

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.

Quoted 12 lines

Signed-off-by ZheNing Hu <[email protected]>

---
 Documentation/git-ls-files.txt |  5 +++++
 builtin/ls-files.c             |  3 +++
 t/t3013-ls-files-format.sh     | 23 +++++++++++++++++++++++
 3 files changed, 31 insertions(+)

Documentation/git-ls-files.txt +0 -0

index 440043cdb8e..2540b404808 100644

Documentation/git-ls-files.txt eolattr:: · line 267

Quoted 8 lines

Documentation/git-ls-files.txt +5 -0

@@ -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.

builtin/ls-files.c static size_t expand_show_index(struct strbuf *sb, const char *start, · line 285

Quoted 13 lines

Documentation/git-ls-files.txt +0 -0

 EXCLUDE PATTERNS
 ----------------

builtin/ls-files.c +3 -0

index a03b559ecaa..bbff868ae6b 100644
@@ -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.

Quoted 23 lines

builtin/ls-files.c +0 -0

        else
                die(_("bad ls-files format: %%%.*s"), (int)len, start);
 

t/t3013-ls-files-format.sh +12 -0

index efb7450bf1e..cd35dba5930 100755
@@ -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

t/t3013-ls-files-format.sh test_expect_success 'git ls-files --format with --debug' ' · line 116

Quoted 10 lines

t/t3013-ls-files-format.sh +10 -0

+       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.

Quoted 19 lines · Elijah Newren

Quoted 9 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>

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:

Quoted 4 lines

   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.

Perhaps it becomes useful in conjunction with %(if) and friends, when they become avaiable?

Until then, I agree that the output format looks pretty klunky. The calling scripts still can do

        --format='%(path) A=%(A) B=%(B) C=%(C)'

and take an empty value as false, though.

Quoted 3 lines

Perhaps it becomes useful in conjunction with %(if) and friends, when they become avaiable?
...

Can this strange design be considered as a bad design of %(if) and %(else) in ref-filter?