diff mbox series

[2/3] sparse-checkout: use default patterns for 'set' only !stdin

Message ID 20231221065925.3234048-3-gitster@pobox.com (mailing list archive)
State Accepted
Commit 53ded839aee86e3544c3e035385fe2ada33180b1
Headers show
Series sparse-checkout with --end-of-options | expand

Commit Message

Junio C Hamano Dec. 21, 2023, 6:59 a.m. UTC
"git sparse-checkout set ---no-cone" uses default patterns when none
is given from the command line, but it should do so ONLY when
--stdin is not being used.  Right now, add_patterns_from_input()
called when reading from the standard input is sloppy and does not
check if there are extra command line parameters that the command
will silently ignore, but that will change soon and not setting this
unnecessary and unused default patterns start to matter when it gets
fixed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This came from f2e3a218 (sparse-checkout: enable `set` to
   initialize sparse-checkout mode, 2021-12-14) by Elijah.

 builtin/sparse-checkout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Elijah Newren Dec. 24, 2023, 7:51 a.m. UTC | #1
On Wed, Dec 20, 2023 at 10:59 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "git sparse-checkout set ---no-cone" uses default patterns when none
> is given from the command line, but it should do so ONLY when
> --stdin is not being used.  Right now, add_patterns_from_input()
> called when reading from the standard input is sloppy and does not
> check if there are extra command line parameters that the command
> will silently ignore, but that will change soon and not setting this
> unnecessary and unused default patterns start to matter when it gets
> fixed.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * This came from f2e3a218 (sparse-checkout: enable `set` to
>    initialize sparse-checkout mode, 2021-12-14) by Elijah.
>
>  builtin/sparse-checkout.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 8f55127202..04ae81fce8 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -837,7 +837,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>          * non-cone mode, if nothing is specified, manually select just the
>          * top-level directory (much as 'init' would do).
>          */
> -       if (!core_sparse_checkout_cone && argc == 0) {
> +       if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
>                 argv = default_patterns;
>                 argc = default_patterns_nr;
>         } else {
> --
> 2.43.0-174-g055bb6e996

Thanks for catching this; the fix looks good to me.
Junio C Hamano Dec. 28, 2023, 12:18 a.m. UTC | #2
Elijah Newren <newren@gmail.com> writes:

>> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
>> index 8f55127202..04ae81fce8 100644
>> --- a/builtin/sparse-checkout.c
>> +++ b/builtin/sparse-checkout.c
>> @@ -837,7 +837,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>>          * non-cone mode, if nothing is specified, manually select just the
>>          * top-level directory (much as 'init' would do).
>>          */
>> -       if (!core_sparse_checkout_cone && argc == 0) {
>> +       if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
>>                 argv = default_patterns;
>>                 argc = default_patterns_nr;
>>         } else {
>> --
>> 2.43.0-174-g055bb6e996
>
> Thanks for catching this; the fix looks good to me.

Actually I am not so sure.

An obvious alternative would be to collect the patterns, either from
the command line or from the standard input, and then use the
default when we collected nothing.

But I guess those who use the standard input should be able to
specify everything fully, so it would probably be OK.
diff mbox series

Patch

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8f55127202..04ae81fce8 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -837,7 +837,7 @@  static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 	 * non-cone mode, if nothing is specified, manually select just the
 	 * top-level directory (much as 'init' would do).
 	 */
-	if (!core_sparse_checkout_cone && argc == 0) {
+	if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
 		argv = default_patterns;
 		argc = default_patterns_nr;
 	} else {