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

From: Derrick Stolee <derrickstolee@github.com>
Date: Mar 14, 2022, 16:13 UTC
Message-ID: <307ac60d-b0a1-ea90-8118-a4e02b809102@github.com>
In-Reply-To: <20220314065659.82029-2-shaoxuan.yuan02@gmail.com>

On 3/14/2022 2:56 AM, Shaoxuan Yuan wrote:

Show 5 quoted lines

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

Thank you for reorganizing the COMMANDS and OPTIONS. I still think there is some improvement to be made here.

>  the 'set' subcommand are stored in the worktree-specific sparse-checkout
>  file. See linkgit:git-worktree[1] and the documentation of
>  `extensions.worktreeConfig` in linkgit:git-config[1] for more details.

Just to provide some extra context to the review, here is the content of the 'set' command up to this point:

'set'::
	Enable the necessary sparse-checkout config settings
	(`core.sparseCheckout`, `core.sparseCheckoutCone`, and
	`index.sparse`) if they are not already set to the desired values,
	populate the sparse-checkout file from the list of arguments
	following the 'set' subcommand, and update the working directory to
	match.
+
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
config if not already present. The sparsity defined by the arguments to
the 'set' subcommand are stored in the worktree-specific sparse-checkout
file. See linkgit:git-worktree[1] and the documentation of
`extensions.worktreeConfig` in linkgit:git-config[1] for more details.

So this mentions that we will "write a set of patterns to the sparse-checkout file from the list of arguments" but with the deletions below we lose understanding of how the arguments match with the patterns.

I think it would be good to insert a paragraph between the two above paragraphs that briefly touches on the input. Something like:

  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.

We might need to refer to the `set` command input when talking about the `add` command.

Show 46 quoted lines

>  'add'::
>  	Update the sparse-checkout file to include additional directories
> @@ -109,11 +71,6 @@ interact with your repository until it is disabled.
>  	cases, it can make sense to run `git sparse-checkout reapply` later
>  	after cleaning up affected paths (e.g. resolving conflicts, undoing
>  	or committing changes, etc.).
> -+
> -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.
>  
>  'disable'::
>  	Disable the `core.sparseCheckout` config setting, and restore the
> @@ -139,6 +96,69 @@ paths to pass to a subsequent 'set' or 'add' command.  However,
>  the disable command, so the easy restore of calling a plain `init`
>  decreased in utility.
>  
> +
> +OPTIONS
> +-------
> +'--[no-]cone'::
> +	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.
> ++

With the recommended change above, this pair of paragraphs can be condensed. Something like...

  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.

Show 6 quoted lines

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

I'm not sure that this mention of `reapply` is necessary, as those options document themselves further down.

Show 27 quoted lines

> +
> +'--[no-]sparse-index'::
> +	Use with the `set` and `reapply` commands.
> +	Specify using a sparse index or not. The default is to not use a
> +	sparse index.
> ++
> +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
> +index to be more closely aligned with your sparse-checkout
> +definition. This can have significant performance advantages for
> +commands such as `git status` or `git add`.  This feature is still
> +experimental. Some commands might be slower with a sparse index until
> +they are properly integrated with the feature.
> ++
> +**WARNING:** Using a sparse index requires modifying the index in a way
> +that is not completely understood by external tools. If you have trouble
> +with this compatibility, then run `git sparse-checkout init --no-sparse-index`
> +to rewrite your index to not be sparse. Older versions of Git will not
> +understand the sparse directory entries index extension and may fail to
> +interact with your repository until it is disabled.
> +
> +'--stdin'::
> +	Use with the `set` and `add` commands.
> ++
> +When the `--stdin` option is provided, the directories or patterns are
> +read from standard in as a newline-delimited list instead of from the
> +arguments.

These options are excellent.

Thanks, -Stolee