diff mbox series

[v4,3/3] completion: ensure cone mode completion with multiple <TAB>s

Message ID 760cae85bd4163d9cbef1585186cda60c8c3b35b.1643318514.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series completion: sparse-checkout updates | expand

Commit Message

Lessley Dennington Jan. 27, 2022, 9:21 p.m. UTC
From: Lessley Dennington <lessleydennington@gmail.com>

Ensure users in a cone mode sparse checkout can tab-complete to
directories at multiple levels with multiple <TAB>s. For example, if path
a/b/c/ exists in the current directory, running a<TAB>b<TAB> will result
in:

        a/b/c/

The 'sparse-checkout completes directory names' test has also been
updated with trailing slashes according to these changes.

Co-authored-by: Elijah Newren <newren@gmail.com>
Co-authored-by: Lessley Dennington <lessleydennington@gmail.com>
Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
---
 contrib/completion/git-completion.bash | 33 ++++++++++++++++++++++++--
 t/t9902-completion.sh                  | 12 +++++-----
 2 files changed, 37 insertions(+), 8 deletions(-)

Comments

Elijah Newren Jan. 28, 2022, 1:53 a.m. UTC | #1
On Thu, Jan 27, 2022 at 1:21 PM Lessley Dennington via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Lessley Dennington <lessleydennington@gmail.com>
>
> Ensure users in a cone mode sparse checkout can tab-complete to
> directories at multiple levels with multiple <TAB>s. For example, if path
> a/b/c/ exists in the current directory, running a<TAB>b<TAB> will result
> in:
>
>         a/b/c/

Um, didn't that already happen before this commit (other than the
trailing slash)?

In my mind, the reasons to switch from __git_complete_index_file to
__gitcomp_directories are:

  * __git_complete_index_file() completes on files, and we want only directories
  * __git_complete_index_file() can only complete on paths stored in
the index.  With --sparse-index mode, the directory (and all files
under it) we want to complete on may not exist in the index.

Both of these things make __git_complete_index_file() unsuitable.

> The 'sparse-checkout completes directory names' test has also been
> updated with trailing slashes according to these changes.
>
> Co-authored-by: Elijah Newren <newren@gmail.com>
> Co-authored-by: Lessley Dennington <lessleydennington@gmail.com>
> Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 33 ++++++++++++++++++++++++--
>  t/t9902-completion.sh                  | 12 +++++-----
>  2 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index a920cdab201..defcc8560c1 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2986,11 +2986,40 @@ _git_show_branch ()
>         __git_complete_revlist
>  }
>
> +__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
> +}

As noted at [1], this is going to have problems with special
characters.  In the testcase setup in t9902, modify the repository as
follows:

   mkdir "spaces are evil"
   mkdir "$(printf "tabs\tare\tworse")"
   >"spaces are evil/randomfile"
   >"$(printf "tabs\tare\tworse")/randomfile"
   git add .
   git commit -m "Add directories containing unusual characters"

and then try to complete any of these:

  git sparse-checkout set --cone spa<TAB>
  git sparse-checkout set --cone tab<TAB>
  git sparse-checkout set --cone ev<TAB>
  git sparse-checkout set --cone "tab<TAB>

You'll see it either gets the wrong thing, or doesn't get nothing (and
when it should get nothing, it gets something instead).

We do definitely want to use some form of __gitcomp_directories as I
noted above, but we'll have to lift some of the intelligent quoting
logic out of __git_complete_index_file.  It may be a little easier
since we don't have to worry about "full file path needs to be quoted
even though leading directory does not need to be".

(Also, if it makes it easier to deal with the testcases, you can just
use non-ascii characters instead of the tab character -- just run `git
shortlog -sn origin/master` in git.git, find someone's name with such
a character and copy and paste it into the directory name.  For
example, you could use the á from Gábor)

[1] https://lore.kernel.org/git/20220115095725.GA1738@szeder.dev/




> +
>  _git_sparse_checkout ()
>  {
>         local subcommands="list init set disable add reapply"
>         local subcommand="$(__git_find_on_cmdline "$subcommands")"
> -
>         if [ -z "$subcommand" ]; then
>                 __gitcomp "$subcommands"
>                 return
> @@ -3002,7 +3031,7 @@ _git_sparse_checkout ()
>                 ;;
>                 set,*|add,*)
>                         if [ $(__git config core.sparseCheckoutCone) ]; then
> -                               __git_complete_index_file "--directory"
> +                               __gitcomp_directories
>                         fi
>                 ;;
>         esac
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index f56ba3f64c9..c04fc25fa32 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1478,30 +1478,30 @@ test_expect_success 'cone mode sparse-checkout completes directory names' '
>         (
>                 cd sparse-checkout &&
>                 test_completion "git sparse-checkout set f" <<-\EOF
> -               folder1
> -               folder2
> -               folder3
> +               folder1/
> +               folder2/
> +               folder3/
>                 EOF
>         ) &&
>
>         (
>                 cd sparse-checkout &&
>                 test_completion "git sparse-checkout set folder1/" <<-\EOF
> -               folder1/0
> +               folder1/0/
>                 EOF
>         ) &&
>
>         (
>                 cd sparse-checkout &&
>                 test_completion "git sparse-checkout set folder1/0/" <<-\EOF
> -               folder1/0/1
> +               folder1/0/1/
>                 EOF
>         ) &&
>
>         (
>                 cd sparse-checkout/folder1 &&
>                 test_completion "git sparse-checkout add 0" <<-\EOF
> -               0
> +               0/
>                 EOF
>         )
>  '
> --
> gitgitgadget
diff mbox series

Patch

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a920cdab201..defcc8560c1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2986,11 +2986,40 @@  _git_show_branch ()
 	__git_complete_revlist
 }
 
+__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
+}
+
 _git_sparse_checkout ()
 {
 	local subcommands="list init set disable add reapply"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
-
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
 		return
@@ -3002,7 +3031,7 @@  _git_sparse_checkout ()
 		;;
 		set,*|add,*)
 			if [ $(__git config core.sparseCheckoutCone) ]; then
-				__git_complete_index_file "--directory"
+				__gitcomp_directories
 			fi
 		;;
 	esac
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f56ba3f64c9..c04fc25fa32 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1478,30 +1478,30 @@  test_expect_success 'cone mode sparse-checkout completes directory names' '
 	(
 		cd sparse-checkout &&
 		test_completion "git sparse-checkout set f" <<-\EOF
-		folder1
-		folder2
-		folder3
+		folder1/
+		folder2/
+		folder3/
 		EOF
 	) &&
 
 	(
 		cd sparse-checkout &&
 		test_completion "git sparse-checkout set folder1/" <<-\EOF
-		folder1/0
+		folder1/0/
 		EOF
 	) &&
 
 	(
 		cd sparse-checkout &&
 		test_completion "git sparse-checkout set folder1/0/" <<-\EOF
-		folder1/0/1
+		folder1/0/1/
 		EOF
 	) &&
 
 	(
 		cd sparse-checkout/folder1 &&
 		test_completion "git sparse-checkout add 0" <<-\EOF
-		0
+		0/
 		EOF
 	)
 '