Re: [PATCH v2 2/2] sparse-checkout: custom tab completion
- From
Lessley Dennington <lessleydennington@gmail.com>
- Date
- Jan 4, 2022, 19:41 UTC
- Message-ID
- <676e8408-5ac3-4293-22ee-c43e9bd6916b@gmail.com>
- In-Reply-To
- <CABPp-BG_Jgyr89z_D355Ytzz31J40nBGX=2cr+aXtcf3U1y6Dg@mail.gmail.com>
On 12/31/21 4:52 PM, Elijah Newren wrote:
Show 67 quoted lines
> On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Lessley Dennington <lessleydennington@gmail.com>
>>
>> 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 <lessleydennington@gmail.com>
>> ---
>> contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
>> t/t9902-completion.sh | 8 +++---
>> 2 files changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index c82ccaebcc7..7de997ee64e 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2986,24 +2986,38 @@ _git_show_branch ()
>> __git_complete_revlist
>> }
>>
>> +__git_sparse_checkout_subcommand_opts="--cone --no-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
>
> Shouldn't this be "$subcommand" rather than "$prev"? I think with
> prev, it will only correctly complete the first path after "set",
> "add", etc.
> Good catch, thank you! Fixing in v3.
Show 50 quoted lines
>> + set)
>> + __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
>> + __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
>> + ;;
>> + add)
>> + __gitcomp "--stdin"
>> + __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
>
> I was going to make a simple suggestion for making it just complete
> one additional level at a time and leaving out the -r, and then tried
> it out and found out it wasn't simple. I got something working,
> eventually, but it's slightly ugly...so it probably belongs in a
> separate patch anyway. If you're curious, it's basically replacing
> the second __gitcomp... call for each of set and add with
> `__gitcomp_directories`, after first defining:
>
> __gitcomp_directories ()
> {
> local _tmp_dir _tmp_completions
>
> # Get the directory of the current token; this differs from dirname
> # in that it keeps up to the final trailing slash. If no slash found
> # that's fine too.
> [[ "$cur" =~ .*/ ]]
> _tmp_dir=$BASH_REMATCH
>
> # Find possible directory completions, adding trailing '/' characters
> _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
> sed -e s%$%/%)"
>
> if [[ -n "$_tmp_completions" ]]; then
> # There were some directory completions, so find ones that
> # start with "$cur", the current token, and put those in COMPREPLY
> local i=0 c IFS=$' \t\n'
> for c in $_tmp_completions; do
> if [[ $c == "$cur"* ]]; then
> COMPREPLY+=("$c")
> fi
> done
> elif [[ "$cur" =~ /$ ]]; then
> # No possible further completions any deeper, so assume we're at
> # a leaf directory and just consider it complete
> __gitcomp_direct_append "$cur "
> fi
> }
>
> But I don't think that needs to be part of this series; I can submit
> it later and hopefully get a completion expert to point out
> better/cleaner ways of what I've done above.
> I'm admittedly curious about what made this so difficult. I removed the '-r' and updated my tests to expect only directories at one level, and they passed. But I imagine I'm being too simplistic.
Show 51 quoted lines
>> + ;; >> + init|reapply) >> + __gitcomp "$__git_sparse_checkout_subcommand_opts" >> + ;; >> + *) >> + ;; >> esac >> } >> >> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh >> index 51d0f2d93a1..4dc93ee0595 100755 >> --- a/t/t9902-completion.sh >> +++ b/t/t9902-completion.sh >> @@ -1447,7 +1447,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 >> @@ -1458,13 +1458,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 >> --no-cone Z >> @@ -1492,7 +1492,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 && >> ( >> -- >> gitgitgadget > > Patch looks good to me, other than the $prev vs. $subcommand thing.
Thanks!