Subject: Re: [PATCH v5 04/17] sparse-checkout: 'set' subcommand

From: Derrick Stolee <[email-masked]>
Date: Thu, 21 Nov 2019 08:04:35 -0500

On 11/19/2019 10:46 AM, SZEDER Gábor wrote:
> On Mon, Oct 21, 2019 at 01:56:13PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <[email-masked]>
>>
>> The 'git sparse-checkout set' subcommand takes a list of patterns
>> as arguments and writes them to the sparse-checkout file. Then, it
>> updates the working directory using 'git read-tree -mu HEAD'.
>>
>> The 'set' subcommand will replace the entire contents of the
>> sparse-checkout file. The write_patterns_and_update() method is
>> extracted from cmd_sparse_checkout() to make it easier to implement
>> 'add' and/or 'remove' subcommands in the future.
> 
> Yeah, an 'add' subcommand would be great, because 'set' throwing away
> all the existing patterns can lead to some frustration:

I plan to extend this feature when this series lands.

> Having said that, being forced to use 'git sparse-checkout set' and
> specify all patterns at once does have its own benefits: I needed like
> 6 subdirectories to build that subset of a big project that I was
> interested in, and now all the necessary patterns are saved in my Bash
> history, and I will be able to easily dig out the complete command in
> the future.  That wouldn't work with using the 'add' subcommand to add
> one pattern at a time.

In general, this "set" command is created first because tools can interact
with it more easily than "add" and "remove". Users would probably prefer the
"add" and "remove" way to interact.
 
>> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
>> index cb74715ca6..bf2dc55bb1 100755
>> --- a/t/t1091-sparse-checkout-builtin.sh
>> +++ b/t/t1091-sparse-checkout-builtin.sh
>> @@ -96,4 +96,36 @@ test_expect_success 'clone --sparse' '
>>  	test_cmp expect dir
>>  '
>>  
>> +test_expect_success 'set enables config' '
>> +	git init empty-config &&
>> +	(
>> +		cd empty-config &&
>> +		test_commit test file &&
>> +		test_path_is_missing .git/config.worktree &&
>> +		test_must_fail git sparse-checkout set nothing &&
> 
> This command prints the same error message twice:
> 
>   + test_must_fail git sparse-checkout set nothing
>   error: Sparse checkout leaves no entry on working directory
>   error: Sparse checkout leaves no entry on working directory

At this commit, I do not see two identical lines, but instead the second
line says

	error: failed to update index with new sparse-checkout paths

(that "paths" should probably be "patterns")

Thanks,
-Stolee