From: Lessley Dennington Date: Fri, 04 Feb 2022 00:34:29 GMT Subject: Re: [PATCH v5 1/3] completion: address sparse-checkout issues Message-ID: In-Reply-To: On 2/3/22 3:52 PM, Elijah Newren wrote: > On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget > wrote: >> >> From: Lessley Dennington >> >> Correct multiple issues with tab completion of the git sparse-checkout >> command. These issues were: >> >> 1. git sparse-checkout previously resulted in an incomplete list of >> subcommands (it was missing reapply and add). >> 2. Subcommand options were not tab-completable. >> 3. git sparse-checkout set and git sparse-checkout add showed >> both file names and directory names. While this may be a less surprising >> behavior for non-cone mode, cone mode sparse checkouts should complete >> only directory names. >> >> Note that while the new strategy of just using git ls-tree to complete on >> directory names is simple and a step in the right direction, it does have >> some caveats. These are: >> >> 1. Likelihood of poor performance in large monorepos (as a result of >> recursively completing directory names). >> 2. Inability to handle paths containing unusual characters. >> >> These caveats will be fixed by subsequent commits in this series. >> >> Signed-off-by: Lessley Dennington >> --- >> contrib/completion/git-completion.bash | 16 ++--- >> t/t9902-completion.sh | 83 ++++++++++++++++++++++++++ >> 2 files changed, 91 insertions(+), 8 deletions(-) >> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash >> index c82ccaebcc7..2976f63747f 100644 >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -2988,7 +2988,7 @@ _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" >> @@ -2996,14 +2996,14 @@ _git_sparse_checkout () >> fi >> >> case "$subcommand,$cur" in >> - init,--*) >> - __gitcomp "--cone" >> - ;; >> - set,--*) >> - __gitcomp "--stdin" >> - ;; >> - *) >> + *,--*) >> + __gitcomp_builtin sparse-checkout_$subcommand "" "--" >> ;; >> + set,*|add,*) >> + if [ $(__git config core.sparseCheckoutCone) ] || > > Shouldn't this be > > if [ "$(__git config core.sparseCheckoutCone)" == "true" ] || > > otherwise I think you'd trigger when it's manually set to false. > >> + [ "$(__git_find_on_cmdline --cone)" ]; then > > This might read better as > > [ -n "$(__git_find_on_cmdline --cone)" ]; then > > because otherwise you are relying on > > [ "--cone" ] > > to evaluate as true which just seems slightly weird. > Yup, this was causing some problems. Odd that the failure only cropped up when merged with 'seen' but I have a fix prepared and will submit as soon as GGG CI completes. >> + __gitcomp "$(git ls-tree -d -r HEAD --name-only)" >> + fi >> esac >> } >> >> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh >> index 518203fbe07..f6eeb9aa035 100755 >> --- a/t/t9902-completion.sh >> +++ b/t/t9902-completion.sh >> @@ -1447,6 +1447,89 @@ test_expect_success 'git checkout - with --detach, complete only references' ' >> EOF >> ' >> >> +test_expect_success 'setup sparse-checkout tests' ' >> + # set up sparse-checkout repo >> + git init sparse-checkout && >> + ( >> + cd sparse-checkout && >> + mkdir -p folder1/0/1 folder2/0 folder3 && >> + touch folder1/0/1/t.txt && >> + touch folder2/0/t.txt && >> + touch folder3/t.txt && >> + git add . && >> + git commit -am "Initial commit" >> + ) >> +' >> + >> +test_expect_success 'sparse-checkout completes subcommands' ' >> + test_completion "git sparse-checkout " <<-\EOF >> + list Z >> + init Z >> + set Z >> + add Z >> + reapply Z >> + disable Z >> + EOF >> +' >> + >> +test_expect_success 'cone mode sparse-checkout completes directory names' ' >> + # initialize sparse-checkout definitions >> + git -C sparse-checkout sparse-checkout set --cone folder1/0 folder3 && >> + >> + # test tab completion >> + ( >> + 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 >> + EOF >> + ) && >> + >> + ( >> + cd sparse-checkout/folder1 && >> + test_completion "git sparse-checkout add " <<-\EOF >> + ./ Z >> + 0 Z >> + 0/1 Z >> + EOF >> + ) >> +' >> + >> +test_expect_success 'non-cone mode sparse-checkout uses bash completion' ' >> + # reset sparse-checkout repo to non-cone mode >> + git -C sparse-checkout sparse-checkout disable && >> + git -C sparse-checkout sparse-checkout set --no-cone && >> + >> + ( >> + cd sparse-checkout && >> + # expected to be empty since we have not configured >> + # custom completion for non-cone mode >> + test_completion "git sparse-checkout set f" <<-\EOF >> + >> + EOF >> + ) >> +' >> + >> +test_expect_success 'git sparse-checkout set --cone completes directory names' ' >> + git -C sparse-checkout sparse-checkout disable && >> + >> + ( >> + cd sparse-checkout && >> + test_completion "git sparse-checkout set --cone f" <<-\EOF >> + folder1 Z >> + folder1/0 Z >> + folder1/0/1 Z >> + folder2 Z >> + folder2/0 Z >> + folder3 Z >> + EOF >> + ) >> +' >> + >> test_expect_success 'git switch - with -d, complete all references' ' >> test_completion "git switch -d " <<-\EOF >> HEAD Z > > The rest looks good to me.