All of lore.kernel.org
 help / color / mirror / 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

* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.