Re: [PATCH v3 05/17] sparse-checkout: add '--stdin' option to set subcommand

[Derrick Stolee <stolee@gmail.com>]

On 10/11/2019 6:27 PM, Elijah Newren wrote:
> On Mon, Oct 7, 2019 at 1:08 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> The 'git sparse-checkout set' subcommand takes a list of patterns
>> and places them in the sparse-checkout file. Then, it updates the
>> working directory to match those patterns. For a large list of
>> patterns, the command-line call can get very cumbersome.
>>
>> Add a '--stdin' option to instead read patterns over standard in.
>>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
>>  builtin/sparse-checkout.c          | 40 ++++++++++++++++++++++++++++--
>>  t/t1091-sparse-checkout-builtin.sh | 27 ++++++++++++++++++++
>>  2 files changed, 65 insertions(+), 2 deletions(-)
>>
>> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
>> index 52d4f832f3..68f3d8433e 100644
>> --- a/builtin/sparse-checkout.c
>> +++ b/builtin/sparse-checkout.c
>> @@ -145,6 +145,11 @@ static int write_patterns_and_update(struct pattern_list *pl)
>>         char *sparse_filename;
>>         FILE *fp;
>>
>> +       if (!core_apply_sparse_checkout) {
>> +               warning(_("core.sparseCheckout is disabled, so changes to the sparse-checkout file will have no effect"));
>> +               warning(_("run 'git sparse-checkout init' to enable the sparse-checkout feature"));
>> +       }
>> +
>>         sparse_filename = get_sparse_checkout_filename();
>>         fp = fopen(sparse_filename, "w");
>>         write_patterns_to_file(fp, pl);
>> @@ -154,16 +159,47 @@ static int write_patterns_and_update(struct pattern_list *pl)
>>         return update_working_directory();
>>  }
>>
>> +static char const * const builtin_sparse_checkout_set_usage[] = {
>> +       N_("git sparse-checkout set [--stdin|<patterns>]"),
>> +       NULL
>> +};
>> +
>> +static struct sparse_checkout_set_opts {
>> +       int use_stdin;
>> +} set_opts;
>> +
>>  static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>>  {
>>         static const char *empty_base = "";
>>         int i;
>>         struct pattern_list pl;
>>         int result;
>> +
>> +       static struct option builtin_sparse_checkout_set_options[] = {
>> +               OPT_BOOL(0, "stdin", &set_opts.use_stdin,
>> +                        N_("read patterns from standard in")),
>> +               OPT_END(),
>> +       };
>> +
>>         memset(&pl, 0, sizeof(pl));
>>
>> -       for (i = 1; i < argc; i++)
>> -               add_pattern(argv[i], empty_base, 0, &pl, 0);
>> +       argc = parse_options(argc, argv, prefix,
>> +                            builtin_sparse_checkout_set_options,
>> +                            builtin_sparse_checkout_set_usage,
>> +                            PARSE_OPT_KEEP_UNKNOWN);
> 
> Does this mean users can also spell it 'git sparse-checkout --stdin
> set', instead of the expected 'git sparse-checkout set --stdin'?

No, because the parse_options() inside cmd_sparse_checkout() parses until
it doesn't recognize an option. ('stdin' in your example). After we "consume"
the subcommand "set", we call this method and the parse_options() can then
read the '--stdin'.

Here is the output from my local command of 'git sparse-checkout --stdin set'
at this commit:

	$ ./git sparse-checkout --stdin set
	error: unknown option `stdin'
	usage: git sparse-checkout [init|list|set] <options>

Thanks,
-Stolee