[2/2] sparse-checkout: custom tab completion

Commit Message

Lessley Dennington Dec. 30, 2021, 12:32 a.m. UTC

From: Lessley Dennington <[e-mail]>

Fix custom tab completion for sparse-checkout command. This will ensure:

1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.

Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.

Signed-off-by: Lessley Dennington <[e-mail]>
---
 contrib/completion/git-completion.bash | 34 +++++++++++++++++---------
 t/t9902-completion.sh                  |  8 +++---
 2 files changed, 26 insertions(+), 16 deletions(-)

Comments

Derrick Stolee Dec. 30, 2021, 1:50 p.m. UTC | #1

On 12/29/2021 7:32 PM, Lessley Dennington via GitGitGadget wrote:
> From: Lessley Dennington <[e-mail]>
> 
> Fix custom tab completion for sparse-checkout command. This will ensure:
> 
> 1. The full list of subcommands is provided when users enter git
> sparse-checkout <TAB>.
> 2. The --help option is tab-completable.
> 3. Subcommand options are tab-completable.
> 4. A list of directories (but not files) is provided when users enter git
> sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
> 
> Failing tests that were added in the previous commit to verify these
> scenarios are now passing with these updates.
> 
> Signed-off-by: Lessley Dennington <[e-mail]>
> ---
>  contrib/completion/git-completion.bash | 34 +++++++++++++++++---------
>  t/t9902-completion.sh                  |  8 +++---
>  2 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 377d6c5494a..b8f1caece83 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2986,24 +2986,34 @@ _git_show_branch ()
>  	__git_complete_revlist
>  }
>  
> +__git_sparse_checkout_init_opts="--cone --sparse-index --no-sparse-index"
> +
>  _git_sparse_checkout ()
>  {
> -	local subcommands="list init set disable"
> +	local subcommands="list init set disable add reapply"
>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
> +
>  	if [ -z "$subcommand" ]; then
> -		__gitcomp "$subcommands"
> -		return
> +		case "$cur" in
> +			--*)
> +				__gitcomp "--help"
> +				;;
> +			*)
> +				__gitcomp "$subcommands"
> +				;;
> +		esac

This part fixes the --<tab> completion. I suppose if someone
did "-<tab>" then nothing would show up?

>  	fi
>  
> -	case "$subcommand,$cur" in
> -	init,--*)
> -		__gitcomp "--cone"
> -		;;
> -	set,--*)
> -		__gitcomp "--stdin"
> -		;;
> -	*)
> -		;;
> +	case "$prev" in
> +		init)
> +			__gitcomp "$__git_sparse_checkout_init_opts"
> +			;;
> +		add|set)
> +			__gitcomp "--stdin"
> +			__gitcomp "$(git ls-tree -d -r HEAD --name-only)"> +			;;

With the thinking of rebasing onto en/sparse-checkout-set, this
could possibly be rearranged so the add|set) cases pass-through
into the init) and reapply) cases (skip the ;; between) to save
some duplication. Or, it is possible that doesn't work, but it
might be worth a try.

Also, since you are using 'git ls-tree' and not 'git ls-files',
the sparse index will not have an effect on the performance.
There will be some corner cases where a directory exists in one
of HEAD or the index but not the other. That's probably still
the right way to go since 'git ls-files' doesn't have a way to
only list directories. It just means that you probably don't
need to explicitly disable the sparse index in your test.

Thanks,
-Stolee

Lessley Dennington Dec. 30, 2021, 4:24 p.m. UTC | #2

On 12/30/21 7:50 AM, Derrick Stolee wrote:
>> +__git_sparse_checkout_init_opts="--cone --sparse-index --no-sparse-index"
>> +
>>   _git_sparse_checkout ()
>>   {
>> -	local subcommands="list init set disable"
>> +	local subcommands="list init set disable add reapply"
>>   	local subcommand="$(__git_find_on_cmdline "$subcommands")"
>> +
>>   	if [ -z "$subcommand" ]; then
>> -		__gitcomp "$subcommands"
>> -		return
>> +		case "$cur" in
>> +			--*)
>> +				__gitcomp "--help"
>> +				;;
>> +			*)
>> +				__gitcomp "$subcommands"
>> +				;;
>> +		esac
> 
> This part fixes the --<tab> completion. I suppose if someone
> did "-<tab>" then nothing would show up?
> 
It actually shows a list of files that contain -- (if any exist).
>>   	fi
>>   
>> -	case "$subcommand,$cur" in
>> -	init,--*)
>> -		__gitcomp "--cone"
>> -		;;
>> -	set,--*)
>> -		__gitcomp "--stdin"
>> -		;;
>> -	*)
>> -		;;
>> +	case "$prev" in
>> +		init)
>> +			__gitcomp "$__git_sparse_checkout_init_opts"
>> +			;;
>> +		add|set)
>> +			__gitcomp "--stdin"
>> +			__gitcomp "$(git ls-tree -d -r HEAD --name-only)"> +			;;
> 
> With the thinking of rebasing onto en/sparse-checkout-set, this
> could possibly be rearranged so the add|set) cases pass-through
> into the init) and reapply) cases (skip the ;; between) to save
> some duplication. Or, it is possible that doesn't work, but it
> might be worth a try.
> 
Thanks, I'll give this a go!
> Also, since you are using 'git ls-tree' and not 'git ls-files',
> the sparse index will not have an effect on the performance.
> There will be some corner cases where a directory exists in one
> of HEAD or the index but not the other. That's probably still
> the right way to go since 'git ls-files' doesn't have a way to
> only list directories. It just means that you probably don't
> need to explicitly disable the sparse index in your test.
> 
Will correct in v2.

-Lessley

Patch

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 377d6c5494a..b8f1caece83 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2986,24 +2986,34 @@  _git_show_branch ()
 	__git_complete_revlist
 }
 
+__git_sparse_checkout_init_opts="--cone --sparse-index --no-sparse-index"
+
 _git_sparse_checkout ()
 {
-	local subcommands="list init set disable"
+	local subcommands="list init set disable add reapply"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
 	if [ -z "$subcommand" ]; then
-		__gitcomp "$subcommands"
-		return
+		case "$cur" in
+			--*)
+				__gitcomp "--help"
+				;;
+			*)
+				__gitcomp "$subcommands"
+				;;
+		esac
 	fi
 
-	case "$subcommand,$cur" in
-	init,--*)
-		__gitcomp "--cone"
-		;;
-	set,--*)
-		__gitcomp "--stdin"
-		;;
-	*)
-		;;
+	case "$prev" in
+		init)
+			__gitcomp "$__git_sparse_checkout_init_opts"
+			;;
+		add|set)
+			__gitcomp "--stdin"
+			__gitcomp "$(git ls-tree -d -r HEAD --name-only)"
+			;;
+		*)
+			;;
 	esac
 }
 
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 22271ac2f3b..9f6eb06fbab 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1444,7 +1444,7 @@  test_expect_success 'git checkout - with --detach, complete only references' '
 	EOF
 '
 
-test_expect_failure 'sparse-checkout completes subcommands' '
+test_expect_success 'sparse-checkout completes subcommands' '
 	test_completion "git sparse-checkout " <<-\EOF
 	list Z
 	init Z
@@ -1455,13 +1455,13 @@  test_expect_failure 'sparse-checkout completes subcommands' '
 	EOF
 '
 
-test_expect_failure 'sparse-checkout completes options' '
+test_expect_success 'sparse-checkout completes options' '
 	test_completion "git sparse-checkout --" <<-\EOF
 	--help Z
 	EOF
 '
 
-test_expect_failure 'sparse-checkout completes subcommand options' '
+test_expect_success 'sparse-checkout completes subcommand options' '
 	test_completion "git sparse-checkout init --" <<-\EOF &&
 	--cone Z
 	--sparse-index Z
@@ -1477,7 +1477,7 @@  test_expect_failure 'sparse-checkout completes subcommand options' '
 	EOF
 '
 
-test_expect_failure 'sparse-checkout completes directory names' '
+test_expect_success 'sparse-checkout completes directory names' '
 	# set up sparse-checkout repo
 	git init sparse-checkout &&
 	(