diff mbox series

[v2,4/9] git-sparse-checkout.txt: update docs for deprecation of 'init'

Message ID eb3b318b39ec3eed754eab7d2c9d20198117545b.1647054681.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series sparse-checkout: make cone mode the default | expand

Commit Message

Elijah Newren March 12, 2022, 3:11 a.m. UTC
From: Elijah Newren <newren@gmail.com>

The 'init' subcommand of sparse-checkout was deprecated in ba2f3f58ac
("git-sparse-checkout.txt: update to document init/set/reapply changes",
2021-12-14), but a couple places in the manual still assumed it was the
primary way to use sparse-checkout.  Correct them.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-sparse-checkout.txt | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Junio C Hamano March 14, 2022, 8:53 p.m. UTC | #1
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> 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.

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

> and the "Pattern Set" sections below for more
> +details.

Are we referring to "Internals - cone/full pattern set" sections?

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

	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.

Thanks.
Elijah Newren April 22, 2022, 2:29 a.m. UTC | #2
On Mon, Mar 14, 2022 at 1:53 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> 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?
Junio C Hamano April 22, 2022, 6:09 a.m. UTC | #3
Elijah Newren <newren@gmail.com> writes:

>> 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?

s/core/cone mode/g, I guess (because it is so long ago I do not
offhand recall what I had in mind exactly back when I wrote it).
diff mbox series

Patch

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 42a984306bb..b9e44e3dd4c 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -70,11 +70,11 @@  SET' below for more details).  In the past, this was not the default,
 and `--cone` needed to be specified or `core.sparseCheckoutCone` needed
 to be enabled.
 +
-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
+not recommended.  See the "Sparse Checkout" section of
+linkgit:git-read-tree[1] and the "Pattern Set" sections below for more
+details.
 +
 Use the `--[no-]sparse-index` option to use a sparse index (the
 default is to not use it).  A sparse index reduces the size of the
@@ -196,9 +196,9 @@  In addition to the above two patterns, we also expect that all files in the
 root directory are included. If a recursive pattern is added, then all
 leading directories are added as parent patterns.
 
-By default, when running `git sparse-checkout init`, the root directory is
-added as a parent pattern. At this point, the sparse-checkout file contains
-the following patterns:
+By default, when running `git sparse-checkout set` with no directories
+specified, the root directory is added as a parent pattern. At this
+point, the sparse-checkout file contains the following patterns:
 
 ----------------
 /*