| Message ID | a1c46c763fd4c832a6784e2a368199efedce17e9.1643921091.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | completion: sparse-checkout updates | expand |
On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Lessley Dennington <lessleydennington@gmail.com> > > Correct multiple issues with tab completion of the git sparse-checkout > command. These issues were: > > 1. git sparse-checkout <TAB> 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 <TAB> and git sparse-checkout add <TAB> 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 <lessleydennington@gmail.com> > --- > 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. > + __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.
On 2/3/22 3:52 PM, Elijah Newren wrote: > On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> >> From: Lessley Dennington <lessleydennington@gmail.com> >> >> Correct multiple issues with tab completion of the git sparse-checkout >> command. These issues were: >> >> 1. git sparse-checkout <TAB> 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 <TAB> and git sparse-checkout add <TAB> 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 <lessleydennington@gmail.com> >> --- >> 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.
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) ] || + [ "$(__git_find_on_cmdline --cone)" ]; then + __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