Re: [PATCH v5 1/3] completion: address sparse-checkout issues

Lessley Dennington <lessleydennington@xxxxxxxxx> · Thu, 3 Feb 2022 16:34:29 -0800

On 2/3/22 3:52 PM, Elijah Newren wrote:
On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:

From: Lessley Dennington <lessleydennington@xxxxxxxxx>

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@xxxxxxxxx>
---
  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.






