Re: [PATCH v2 2/9] sparse-checkout: make --cone the default
Sorry for the long delay...
On Mon, Mar 14, 2022 at 1:34 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 58 quoted lines
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > The subset of files is chosen by providing a list of directories in
> > -cone mode (which is recommended), or by providing a list of patterns
> > -in non-cone mode.
> > +cone mode (the default), or by providing a list of patterns in
> > +non-cone mode.
>
> OK.
>
> > @@ -60,7 +60,7 @@ 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.
> > +
> > -When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
> > +When `--cone` is passed or `core.sparseCheckoutCone` is not false, the
> > input list is considered a list of directories. This allows for
>
>
> I expected a change to Documentation/config/core.txt in this step,
> because the default value for core.sparseCheckoutCone becomes true
> if unspecified with this step, and the normal expectation by the
> readers is what is not explicitly set to true is either 'false' or
> 'unspecified' (when 'unspecified' has its own meaning, like in
> gitattributes).
>
> Something like this?
>
> diff --git i/Documentation/config/core.txt w/Documentation/config/core.txt
> index c04f62a54a..03cf075821 100644
> --- i/Documentation/config/core.txt
> +++ w/Documentation/config/core.txt
> @@ -615,8 +615,10 @@ core.sparseCheckout::
>
> core.sparseCheckoutCone::
> Enables the "cone mode" of the sparse checkout feature. When the
> - sparse-checkout file contains a limited set of patterns, then this
> - mode provides significant performance advantages. See
> + sparse-checkout file contains a limited set of patterns, this
> + mode provides significant performance advantages. The "non
> + cone mode" can be requested to allow specifying a more flexible
> + patterns by setting this variable to 'false'. See
> linkgit:git-sparse-checkout[1] for more information.
>
> core.abbrev::
>
> > -When `--no-cone` is passed or `core.sparseCheckoutCone` is not enabled,
> > +When `--no-cone` is passed or `core.sparseCheckoutCone` is false,
>
> "is set to 'false'" would make it clearer.
>
> > +If `core.sparseCheckoutCone=true` (set by default or with an explicit
> > +`--cone`), then Git will parse the sparse-checkout file expecting
>
> "Unless `core.sparseCheckoutCone` is explicitly set to `false`"
> might be clearer, but I am not sure. It is just that I found the
> phrase "set by default" confusing.
Thanks; I applied all three suggestions.
Show 7 quoted lines
> > /* Set cone/non-cone mode appropriately */
> > core_apply_sparse_checkout = 1;
> > - if (*cone_mode == 1) {
> > + if (*cone_mode) { /* also handles "not specified" (value of -1) */
>
> The comment is correct, but if we can make the code not to require
> such a comment, that would be even better.I had that in v1; I'll revert back to it.