Re: [PATCH v4 2/3] completion: sparse-checkout updates

[Lessley Dennington <lessleydennington@gmail.com>]

On 1/27/22 5:21 PM, Elijah Newren wrote:
> On Thu, Jan 27, 2022 at 1:21 PM Lessley Dennington via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Lessley Dennington <lessleydennington@gmail.com>
>>
>> Fix custom tab completion for the sparse-checkout command. This will
>> ensure:
>>
>> 1. The full list of subcommands is provided when users enter git
>> sparse-checkout <TAB>.
>> 2. Subcommand options are tab-completable.
>> 3. A list of directories (but not files) is provided when users enter git
>> sparse-checkout add <TAB> or git sparse-checkout set <TAB> in cone mode
>> only. In non-cone mode, these commands will continue to complete both
>> directory and file names.
>>
>> 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 | 15 ++++++++-------
>>   t/t9902-completion.sh                  |  4 ++--
>>   2 files changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index c82ccaebcc7..a920cdab201 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2988,21 +2988,22 @@ _git_show_branch ()
>>
>>   _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
>>          fi
>>
>>          case "$subcommand,$cur" in
>> -       init,--*)
>> -               __gitcomp "--cone"
>> -               ;;
>> -       set,--*)
>> -               __gitcomp "--stdin"
>> +               *,--*)
>> +                       __gitcomp_builtin sparse-checkout_$subcommand "" "--"
>>                  ;;
>> -       *)
>> +               set,*|add,*)
>> +                       if [ $(__git config core.sparseCheckoutCone) ]; then
>> +                               __git_complete_index_file "--directory"
>> +                       fi
> 
> --directory only affects how untracked files are presented in `git
> ls-files` (which _git_complete_index_file uses), and you're not asking
> for untracked paths at all here.  So you've got a useless flag.
> 
> Testing this commit out, I can verify that it completes on all tracked
> paths in the index.  Perhaps that was an intended intermediate step,
> but if so the commit message needs to be changed since it claims this
> commit is doing something else.
> 
> There's also a problem in that `git sparse-checkout --cone path<TAB>`
> is not using this codepath.  If the user specifies --cone, then we
> should give them cone-mode-style completion the same as if they
> already have the config setting set.  Or, we at least need to mention
> in the commit message that we don't yet correctly check for the
> `--cone` option to get the right style of completion and are leaving
> it for future work.

> 
Thank you for your feedback, it is very helpful. As I've been thinking
about this, it feels like this commit should perhaps be dropped in favor
of the __gitcomp_directories commit (with some modifications per [1]). I
know you initially said to add the __gitcomp_directories commit in
addition to what I had, though, so I wanted to confirm whether removing
this is the right course of action.

[1]: 
https://lore.kernel.org/git/CABPp-BEq9pTqsy_R_SR1DSgUK58ubNR1Gk4G1RoL8wkadyo6zw@mail.gmail.com/