On Mon, Mar 14, 2022 at 1:53 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > -When `--no-cone` is passed or `core.sparseCheckoutCone` is false, > > -the input list is considered a list of patterns. This mode is harder > > -to use and less performant, and is thus not recommended. See the > > -"Sparse Checkout" section of linkgit:git-read-tree[1] and the "Pattern > > -Set" sections below for more details. > > +When `--no-cone` is passed, the input list is considered a list of > > +patterns. This mode is harder to use and less performant, and is thus > > "less perfromant" can be quantified, but "harder to use" is probably > harder to defend. Those on a project with need for more flexible > way to specify than "these are the directories I care about" would > not find it convincing. That text wasn't added by this patch (or even by any patch in this series); it was in the pre-image. I can still change it, of course, but I don't think it belongs in this particular patch. How about I replace this text in 8/9 with a link to the new section that was added (the one that explains the problems we see with non-cone mode). > > +not recommended. See the "Sparse Checkout" section of > > +linkgit:git-read-tree[1] > > The referenced section (I am reading "git read-tree --help" from > seen with these patches) may need updating. It shows an example > of including everything except for unwanted, without mentioning > if that is for cone or non-cone. I wouldn't really say that the example is for either mode (the point of the sparse-checkout command is so users don't need to directly edit the $GIT_DIR/info/sparse-checkout file), but the examples might be useful for people trying to understand the patterns used in non-cone mode. Perhaps this change would be helpful?: diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt index 99bb387134..0eae9e01fd 100644 --- a/Documentation/git-read-tree.txt +++ b/Documentation/git-read-tree.txt @@ -378,7 +378,9 @@ SPARSE CHECKOUT Note: The `update-index` and `read-tree` primitives for supporting the skip-worktree bit predated the introduction of linkgit:git-sparse-checkout[1]. Users are encouraged to use -`sparse-checkout` in preference to these low-level primitives. +`sparse-checkout` in preference to these low-level primitives. However, +the information below might be useful to users trying to understand the +pattern style used in non-cone mode of the `sparse-checkout` command. However, I don't think that belongs in this patch (since you were commenting on text that only appeared in the diff due to reflowing paragraphs), but I can add it to 9/9. > > and the "Pattern Set" sections below for more > > +details. > > Are we referring to "Internals - cone/full pattern set" sections? Yeah, as you noted in a later patch, you got confused by looking at the end result rather than the state of the tree as of this patch. The "Internals -" prefix was added in a later patch. > This may be a topic of another step in this series, but the "core" > section starts by mentioning what characteristics the full pattern > set has and uses it to steer readers away from it, which I find it > less than ideal. As we present "core" first (because it is the > default), we should present "core" by itself, without requiring to > know what the other thing is. By '"core" section' do you mean the "cone pattern set" section? That's my best guess after several comparisons. I agree that it's less than ideal, though I thought that perhaps adding the "Internals" prefix would allow folks to just ignore it. But you may be right that I should also overhaul it a bit, maybe splitting it into two sections, one about some implementation details about the mode, and another later section about how the patterns in cone mode are a strictly controlled subset of the possible full pattern set. However, I think that'd go better in patch 7 rather than here. > Perhaps replace the entire first > paragraph so that the section begins more like so: > > The "cone mode", which is the default, lets you specify only > what directories to include and what directories to exclude. There's no provision to specify individual directories to exclude, only which to include. Everything not listed is excluded. > The accepted patterns in the cone pattern set are:... > > Especially, the last sentence in the paragraph to be struck still > lives in the old world in which you needed to opt into the cone > mode. This patch didn't strike any paragraph, so I'm not sure which sentence you're referring to. I tried looking around, and didn't readily find it either. Perhaps my big reshufflings in my new patch 7 addressed what you're talking about, though?
