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

[Junio C Hamano <gitster@pobox.com>]

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.