git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Cc: bagasdotme@gmail.com, git@vger.kernel.org, newren@gmail.com,
	vdye@github.com
Subject: Re: [PATCH v3 1/1] Documentation/git-sparse-checkout.txt: add an OPTIONS section
Date: Mon, 14 Mar 2022 12:13:11 -0400	[thread overview]
Message-ID: <307ac60d-b0a1-ea90-8118-a4e02b809102@github.com> (raw)
In-Reply-To: <20220314065659.82029-2-shaoxuan.yuan02@gmail.com>

On 3/14/2022 2:56 AM, Shaoxuan Yuan wrote:
> 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.

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

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

> +
> +'--[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


  reply	other threads:[~2022-03-14 16:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-11 13:21 [RFC PATCH 0/1] Documentation/git-sparse-checkout.txt: add an OPTIONS section Shaoxuan Yuan
2022-03-11 13:21 ` [RFC PATCH 1/1] " Shaoxuan Yuan
2022-03-11 20:56   ` Derrick Stolee
2022-03-14  6:34 ` [PATCH v2 0/1] " Shaoxuan Yuan
2022-03-14  6:34   ` [PATCH v2 1/1] " Shaoxuan Yuan
2022-03-14  6:56 ` [PATCH v3 0/1] " Shaoxuan Yuan
2022-03-14  6:56   ` [PATCH v3 1/1] " Shaoxuan Yuan
2022-03-14 16:13     ` Derrick Stolee [this message]
2022-03-17 12:37 ` [PATCH v4 0/1] " Shaoxuan Yuan
2022-03-17 12:37   ` [PATCH v4 1/1] " Shaoxuan Yuan
2022-03-18 16:47     ` Junio C Hamano
2022-03-18 16:30   ` [PATCH v4 0/1] " Junio C Hamano
2022-03-19  6:19 ` [PATCH v5 0/4] " Shaoxuan Yuan
2022-03-19  6:19   ` [PATCH v5 1/4] " Shaoxuan Yuan
2022-03-19  6:19   ` [PATCH v5 2/4] Documentation/git-sparse-checkout.txt: move OPTIONS after COMMANDS Shaoxuan Yuan
2022-03-19  6:19   ` [PATCH v5 3/4] Documentation/git-sparse-checkout.txt: some reword and modifications Shaoxuan Yuan
2022-03-19  6:19   ` [PATCH v5 4/4] " Shaoxuan Yuan
2022-03-22 15:05   ` [PATCH v5 0/4] Documentation/git-sparse-checkout.txt: add an OPTIONS section Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=307ac60d-b0a1-ea90-8118-a4e02b809102@github.com \
    --to=derrickstolee@github.com \
    --cc=bagasdotme@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=shaoxuan.yuan02@gmail.com \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).