Re: [PATCH v4 2/3] completion: sparse-checkout updates
- From
Lessley Dennington <lessleydennington@gmail.com>
- Date
- Jan 31, 2022, 20:03 UTC
- Message-ID
- <82bb12d0-0505-1997-21ba-b9863bfdf2c8@gmail.com>
- In-Reply-To
- <CABPp-BFG6fD3hDTrG_HZP9qv0qYaHBFxV3idik6YHE53cnFt6Q@mail.gmail.com>
On 1/27/22 5:21 PM, Elijah Newren wrote:
Show 73 quoted lines
> 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/