diff mbox series

[v7,3/3] completion: handle unusual characters for sparse-checkout

Message ID 996cbe7dfb73c211eae012813b352b0d0de3954c.1644255105.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 48803821b1712687d6e06e9d7a0e911eabecf4d1
Headers show
Series completion: sparse-checkout updates | expand

Commit Message

Lessley Dennington Feb. 7, 2022, 5:31 p.m. UTC
From: Lessley Dennington <lessleydennington@gmail.com>

Update the __gitcomp_directories method to de-quote and handle unusual
characters in directory names. Although this initially involved an attempt
to re-use the logic in __git_index_files, this method removed
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.

Note there are two tests for this new functionality - one for spaces and
accents and one for backslashes and tabs. The backslashes and tabs test
uses FUNNYNAMES to avoid running on Windows. This is because:

1. Backslashes are explicitly not allowed in Windows file paths.
2. Although tabs appear to be allowed when creating a file in a Windows
bash shell, they actually are not renderable (and appear as empty boxes
in the shell).

Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Co-authored-by: Lessley Dennington <lessleydennington@gmail.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
Reviewed-by: Elijah Newren <newren@gmail.com>
---
 contrib/completion/git-completion.bash | 24 ++++++-------
 t/t9902-completion.sh                  | 49 ++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 13 deletions(-)

Comments

Adam Dinwoodie April 6, 2022, 9:42 a.m. UTC | #1
On Mon, Feb 07, 2022 at 05:31:45PM +0000, Lessley Dennington via GitGitGadget wrote:
> <snip>
> 
> Note there are two tests for this new functionality - one for spaces and
> accents and one for backslashes and tabs. The backslashes and tabs test
> uses FUNNYNAMES to avoid running on Windows. This is because:
> 
> 1. Backslashes are explicitly not allowed in Windows file paths.
> 2. Although tabs appear to be allowed when creating a file in a Windows
> bash shell, they actually are not renderable (and appear as empty boxes
> in the shell).
> 
> <snip>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index b38a7302249..da6c86c64b3 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1508,6 +1508,55 @@ test_expect_success 'cone mode sparse-checkout completes directory names' '
> <snip>
> +# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes or tabs in paths
> +test_expect_success FUNNYNAMES 'cone mode sparse-checkout completes directory names with backslashes and tabs' '
> +	# reset sparse-checkout
> +	git -C sparse-checkout sparse-checkout disable &&
> +	(
> +		cd sparse-checkout &&
> +		mkdir "directory\with\backslashes" &&
> +		mkdir "$(printf "directory\twith\ttabs")" &&
> +		>"directory\with\backslashes/randomfile" &&
> +		>"$(printf "directory\twith\ttabs")/randomfile" &&
> +		git add . &&
> +		git commit -m "Add directory with backslashes and directory with tabs" &&
> +		git sparse-checkout set --cone "directory\with\backslashes" \
> +			"$(printf "directory\twith\ttabs")" &&
> +		test_completion "git sparse-checkout add dir" <<-\EOF &&
> +		directory\with\backslashes/
> +		directory	with	tabs/
> +		EOF
> +		rm -rf "directory\with\backslashes" &&
> +		rm -rf "$(printf "directory\twith\ttabs")" &&
> +		git add . &&
> +		git commit -m "Remove directory with backslashes and directory with tabs"
> +	)
> +'
> +
>  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 &&

On Cygwin this test is failing: `FUNNYNAMES` checks (a) the system isn't
MinGW, and (b) the filesystem supports tabs, quotes and newlines.
Cygwin isn't MinGW, and its interface to the file system supports all
those characters, but doesn't support backslashes.

I think the ideal solution here would be to split this test into two:
one part to cover the tab part, which will work on Cygwin, and a
separate part that has another prereq (which might be a new variant of
FUN*NAMES, or might be something more explicit like
SLASHALLOWEDINPATHCOMPONENTS, or might just be FUNNYNAMES,!CYGWIN).  But
it might be easier to just gate this test on FUNNYNAMES,!CYGWIN and not
worry about not being able to test the tab handling on Cygwin systems,
as the chances of a Cygwin-specific regression that'd only be caught by
this test seems pretty low to me.
diff mbox series

Patch

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index eeb80fdc6e4..6d81f03f291 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2988,7 +2988,7 @@  _git_show_branch ()
 
 __gitcomp_directories ()
 {
-	local _tmp_dir _tmp_completions
+	local _tmp_dir _tmp_completions _found=0
 
 	# 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
@@ -2996,20 +2996,18 @@  __gitcomp_directories ()
 	[[ "$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
+	# Find possible directory completions, adding trailing '/' characters,
+	# de-quoting, and handling unusual characters.
+	while IFS= read -r -d $'\0' c ; do
+		# If there are directory completions, find ones that start
+		# with "$cur", the current token, and put those in COMPREPLY
 		if [[ $c == "$cur"* ]]; then
-			COMPREPLY+=("$c")
+			COMPREPLY+=("$c/")
+			_found=1
 		fi
-		done
-	elif [[ "$cur" =~ /$ ]]; then
+	done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)
+
+	if [[ $_found == 0 ]] && [[ "$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 "
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index b38a7302249..da6c86c64b3 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1508,6 +1508,55 @@  test_expect_success 'cone mode sparse-checkout completes directory names' '
 	)
 '
 
+test_expect_success 'cone mode sparse-checkout completes directory names with spaces and accents' '
+	# reset sparse-checkout
+	git -C sparse-checkout sparse-checkout disable &&
+	(
+		cd sparse-checkout &&
+		mkdir "directory with spaces" &&
+		mkdir "directory-with-áccent" &&
+		>"directory with spaces/randomfile" &&
+		>"directory-with-áccent/randomfile" &&
+		git add . &&
+		git commit -m "Add directory with spaces and directory with accent" &&
+		git sparse-checkout set --cone "directory with spaces" \
+			"directory-with-áccent" &&
+		test_completion "git sparse-checkout add dir" <<-\EOF &&
+		directory with spaces/
+		directory-with-áccent/
+		EOF
+		rm -rf "directory with spaces" &&
+		rm -rf "directory-with-áccent" &&
+		git add . &&
+		git commit -m "Remove directory with spaces and directory with accent"
+	)
+'
+
+# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes or tabs in paths
+test_expect_success FUNNYNAMES 'cone mode sparse-checkout completes directory names with backslashes and tabs' '
+	# reset sparse-checkout
+	git -C sparse-checkout sparse-checkout disable &&
+	(
+		cd sparse-checkout &&
+		mkdir "directory\with\backslashes" &&
+		mkdir "$(printf "directory\twith\ttabs")" &&
+		>"directory\with\backslashes/randomfile" &&
+		>"$(printf "directory\twith\ttabs")/randomfile" &&
+		git add . &&
+		git commit -m "Add directory with backslashes and directory with tabs" &&
+		git sparse-checkout set --cone "directory\with\backslashes" \
+			"$(printf "directory\twith\ttabs")" &&
+		test_completion "git sparse-checkout add dir" <<-\EOF &&
+		directory\with\backslashes/
+		directory	with	tabs/
+		EOF
+		rm -rf "directory\with\backslashes" &&
+		rm -rf "$(printf "directory\twith\ttabs")" &&
+		git add . &&
+		git commit -m "Remove directory with backslashes and directory with tabs"
+	)
+'
+
 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 &&