Re: [PATCH v2 2/2] sparse-checkout: custom tab completion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/31/21 4:52 PM, Elijah Newren wrote:
On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:

From: Lessley Dennington <lessleydennington@xxxxxxxxx>

Fix custom tab completion for sparse-checkout command. This will ensure:

1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.

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@xxxxxxxxx>
---
  contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
  t/t9902-completion.sh                  |  8 +++---
  2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c82ccaebcc7..7de997ee64e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2986,24 +2986,38 @@ _git_show_branch ()
         __git_complete_revlist
  }

+__git_sparse_checkout_subcommand_opts="--cone --no-cone --sparse-index --no-sparse-index"
+
  _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
+               case "$cur" in
+                       --*)
+                               __gitcomp "--help"
+                               ;;
+                       *)
+                               __gitcomp "$subcommands"
+                               ;;
+               esac
         fi

-       case "$subcommand,$cur" in
-       init,--*)
-               __gitcomp "--cone"
-               ;;
-       set,--*)
-               __gitcomp "--stdin"
-               ;;
-       *)
-               ;;
+       case "$prev" in

Shouldn't this be "$subcommand" rather than "$prev"?  I think with
prev, it will only correctly complete the first path after "set",
"add", etc.

Good catch, thank you! Fixing in v3.
+               set)
+                       __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
+                       __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
+                       ;;
+               add)
+                       __gitcomp "--stdin"
+                       __gitcomp "$(git ls-tree -d -r HEAD --name-only)"

I was going to make a simple suggestion for making it just complete
one additional level at a time and leaving out the -r, and then tried
it out and found out it wasn't simple.  I got something working,
eventually, but it's slightly ugly...so it probably belongs in a
separate patch anyway.  If you're curious, it's basically replacing
the second __gitcomp... call for each of set and add with
`__gitcomp_directories`, after first defining:

__gitcomp_directories ()
{
     local _tmp_dir _tmp_completions

     # Get the directory of the current token; this differs from dirname
     # in that it keeps up to the final trailing slash.  If no slash found
     # that's fine too.
     [[ "$cur" =~ .*/ ]]
     _tmp_dir=$BASH_REMATCH

     # Find possible directory completions, adding trailing '/' characters
     _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
         sed -e s%$%/%)"

     if [[ -n "$_tmp_completions" ]]; then
         # There were some directory completions, so find ones that
         # start with "$cur", the current token, and put those in COMPREPLY
         local i=0 c IFS=$' \t\n'
         for c in $_tmp_completions; do
             if [[ $c == "$cur"* ]]; then
                 COMPREPLY+=("$c")
             fi
         done
     elif [[ "$cur" =~ /$ ]]; then
         # No possible further completions any deeper, so assume we're at
         # a leaf directory and just consider it complete
         __gitcomp_direct_append "$cur "
     fi
}

But I don't think that needs to be part of this series; I can submit
it later and hopefully get a completion expert to point out
better/cleaner ways of what I've done above.

I'm admittedly curious about what made this so difficult. I removed the '-r' and updated my tests to expect only directories at one level, and
they passed. But I imagine I'm being too simplistic.
+                       ;;
+               init|reapply)
+                       __gitcomp "$__git_sparse_checkout_subcommand_opts"
+                       ;;
+               *)
+                       ;;
         esac
  }

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 51d0f2d93a1..4dc93ee0595 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1447,7 +1447,7 @@ test_expect_success 'git checkout - with --detach, complete only references' '
         EOF
  '

-test_expect_failure 'sparse-checkout completes subcommands' '
+test_expect_success 'sparse-checkout completes subcommands' '
         test_completion "git sparse-checkout " <<-\EOF
         list Z
         init Z
@@ -1458,13 +1458,13 @@ test_expect_failure 'sparse-checkout completes subcommands' '
         EOF
  '

-test_expect_failure 'sparse-checkout completes options' '
+test_expect_success 'sparse-checkout completes options' '
         test_completion "git sparse-checkout --" <<-\EOF
         --help Z
         EOF
  '

-test_expect_failure 'sparse-checkout completes subcommand options' '
+test_expect_success 'sparse-checkout completes subcommand options' '
         test_completion "git sparse-checkout init --" <<-\EOF &&
         --cone Z
         --no-cone Z
@@ -1492,7 +1492,7 @@ test_expect_failure 'sparse-checkout completes subcommand options' '
         EOF
  '

-test_expect_failure 'sparse-checkout completes directory names' '
+test_expect_success 'sparse-checkout completes directory names' '
         # set up sparse-checkout repo
         git init sparse-checkout &&
         (
--
gitgitgadget

Patch looks good to me, other than the $prev vs. $subcommand thing.

Thanks!



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux