diff mbox series

[v4,1/1] Documentation/git-sparse-checkout.txt: add an OPTIONS section

Message ID 20220317123718.480093-2-shaoxuan.yuan02@gmail.com (mailing list archive)
State Superseded
Headers show
Series Documentation/git-sparse-checkout.txt: add an OPTIONS section | expand

Commit Message

Shaoxuan Yuan March 17, 2022, 12:37 p.m. UTC
Add an OPTIONS section to the manual and move the descriptions about
these options from COMMANDS to the section.

Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
---
 Documentation/git-sparse-checkout.txt | 44 +++++++++------------------
 1 file changed, 15 insertions(+), 29 deletions(-)

Comments

Junio C Hamano March 18, 2022, 4:47 p.m. UTC | #1
Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> writes:

> Add an OPTIONS section to the manual and move the descriptions about
> these options from COMMANDS to the section.

The above description does not seem to match what the patch does at
all, though.  Sent a wrong patch, possibly just the tip of a series,
when you needed to send more than one patches?  For example, the
header for the hunk -103,33 tells us that the file already has OPTIONS
section before this patch gets applied.

> @@ -48,6 +48,14 @@ COMMANDS
>  	following the 'set' subcommand, and update the working directory to
>  	match.
>  +
> +By default, the arguments to the `set` command are interpreted as a
> +list of directories. The sparse-checkout patterns are set to match
> +all files within those directories, recursively, as well as any file
> +directly contained in a parent of those directories. See INTERNALS
> +-- CONE PATTERN SET below for full details. If --no-cone is specified,
> +then the arguments are interpreted as sparse-checkout patterns. See
> +INTERNALS -- FULL PATTERN SET below for more information.
> ++
>  To ensure that adjusting the sparse-checkout settings within a worktree

That reads well.

> @@ -59,8 +67,10 @@ file. See linkgit:git-worktree[1] and the documentation of
>  'add'::
>  	Update the sparse-checkout file to include additional directories
>  	(in cone mode) or patterns (in non-cone mode).  By default, these
> -	directories or patterns are read from the command-line arguments,
> -	but they can be read from stdin using the `--stdin` option.
> +	directories or patterns are read from the command-line arguments.
> +  These directories or patterns are interpreted the same way as stated
> +  above in `set` command, and they can be read from stdin using the
> +  `--stdin` option.

The original removed by the patch said "directories or patterns",
and I understand that this change is an attempt to say that the
command chooses between directories and patterns using the same
criteria as the "set" command, but the added "are interpreted the
same way as ..." in the middle interrupts the flow of thought the
sentence conveys.  To me, it looks like the first sentence gives
clear enough explanation of that already.

Broken indentation aside, I do not see why this change is needed.

> @@ -103,33 +113,9 @@ OPTIONS
>  	Use with the `set` and `reapply` commands.
>  	Specify using cone mode or not. The default is to use cone mode.
>  +
> -For `set` command:
> ...
> -flags, with the same meaning as the flags from the `set` command, in order
> -to change which sparsity mode you are using without needing to also respecify
> -all sparsity paths.
> +For the `set` command, the option to use cone mode or not changes
> +the interpretation of the remaining arguments to either be a list
> +of directories or a list of patterns.

These three lines are not technically incorrect, but is not written
in a way that helps readers.

Read these three lines a few times, pretending that you have never
used the sparse-checkout, and try to answer this question: "I am
giving a few arguments to the command using the cone mode.  Are they
taken as directories, or patterns?"

It is hard for me to guess what is being improved upon because I do
not have the preimage of this hunk, but the new text is much less
clear than "directories (in cone mode) or patterns (in non-cone
mode)" we saw earlier in the description for the 'add' command,
which would help us answer the question (answer: they are taken as
directories).

Thanks.
diff mbox series

Patch

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index b8f3b89b74..0178d63f56 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -48,6 +48,14 @@  COMMANDS
 	following the 'set' subcommand, and update the working directory to
 	match.
 +
+By default, the arguments to the `set` command are interpreted as a
+list of directories. The sparse-checkout patterns are set to match
+all files within those directories, recursively, as well as any file
+directly contained in a parent of those directories. See INTERNALS
+-- CONE PATTERN SET below for full details. If --no-cone is specified,
+then the arguments are interpreted as sparse-checkout patterns. See
+INTERNALS -- FULL PATTERN SET below for more information.
++
 To ensure that adjusting the sparse-checkout settings within a worktree
 does not alter the sparse-checkout settings in other worktrees, the 'set'
 subcommand will upgrade your repository config to use worktree-specific
@@ -59,8 +67,10 @@  file. See linkgit:git-worktree[1] and the documentation of
 'add'::
 	Update the sparse-checkout file to include additional directories
 	(in cone mode) or patterns (in non-cone mode).  By default, these
-	directories or patterns are read from the command-line arguments,
-	but they can be read from stdin using the `--stdin` option.
+	directories or patterns are read from the command-line arguments.
+  These directories or patterns are interpreted the same way as stated
+  above in `set` command, and they can be read from stdin using the
+  `--stdin` option.
 
 'reapply'::
 	Reapply the sparsity pattern rules to paths in the working tree.
@@ -103,33 +113,9 @@  OPTIONS
 	Use with the `set` and `reapply` commands.
 	Specify using cone mode or not. The default is to use cone mode.
 +
-For `set` command:
-+
-By default, the input list is considered a list of directories, matching
-the output of `git ls-tree -d --name-only`.  This includes interpreting
-pathnames that begin with a double quote (") as C-style quoted strings.
-Note that all files under the specified directories (at any depth) will
-be included in the sparse checkout, as well as files that are siblings
-of either the given directory or any of its ancestors (see 'CONE PATTERN
-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, the input list is considered a list of
-patterns.  This mode is harder to use, and unless you can keep the
-number of patterns small, its design also scales poorly.  It used to be
-the default mode, but we do not recommend using it.  It does not work
-with the `--sparse-index` option, and will likely be incompatible with
-other new features as they are added.  See the "Non-cone Problems"
-section below and the "Sparse Checkout" section of
-linkgit:git-read-tree[1] for more details.
-+
-For `reapply` command:
-+
-The `reapply` command can also take `--[no-]cone` and `--[no-]sparse-index`
-flags, with the same meaning as the flags from the `set` command, in order
-to change which sparsity mode you are using without needing to also respecify
-all sparsity paths.
+For the `set` command, the option to use cone mode or not changes
+the interpretation of the remaining arguments to either be a list
+of directories or a list of patterns.
 
 '--[no-]sparse-index'::
 	Use with the `set` and `reapply` commands.