Re: [PATCH 1/2] sparse-checkout: custom tab completion tests

From: Lessley Dennington <lessleydennington@gmail.com>
Date: Dec 30, 2021, 16:19 UTC
Message-ID: <e6728ed1-52c6-05eb-e816-a9d919d3e9d1@gmail.com>
In-Reply-To: <cd190111-1930-9538-a844-4d9aa08d98b0@gmail.com>

On 12/30/21 7:43 AM, Derrick Stolee wrote:

Show 21 quoted lines

>> +test_expect_failure 'sparse-checkout completes subcommand options' '
>> +	test_completion "git sparse-checkout init --" <<-\EOF &&
>> +	--cone Z
>> +	--sparse-index Z
>> +	--no-sparse-index Z
>> +	EOF
>> +
>> +	test_completion "git sparse-checkout set --" <<-\EOF &&
>> +	--stdin Z
> 
> In en/sparse-checkout-set, the 'set' subcommand learns the options
> for init, including --cone, --sparse-index, and --no-sparse-index.
> I think technically, --no-cone is in there, too.
> 
> Further, the 'reapply' subcommand learns these options in that
> series and I see you do not include that subcommand in this test.
> 
> You might want to rebase onto en/sparse-checkout-set and adjust
> these tests for the new options (plus change the next patch that
> implements the completion).
>

Ah great point - I was going off of the sparse-checkout docs and forgot about Elijah's changes. I will do the rebase and make your suggested corrections in v2.

Show 6 quoted lines

>> +	git -C sparse-checkout config index.sparse false &&
> 
> I'm guessing that the implementation actually requires this, but
> I'll take a look in the next patch. It might just be slow to expand
> the full list of directories in the sparse index case.
>

I'll plan to remove in v2 per your comments in [1].>> +	# test tab completion

Show 20 quoted lines

>> +	(
>> +		cd sparse-checkout &&
>> +		test_completion "git sparse-checkout set f" <<-\EOF
>> +		folder1 Z
>> +		folder1/0 Z
>> +		folder1/0/1 Z
>> +		folder2 Z
>> +		folder2/0 Z
>> +		folder3 Z
> 
> This tab-completion doing a full directory walk seems like it could
> be expensive for a large repository, but I suppose it is the only way
> to allow the following sequence:
> 
> 	fol<tab> -> folder
> 	folder1/<tab> -> folder1/0
> 	folder1/0/<tab> -> folder1/0/1
> 
> (Hopefully that makes sense.)
>

Yes, it does.

> It would be more efficient to only go one level deep at a time, but
> that might not be possible with the tab completion mechanisms.
>

When you say one level deep, do you mean that from the sparse-checkout
directory, tab completion would only show the following?
	
	folder1
	folder2
	folder3

-Lessley

[1]: https://lore.kernel.org/git/c79ccf4a-4da9-1c60-32eb-124d3fa94400@gmail.com/