completion: improve ls-files filter performance

From the output of ls-files, we remove all but the leftmost path component and then we eliminate duplicates. We do this in a while loop, which is a performance bottleneck when the number of iterations is large (e.g. for 60000 files in linux.git).

$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git

real    0m11.876s
user    0m4.685s
sys     0m6.808s

Using an equivalent sed script improves performance significantly:

$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git

real    0m1.372s
user    0m0.263s
sys     0m0.167s

The measurements were done with mingw64 bash, which is used by Git for Windows.

---
 contrib/completion/git-completion.bash | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

index 6da95b8..e3ddf27 100644
@@ -384,12 +384,7 @@ __git_index_files ()
 	local root="${2-.}" file
 
 	__git_ls_files_helper "$root" "$1" |
-	while read -r file; do
-		case "$file" in
-		?*/*) echo "${file%%/*}" ;;
-		*) echo "$file" ;;
-		esac
-	done | sort | uniq
+	sed -e '/^\//! s#/.*##' | sort | uniq
 }
 
 # Lists branches from the local repository.

2.7.4

When filtering the ls-files output we take care not to touch absolute paths. This is redundant, because ls-files will never output absolute paths. Furthermore, sorting the output is also redundant, because the output of ls-files is already sorted.

Remove the unnecessary operations.

---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index e3ddf27..394c3df 100644
@@ -384,7 +384,7 @@ __git_index_files ()
 	local root="${2-.}" file
 
 	__git_ls_files_helper "$root" "$1" |
-	sed -e '/^\//! s#/.*##' | sort | uniq
+	cut -f1 -d/ | uniq
 }
 
 # Lists branches from the local repository.

2.7.4

Makes sense, and I think you can and should just directly jump to this concluding state without having an intermediate "sed" version. The fact that the code does not have to worry about absolute paths and unsorted input is shared with the original version, too, so the proposed log message for this one applies equally well to such a squashed patch.

You didn't run the test suite, did you? ;)

First, neither 'git ls-files' nor 'git diff-index' produce quite the same order as the 'sort' utility does, e.g.:

  $ touch foo.c foo-zzz.c
  $ git add foo*
  $ git diff-index --name-only HEAD
  foo-zzz.c
  foo.c
  $ git diff-index --name-only HEAD |sort
  foo.c
  foo-zzz.c

Second, the output of 'git ls-files' is kind of "block-sorted": if you were to invoke it with the options '--cached --modified --others', then it will first list all untracked files in order, then all cached files in order, and finally all modified files in order. Note the implications:

- A file could theoretically be listed twice, because a modified file is inherently cached as well. I believe this doesn't happen currently, because no path completions use the combination of '--modified --cached', but we use a lot of options when completing paths for 'git status', and I haven't thought that through.

- A directory name is repeated in two (or more) blocks, if it contains modified and untracked files as well. We do use the combination of '--modified --others' for 'git add', and '--cached --others' for 'git mv', so this does happen.

Note also that there can be any number of other files between the same directory listed in two different blocks. That 'sort' that this patch is about to remove took care of this, but without that 'sort' the same directory name can be listed more than once even after 'uniq'. Consequently, the subsequent filtering of paths matching the current word to be completed might have twice as much work to do.

All this leads to the failure of an enormous test in t9902, hence my rethorical question at the beginning of my reply.

I have a short patch series collecting dust somewhere for a long while, which pulls a couple more tricks to make git-aware path completion faster, but haven't submitted it yet, because it doesn't work quite that well when filenames require quoting. Though, arguably the current version doesn't work quite that well with quoted filenames either, so...
Will try to dig up those patches.

Doesn't this depend on your locale?

    $ printf "foo%s\n" .c -zzz.c /c | LC_ALL=C sort
    foo-zzz.c
    foo.c
    foo/c

    $ printf "foo%s\n" .c -zzz.c /c | LC_ALL=en_US.UTF-8 sort
    foo/c
    foo.c
    foo-zzz.c

This is a lot more important consideration.

Thanks.

From the output of ls-files, we remove all but the leftmost path component and then we eliminate duplicates. We do this in a while loop, which is a performance bottleneck when the number of iterations is large (e.g. for 60000 files in linux.git).

$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git

real    0m11.876s
user    0m4.685s
sys     0m6.808s

Replacing the loop with the cut command improves performance significantly:

$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git

real    0m1.372s
user    0m0.263s
sys     0m0.167s

The measurements were done with Msys2 bash, which is used by Git for Windows.

When filtering the ls-files output we take care not to touch absolute paths. This is redundant, because ls-files will never output absolute paths. Remove the unnecessary operations.

The issue was reported here:
https://github.com/git-for-windows/git/issues/1533

---

My bad. I put the sort back in. Test t9902 is now pass. I did not run the other tests. I think the completion script is not used there.

I also considered Junio's and Johannes' comments.

Cool. Bash completion can certainly use more performance improvements.

 contrib/completion/git-completion.bash | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

index 6da95b8..69a2d41 100644
@@ -384,12 +384,7 @@ __git_index_files ()
 	local root="${2-.}" file
 
 	__git_ls_files_helper "$root" "$1" |
-	while read -r file; do
-		case "$file" in
-		?*/*) echo "${file%%/*}" ;;
-		*) echo "$file" ;;
-		esac
-	done | sort | uniq
+	cut -f1 -d/ | sort | uniq
 }
 
 # Lists branches from the local repository.

2.7.4

Hi drizzd,

Those are nice numbers right there, so I am eager to get this into Git for Windows as quickly as it stabilizes (i.e. when it hits next or so).

I was wondering about one thing, though:

So, the highlights of this patch series are:

- At the top of the worktree in linux.git with over 62k files the time needed for 'git rm <TAB>' goes down from 2.15s to 0.052s.

- Same place, uniquely completing Makefile with 'git rm Mak<TAB>' goes

    down from 2.16s to 0.033s.

- Completing paths containing escaped or quoted characters (except newline) will work properly, and won't fall back to Bash's filename completion, e.g. git add File\\w<TAB>' will list 'File\with\backslashes.c', but not the corresponding ignored object file.

Well, at least it appears to be working for me, but dealing with quoting especially is nuts. If someone knows simpler ways to remove escaping and quoting than in the functions added in patches 5 and 10, then please, I would really love to learn something :)

Someone using ZSH regularly should have a look and test it.

SZEDER Gábor (11):
t9902-completion: add tests demonstrating issues with quoted pathnames
completion: move __git_complete_index_file() next to its helpers
completion: simplify prefix path component handling during path
completion
completion: support completing non-ASCII pathnames
completion: improve handling quoted paths on the command line
completion: let 'ls-files' and 'diff-index' filter matching paths
completion: use 'awk' to strip trailing path components
t9902-completion: ignore COMPREPLY element order in some tests
completion: remove repeated dirnames with 'awk' during path completion
completion: improve handling quoted paths in 'git ls-files's output
completion: fill COMPREPLY directly when completing paths

 contrib/completion/git-completion.bash | 218 +++++++++++++++++++++----
 contrib/completion/git-completion.zsh  |   9 +
 t/t9902-completion.sh                  | 153 ++++++++++++++++-
 3 files changed, 348 insertions(+), 32 deletions(-)

2.17.0.366.gbe216a3084

Completion functions see all words on the command line verbatim, including any backslash-escapes, single and double quotes that might be there. Furthermore, git commands quote pathnames if they contain certain special characters. All these create various issues when doing git-aware path completion.

Add a couple of failing tests to demonstrate these issues.

Later patches in this series will discuss these issues in detail as they fix them.

---

Notes:
Do any more new tests need FUNNYNAMES* prereq?

 t/t9902-completion.sh | 91 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

index b7f5b1e632..ff2e4a8f5f 100755
@@ -1427,6 +1427,97 @@ test_expect_success 'complete files' '
 	test_completion "git add mom" "momified"
 '
 
+# The next tests only care about how the completion script deals with
+# unusual characters in path names.  By defining a custom completion
+# function to list untracked files they won't be influenced by future
+# changes of the completion functions of real git commands, and we
+# don't have to bother with adding files to the index in these tests.
+_git_test_path_comp ()
+{
+	__git_complete_index_file --others
+}
+
+test_expect_failure 'complete files - escaped characters on cmdline' '
+	test_when_finished "rm -rf \"New|Dir\"" &&
+	mkdir "New|Dir" &&
+	>"New|Dir/New&File.c" &&
+
+	test_completion "git test-path-comp N" \
+			"New|Dir" &&	# Bash will turn this into "New\|Dir/"
+	test_completion "git test-path-comp New\\|D" \
+			"New|Dir" &&
+	test_completion "git test-path-comp New\\|Dir/N" \
+			"New|Dir/New&File.c" &&	# Bash will turn this into
+						# "New\|Dir/New\&File.c "
+	test_completion "git test-path-comp New\\|Dir/New\\&F" \
+			"New|Dir/New&File.c"
+'
+
+test_expect_failure 'complete files - quoted characters on cmdline' '
+	test_when_finished "rm -r \"New(Dir\"" &&
+	mkdir "New(Dir" &&
+	>"New(Dir/New)File.c" &&
+
+	test_completion "git test-path-comp \"New(D" "New(Dir" &&
+	test_completion "git test-path-comp \"New(Dir/New)F" \
+			"New(Dir/New)File.c"
+'
+
+test_expect_failure 'complete files - UTF-8 in ls-files output' '
+	test_when_finished "rm -r árvíztűrő" &&
+	mkdir árvíztűrő &&
+	>"árvíztűrő/Сайн яваарай" &&
+
+	test_completion "git test-path-comp á" "árvíztűrő" &&
+	test_completion "git test-path-comp árvíztűrő/С" \
+			"árvíztűrő/Сайн яваарай"
+'
+
+if test_have_prereq !MINGW &&
+   mkdir 'New\Dir' 2>/dev/null &&
+   touch 'New\Dir/New"File.c' 2>/dev/null
+then
+	test_set_prereq FUNNYNAMES_BS_DQ
+else
+	say "Your filesystem does not allow \\ and \" in filenames."
+	rm -rf 'New\Dir'
+fi
+test_expect_failure FUNNYNAMES_BS_DQ \
+    'complete files - C-style escapes in ls-files output' '
+	test_when_finished "rm -r \"New\\\\Dir\"" &&
+
+	test_completion "git test-path-comp N" "New\\Dir" &&
+	test_completion "git test-path-comp New\\\\D" "New\\Dir" &&
+	test_completion "git test-path-comp New\\\\Dir/N" \
+			"New\\Dir/New\"File.c" &&
+	test_completion "git test-path-comp New\\\\Dir/New\\\"F" \
+			"New\\Dir/New\"File.c"
+'
+
+if test_have_prereq !MINGW &&
+   mkdir $'New\034Special\035Dir' 2>/dev/null &&
+   touch $'New\034Special\035Dir/New\036Special\037File' 2>/dev/null
+then
+	test_set_prereq FUNNYNAMES_SEPARATORS
+else
+	say 'Your filesystem does not allow special separator characters (FS, GS, RS, US) in filenames.'
+	rm -rf $'New\034Special\035Dir'
+fi
+test_expect_failure FUNNYNAMES_SEPARATORS \
+    'complete files - \nnn-escaped control characters in ls-files output' '
+	test_when_finished "rm -r '$'New\034Special\035Dir''" &&
+
+	# Note: these will be literal separator characters on the cmdline.
+	test_completion "git test-path-comp N" "'$'New\034Special\035Dir''" &&
+	test_completion "git test-path-comp '$'New\034S''" \
+			"'$'New\034Special\035Dir''" &&
+	test_completion "git test-path-comp '$'New\034Special\035Dir/''" \
+			"'$'New\034Special\035Dir/New\036Special\037File''" &&
+	test_completion "git test-path-comp '$'New\034Special\035Dir/New\036S''" \
+			"'$'New\034Special\035Dir/New\036Special\037File''"
+'
+
+
 test_expect_success "completion uses <cmd> completion for alias: !sh -c 'git <cmd> ...'" '
 	test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" &&
 	test_completion "git co m" <<-\EOF

2.17.0.366.gbe216a3084

Hmph, all of these look like they involve some funnynames ;-)

	$ >'hello world' && git add hel<TAB>

Well, I can' create a directory with a '|' in its name on FAT32 (on Linux), so this needs FUNNYNAMES prereq, too.

Noted.

The lack of '-f' is leftover from early versions of these tests, when I had a hard time getting the quoting-escaping right. Without the '-f' 'rm' errored out when I messed up, and the error message helpfully contained the path it wasn't able to delete.

This particular case already works, even without this patch series.

The problems start when you want to complete the filename after a space, e.g. 'hello\ w<TAB', as discussed in detail in patch 5. Actually, this was the first thing I tried to write a test for, but it didn't work out:
inside the 'test_completion' helper function the space acts as separator, and the completion script then sees 'hello\' and 'w' as two separate words.

Or, on second thought, using a different, more widely usable character would be better, so the test can be run on more platforms.

On another second thought, a test for the already working 'hel<TAB>' case could make sure that we won't mess up the value of IFS when filling COMPREPLY.

Sounds like we do not want 'f' from both tests, then? I think it is OK either way.

I was more wondering about preventing regressions---"it worked without this patch series, but now it is broken" is what I was worried about.

Hmph. That is somewhat unfortunate.

Actually, I used 'test_completion' in these new tests, because there is that big test checking file completion for various commands, and it already uses 'test_completion', so I just followed suit. Now, that test checks that the right type(s) of files are listed for various git commands, e.g. modified and untracked for 'git add', IOW that the caller of __git_complete_index_file() specifies the appropriate 'git ls-files' options. For those kind of checks 'test_completion' is great.

These new tests, however, are primarily interested in the inner workings of __git_complete_index_file() in the presence of escapes and/or quotes in the path to be completed and/or in the output of 'git ls-files'. For these kind of tests we could simply invoke __git_complete_index_file() directly, like we call __git_refs() directly to test refs completion. Then we could set the current path to be completed to whatever we want, including spaces, because it won't be subject to field splitting like the command line given to 'test_completion'.

So, I think for v2 I will rewrite these tests to call __git_complete_index_file() directly instead of using 'test_completion', and will include a test with spaces in path names.

Quite well thought-out reasoning. Thanks.

Unfortunately I couldn't get around to it soon enough, and now the topic 'sg/complete-paths' is already in next, so here are those test improvements on top.

SZEDER Gábor (2):
completion: don't return with error from __gitcomp_file_direct()
t9902-completion: exercise __git_complete_index_file() directly

 contrib/completion/git-completion.bash |   6 +-
 t/t9902-completion.sh                  | 225 +++++++++++++------------
 2 files changed, 122 insertions(+), 109 deletions(-)

2.17.0.799.gd371044c7c

In __gitcomp_file_direct() we tell Bash that it should handle our possible completion words as filenames with the following piece of cleverness:

  # use a hack to enable file mode in bash < 4
  compopt -o filenames +o nospace 2>/dev/null ||
  compgen -f /non-existing-dir/ > /dev/null

Unfortunately, this makes this function always return with error when it is not invoked in real completion, but e.g. in tests of 't9902-completion.sh':

- First the 'compopt' line errors out
- either because in Bash v3.x there is no such command,
- or because in Bash v4.x it complains about "not currently executing completion function",

- then 'compgen' just silently returns with error because of the non-existing directory.

Since __gitcomp_file_direct() is now the last command executed in __git_complete_index_file(), that function returns with error as well, which prevents it from being invoked in tests directly as is, and would require extra steps in test to hide its error code.

So let's make sure that __gitcomp_file_direct() doesn't return with error, because in the tests coming in the following patch we do want to exercise __git_complete_index_file() directly,

__gitcomp_file() contains the same construct, and thus it, too, always returns with error. Update that function accordingly as well.

While at it, also remove the space from between the redirection operator and the filename in both functions.

---
 contrib/completion/git-completion.bash | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 816901f0f0..8bc79a5226 100644
@@ -420,7 +420,8 @@ __gitcomp_file_direct ()
 
 	# use a hack to enable file mode in bash < 4
 	compopt -o filenames +o nospace 2>/dev/null ||
-	compgen -f /non-existing-dir/ > /dev/null
+	compgen -f /non-existing-dir/ >/dev/null ||
+	true
 }
 
 # Generates completion reply with compgen from newline-separated possible

@@ -442,7 +443,8 @@ __gitcomp_file ()
 
 	# use a hack to enable file mode in bash < 4
 	compopt -o filenames +o nospace 2>/dev/null ||
-	compgen -f /non-existing-dir/ > /dev/null
+	compgen -f /non-existing-dir/ >/dev/null ||
+	true
 }
 
 # Execute 'git ls-files', unless the --committable option is specified, in

2.17.0.799.gd371044c7c

The tests added in 2f271cd9cf (t9902-completion: add tests demonstrating issues with quoted pathnames, 2018-05-08) and in 2ab6eab4fe (completion: improve handling quoted paths in 'git ls-files's output, 2018-03-28) have a few shortcomings:

- All these test use the 'test_completion' helper function, thus they are exercising the whole completion machinery, although they are only interested in how git-aware path completion, specifically the __git_complete_index_file() function deals with unusual characters in pathnames and on the command line.

- These tests can't satisfactorily test the case of pathnames containing spaces, because 'test_completion' gets the words on the command line as a single argument and it uses space as word separator.

- Some of the tests are protected by different FUNNYNAMES_* prereqs depending on whether they put backslashes and double quotes or separator characters (FS, GS, RS, US) in pathnames, although a filesystem not allowing one likely doesn't allow the others either.

- One of the tests operates on paths containing '|' and '&' characters without being protected by a FUNNYNAMES prereq, but some filesystems (notably on Windows) don't allow these characters in pathnames, either.

Replace these tests with basically equivalent, more focused tests that call __git_complete_index_file() directly. Since this function only looks at the current word to be completed, i.e. the $cur variable, we can easily include pathnames containing spaces in the tests, so use such pathnames instead of pathnames containing '|' and '&'. Finally, use only a single FUNNYNAMES prereq for all kinds of special characters.

---
 t/t9902-completion.sh | 225 ++++++++++++++++++++++--------------------
 1 file changed, 118 insertions(+), 107 deletions(-)

index 955932174c..1b6d275254 100755
@@ -1209,6 +1209,124 @@ test_expect_success 'teardown after ref completion' '
 	git remote remove other
 '
 
+
+test_path_completion ()
+{
+	test $# = 2 || error "bug in the test script: not 2 parameters to test_path_completion"
+
+	local cur="$1" expected="$2"
+	echo "$expected" >expected &&
+	(
+		# In the following tests calling this function we only
+		# care about how __git_complete_index_file() deals with
+		# unusual characters in path names.  By requesting only
+		# untracked files we dont have to bother adding any
+		# paths to the index in those tests.
+		__git_complete_index_file --others &&
+		print_comp
+	) &&
+	test_cmp expected out
+}
+
+test_expect_success 'setup for path completion tests' '
+	mkdir simple-dir \
+	      "spaces in dir" \
+	      árvíztűrő &&
+	touch simple-dir/simple-file \
+	      "spaces in dir/spaces in file" \
+	      "árvíztűrő/Сайн яваарай" &&
+	if test_have_prereq !MINGW &&
+	   mkdir BS\\dir \
+		 '$'separators\034in\035dir'' &&
+	   touch BS\\dir/DQ\"file \
+		 '$'separators\034in\035dir/sep\036in\037file''
+	then
+		test_set_prereq FUNNYNAMES
+	else
+		rm -rf BS\\dir '$'separators\034in\035dir''
+	fi
+'
+
+test_expect_success '__git_complete_index_file - simple' '
+	test_path_completion simple simple-dir &&  # Bash is supposed to
+						   # add the trailing /.
+	test_path_completion simple-dir/simple simple-dir/simple-file
+'
+
+test_expect_success \
+    '__git_complete_index_file - escaped characters on cmdline' '
+	test_path_completion spac "spaces in dir" &&  # Bash will turn this
+						      # into "spaces\ in\ dir"
+	test_path_completion "spaces\\ i" \
+			     "spaces in dir" &&
+	test_path_completion "spaces\\ in\\ dir/s" \
+			     "spaces in dir/spaces in file" &&
+	test_path_completion "spaces\\ in\\ dir/spaces\\ i" \
+			     "spaces in dir/spaces in file"
+'
+
+test_expect_success \
+    '__git_complete_index_file - quoted characters on cmdline' '
+	# Testing with an opening but without a corresponding closing
+	# double quote is important.
+	test_path_completion \"spac "spaces in dir" &&
+	test_path_completion "\"spaces i" \
+			     "spaces in dir" &&
+	test_path_completion "\"spaces in dir/s" \
+			     "spaces in dir/spaces in file" &&
+	test_path_completion "\"spaces in dir/spaces i" \
+			     "spaces in dir/spaces in file"
+'
+
+test_expect_success '__git_complete_index_file - UTF-8 in ls-files output' '
+	test_path_completion á árvíztűrő &&
+	test_path_completion árvíztűrő/С "árvíztűrő/Сайн яваарай"
+'
+
+test_expect_success FUNNYNAMES \
+    '__git_complete_index_file - C-style escapes in ls-files output' '
+	test_path_completion BS \
+			     BS\\dir &&
+	test_path_completion BS\\\\d \
+			     BS\\dir &&
+	test_path_completion BS\\\\dir/DQ \
+			     BS\\dir/DQ\"file &&
+	test_path_completion BS\\\\dir/DQ\\\"f \
+			     BS\\dir/DQ\"file
+'
+
+test_expect_success FUNNYNAMES \
+    '__git_complete_index_file - \nnn-escaped characters in ls-files output' '
+	test_path_completion sep '$'separators\034in\035dir'' &&
+	test_path_completion '$'separators\034i'' \
+			     '$'separators\034in\035dir'' &&
+	test_path_completion '$'separators\034in\035dir/sep'' \
+			     '$'separators\034in\035dir/sep\036in\037file'' &&
+	test_path_completion '$'separators\034in\035dir/sep\036i'' \
+			     '$'separators\034in\035dir/sep\036in\037file''
+'
+
+test_expect_success FUNNYNAMES \
+    '__git_complete_index_file - removing repeated quoted path components' '
+	test_when_finished rm -r repeated-quoted &&
+	mkdir repeated-quoted &&      # A directory whose name in itself
+				      # would not be quoted ...
+	>repeated-quoted/0-file &&
+	>repeated-quoted/1\"file &&   # ... but here the file makes the
+				      # dirname quoted ...
+	>repeated-quoted/2-file &&
+	>repeated-quoted/3\"file &&   # ... and here, too.
+
+	# Still, we shold only list the directory name only once.
+	test_path_completion repeated repeated-quoted
+'
+
+test_expect_success 'teardown after path completion tests' '
+	rm -rf simple-dir "spaces in dir" árvíztűrő \
+	       BS\\dir '$'separators\034in\035dir''
+'
+
+
 test_expect_success '__git_get_config_variables' '
 	cat >expect <<-EOF &&
 	name-1

@@ -1469,113 +1587,6 @@ test_expect_success 'complete files' '
 	test_completion "git add mom" "momified"
 '
 
-# The next tests only care about how the completion script deals with
-# unusual characters in path names.  By defining a custom completion
-# function to list untracked files they won't be influenced by future
-# changes of the completion functions of real git commands, and we
-# don't have to bother with adding files to the index in these tests.
-_git_test_path_comp ()
-{
-	__git_complete_index_file --others
-}
-
-test_expect_success 'complete files - escaped characters on cmdline' '
-	test_when_finished "rm -rf \"New|Dir\"" &&
-	mkdir "New|Dir" &&
-	>"New|Dir/New&File.c" &&
-
-	test_completion "git test-path-comp N" \
-			"New|Dir" &&	# Bash will turn this into "New\|Dir/"
-	test_completion "git test-path-comp New\\|D" \
-			"New|Dir" &&
-	test_completion "git test-path-comp New\\|Dir/N" \
-			"New|Dir/New&File.c" &&	# Bash will turn this into
-						# "New\|Dir/New\&File.c "
-	test_completion "git test-path-comp New\\|Dir/New\\&F" \
-			"New|Dir/New&File.c"
-'
-
-test_expect_success 'complete files - quoted characters on cmdline' '
-	test_when_finished "rm -r \"New(Dir\"" &&
-	mkdir "New(Dir" &&
-	>"New(Dir/New)File.c" &&
-
-	# Testing with an opening but without a corresponding closing
-	# double quote is important.
-	test_completion "git test-path-comp \"New(D" "New(Dir" &&
-	test_completion "git test-path-comp \"New(Dir/New)F" \
-			"New(Dir/New)File.c"
-'
-
-test_expect_success 'complete files - UTF-8 in ls-files output' '
-	test_when_finished "rm -r árvíztűrő" &&
-	mkdir árvíztűrő &&
-	>"árvíztűrő/Сайн яваарай" &&
-
-	test_completion "git test-path-comp á" "árvíztűrő" &&
-	test_completion "git test-path-comp árvíztűrő/С" \
-			"árvíztűrő/Сайн яваарай"
-'
-
-# Testing with a path containing a backslash is important.
-if test_have_prereq !MINGW &&
-   mkdir 'New\Dir' 2>/dev/null &&
-   touch 'New\Dir/New"File.c' 2>/dev/null
-then
-	test_set_prereq FUNNYNAMES_BS_DQ
-else
-	say "Your filesystem does not allow \\ and \" in filenames."
-	rm -rf 'New\Dir'
-fi
-test_expect_success FUNNYNAMES_BS_DQ \
-    'complete files - C-style escapes in ls-files output' '
-	test_when_finished "rm -r \"New\\\\Dir\"" &&
-
-	test_completion "git test-path-comp N" "New\\Dir" &&
-	test_completion "git test-path-comp New\\\\D" "New\\Dir" &&
-	test_completion "git test-path-comp New\\\\Dir/N" \
-			"New\\Dir/New\"File.c" &&
-	test_completion "git test-path-comp New\\\\Dir/New\\\"F" \
-			"New\\Dir/New\"File.c"
-'
-
-if test_have_prereq !MINGW &&
-   mkdir $'New\034Special\035Dir' 2>/dev/null &&
-   touch $'New\034Special\035Dir/New\036Special\037File' 2>/dev/null
-then
-	test_set_prereq FUNNYNAMES_SEPARATORS
-else
-	say 'Your filesystem does not allow special separator characters (FS, GS, RS, US) in filenames.'
-	rm -rf $'New\034Special\035Dir'
-fi
-test_expect_success FUNNYNAMES_SEPARATORS \
-    'complete files - \nnn-escaped control characters in ls-files output' '
-	test_when_finished "rm -r '$'New\034Special\035Dir''" &&
-
-	# Note: these will be literal separator characters on the cmdline.
-	test_completion "git test-path-comp N" "'$'New\034Special\035Dir''" &&
-	test_completion "git test-path-comp '$'New\034S''" \
-			"'$'New\034Special\035Dir''" &&
-	test_completion "git test-path-comp '$'New\034Special\035Dir/''" \
-			"'$'New\034Special\035Dir/New\036Special\037File''" &&
-	test_completion "git test-path-comp '$'New\034Special\035Dir/New\036S''" \
-			"'$'New\034Special\035Dir/New\036Special\037File''"
-'
-
-test_expect_success FUNNYNAMES_BS_DQ \
-    'complete files - removing repeated quoted path components' '
-	test_when_finished rm -rf NewDir &&
-	mkdir NewDir &&    # A dirname which in itself would not be quoted ...
-	>NewDir/0-file &&
-	>NewDir/1\"file && # ... but here the file makes the dirname quoted ...
-	>NewDir/2-file &&
-	>NewDir/3\"file && # ... and here, too.
-
-	# Still, we should only list it once.
-	test_completion "git test-path-comp New" "NewDir"
-'
-
-
 test_expect_success "completion uses <cmd> completion for alias: !sh -c 'git <cmd> ...'" '
 	test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" &&
 	test_completion "git co m" <<-\EOF

2.17.0.799.gd371044c7c

s/these test/&s/

Hi Gábor,

Makes sense.

	test $# = 2 || error "BUG: test_path_completion requires 2 parameters"

Hi Gábor,

I can verify that the weeks-long breakage of pu on Windows has been addressed, probably by this patch series.

Ciao,
Dscho

Hi Junio,

Please note that as the branch that is fixed by these two patches was already merged down to next, the Continuous Testing on Windows reports that next is broken.

(I saw that breakage for over a week, but was too busy elsewhere to act on it.)

It would be really lovely to see it fixed soon, so that other bugs cannot be hidden by that breakage.

And I would also love to see sg/complete-paths to be merged down to master *only* in conjunction with these two patches, not on its own (because that would break the Continuous Testing on Windows of master, which is something I really want us to avoid).

Thanks,
Dscho

Hi Gábor,

Yes.

2018-04-18T11:12:55.0436371Z expecting success: 
2018-04-18T11:12:55.0436665Z 	test_when_finished "rm -rf \"New|Dir\"" &&
2018-04-18T11:12:55.0436799Z 	mkdir "New|Dir" &&
2018-04-18T11:12:55.0436904Z 	>"New|Dir/New&File.c" &&
2018-04-18T11:12:55.0436972Z 
2018-04-18T11:12:55.0437158Z 	test_completion "git test-path-comp N" \
2018-04-18T11:12:55.0437296Z 			"New|Dir" &&	# Bash will turn this into "New\|Dir/"
2018-04-18T11:12:55.0437413Z 	test_completion "git test-path-comp New\\|D" \
2018-04-18T11:12:55.0437522Z 			"New|Dir" &&
2018-04-18T11:12:55.0437629Z 	test_completion "git test-path-comp New\\|Dir/N" \
2018-04-18T11:12:55.0437767Z 			"New|Dir/New&File.c" &&	# Bash will turn this into
2018-04-18T11:12:55.0438040Z 						# "New\|Dir/New\&File.c "
2018-04-18T11:12:55.0438152Z 	test_completion "git test-path-comp New\\|Dir/New\\&F" \
2018-04-18T11:12:55.0438504Z 			"New|Dir/New&File.c"
2018-04-18T11:12:55.0438742Z 
2018-04-18T11:12:55.0590984Z ++ test_when_finished 'rm -rf "New|Dir"'
2018-04-18T11:12:55.0591722Z ++ test 0 = 0
2018-04-18T11:12:55.0592001Z ++ test_cleanup='{ rm -rf "New|Dir"
2018-04-18T11:12:55.0592290Z 		} && (exit "$eval_ret"); eval_ret=$?; :'
2018-04-18T11:12:55.0592472Z ++ mkdir 'New|Dir'
2018-04-18T11:12:55.0717255Z ++ test_completion 'git test-path-comp N' 'New|Dir'
2018-04-18T11:12:55.0717680Z ++ test 2 -gt 1
2018-04-18T11:12:55.0718062Z ++ printf '%s\n' 'New|Dir'
2018-04-18T11:12:55.0718275Z ++ run_completion 'git test-path-comp N'
2018-04-18T11:12:55.0718447Z ++ local -a COMPREPLY _words
2018-04-18T11:12:55.0718631Z ++ local _cword
2018-04-18T11:12:55.0718806Z ++ _words=($1)
2018-04-18T11:12:55.0718965Z ++ test N = ' '
2018-04-18T11:12:55.0719124Z ++ ((  _cword = 3 - 1  ))
2018-04-18T11:12:55.0719286Z ++ __git_wrap__git_main
2018-04-18T11:12:55.0719467Z ++ __git_func_wrap __git_main
2018-04-18T11:12:55.0719633Z ++ local cur words cword prev
2018-04-18T11:12:55.0719801Z ++ _get_comp_words_by_ref -n =: cur words cword prev
2018-04-18T11:12:55.0720074Z ++ '[' 6 -gt 0 ']'
2018-04-18T11:12:55.0720239Z ++ case "$1" in
2018-04-18T11:12:55.0720406Z ++ shift
2018-04-18T11:12:55.0720584Z ++ '[' 5 -gt 0 ']'
2018-04-18T11:12:55.0720742Z ++ case "$1" in
2018-04-18T11:12:55.0720899Z ++ shift
2018-04-18T11:12:55.0721054Z ++ '[' 4 -gt 0 ']'
2018-04-18T11:12:55.0721240Z ++ case "$1" in
2018-04-18T11:12:55.0721392Z ++ cur=N
2018-04-18T11:12:55.0721547Z ++ shift
2018-04-18T11:12:55.0721717Z ++ '[' 3 -gt 0 ']'
2018-04-18T11:12:55.0721879Z ++ case "$1" in
2018-04-18T11:12:55.0722040Z ++ words=("${_words[@]}")
2018-04-18T11:12:55.0722201Z ++ shift
2018-04-18T11:12:55.0722396Z ++ '[' 2 -gt 0 ']'
2018-04-18T11:12:55.0722931Z ++ case "$1" in
2018-04-18T11:12:55.0723070Z ++ cword=2
2018-04-18T11:12:55.0723221Z ++ shift
2018-04-18T11:12:55.0723357Z ++ '[' 1 -gt 0 ']'
2018-04-18T11:12:55.0723575Z ++ case "$1" in
2018-04-18T11:12:55.0723735Z ++ prev=test-path-comp
2018-04-18T11:12:55.0723874Z ++ shift
2018-04-18T11:12:55.0724009Z ++ '[' 0 -gt 0 ']'
2018-04-18T11:12:55.0724397Z ++ __git_main
2018-04-18T11:12:55.0724984Z ++ local i c=1 command __git_dir __git_repo_path
2018-04-18T11:12:55.0725183Z ++ local __git_C_args C_args_count=0
2018-04-18T11:12:55.0725353Z ++ '[' 1 -lt 2 ']'
2018-04-18T11:12:55.0725537Z ++ i=test-path-comp
2018-04-18T11:12:55.0725712Z ++ case "$i" in
2018-04-18T11:12:55.0725882Z ++ command=test-path-comp
2018-04-18T11:12:55.0726057Z ++ break
2018-04-18T11:12:55.0726270Z ++ '[' -z test-path-comp ']'
2018-04-18T11:12:55.0726446Z ++ __git_complete_command test-path-comp
2018-04-18T11:12:55.0726621Z ++ local command=test-path-comp
2018-04-18T11:12:55.0726816Z ++ local completion_func=_git_test_path_comp
2018-04-18T11:12:55.0726992Z ++ declare -f _git_test_path_comp
2018-04-18T11:12:55.0727353Z ++ declare -f _git_test_path_comp
2018-04-18T11:12:55.0727547Z ++ _git_test_path_comp
2018-04-18T11:12:55.0727716Z ++ __git_complete_index_file --others
2018-04-18T11:12:55.0727890Z ++ local dequoted_word pfx= cur_
2018-04-18T11:12:55.0728234Z ++ __git_dequote N
2018-04-18T11:12:55.0728418Z ++ local rest=N len ch
2018-04-18T11:12:55.0728869Z ++ dequoted_word=
2018-04-18T11:12:55.0729020Z ++ test -n N
2018-04-18T11:12:55.0729152Z ++ len=0
2018-04-18T11:12:55.0729309Z ++ dequoted_word=N
2018-04-18T11:12:55.0729440Z ++ rest=
2018-04-18T11:12:55.0729666Z ++ case "${rest:0:1}" in
2018-04-18T11:12:55.0729822Z ++ test -n ''
2018-04-18T11:12:55.0729993Z ++ case "$dequoted_word" in
2018-04-18T11:12:55.0730133Z ++ cur_=N
2018-04-18T11:12:55.0782504Z +++ __git_index_files --others '' N
2018-04-18T11:12:55.0782805Z +++ local root= match=N
2018-04-18T11:12:55.0845235Z +++ __git_ls_files_helper '' --others N
2018-04-18T11:12:55.0845440Z +++ '[' --others == --committable ']'
2018-04-18T11:12:55.0845567Z +++ __git -C '' -c core.quotePath=false ls-files --exclude-standard --others -- 'N*'
2018-04-18T11:12:55.0845706Z +++ git -C '' -c core.quotePath=false ls-files --exclude-standard --others -- 'N*'
2018-04-18T11:12:55.0907632Z +++ awk -F / -v pfx= '{
2018-04-18T11:12:55.0907806Z 		paths[$1] = 1
2018-04-18T11:12:55.0908985Z 	}
2018-04-18T11:12:55.0942839Z 	END {
2018-04-18T11:12:55.0943072Z 		for (p in paths) {
2018-04-18T11:12:55.0949175Z 			if (substr(p, 1, 1) != "\"") {
2018-04-18T11:12:55.0949458Z 				# No special characters, easy!
2018-04-18T11:12:55.0949659Z 				print pfx p
2018-04-18T11:12:55.0949823Z 				continue
2018-04-18T11:12:55.0949999Z 			}
2018-04-18T11:12:55.0950121Z 
2018-04-18T11:12:55.0950335Z 			# The path is quoted.
2018-04-18T11:12:55.0950829Z 			p = dequote(p)
2018-04-18T11:12:55.0951171Z 			if (p == "")
2018-04-18T11:12:55.0951555Z 				continue
2018-04-18T11:12:55.0951672Z 
2018-04-18T11:12:55.0951856Z 			# Even when a directory name itself does not contain
2018-04-18T11:12:55.0952038Z 			# any special characters, it will still be quoted if
2018-04-18T11:12:55.0952213Z 			# any of its (stripped) trailing path components do.
2018-04-18T11:12:55.0952407Z 			# Because of this we may have seen the same direcory
2018-04-18T11:12:55.0952583Z 			# both quoted and unquoted.
2018-04-18T11:12:55.0952762Z 			if (p in paths)
2018-04-18T11:12:55.0952948Z 				# We have seen the same directory unquoted,
2018-04-18T11:12:55.0953117Z 				# skip it.
2018-04-18T11:12:55.0953276Z 				continue
2018-04-18T11:12:55.0953441Z 			else
2018-04-18T11:12:55.0953613Z 				print pfx p
2018-04-18T11:12:55.0953766Z 		}
2018-04-18T11:12:55.0953914Z 	}
2018-04-18T11:12:55.0954461Z 	function dequote(p,    bs_idx, out, esc, esc_idx, dec) {
2018-04-18T11:12:55.0954650Z 		# Skip opening double quote.
2018-04-18T11:12:55.0954813Z 		p = substr(p, 2)
2018-04-18T11:12:55.0954935Z 
2018-04-18T11:12:55.0955237Z 		# Interpret backslash escape sequences.
2018-04-18T11:12:55.0955415Z 		while ((bs_idx = index(p, "\\")) != 0) {
2018-04-18T11:12:55.0955533Z 			out = out substr(p, 1, bs_idx - 1)
2018-04-18T11:12:55.0955638Z 			esc = substr(p, bs_idx + 1, 1)
2018-04-18T11:12:55.0955743Z 			p = substr(p, bs_idx + 2)
2018-04-18T11:12:55.0955830Z 
2018-04-18T11:12:55.0955939Z 			if ((esc_idx = index("abtvfr\"\\", esc)) != 0) {
2018-04-18T11:12:55.0956079Z 				# C-style one-character escape sequence.
2018-04-18T11:12:55.0956513Z 				out = out substr("\a\b\t\v\f\r\"\\",
2018-04-18T11:12:55.0956631Z esc_idx, 1)
2018-04-18T11:12:55.0956745Z 			} else if (esc == "n") {
2018-04-18T11:12:55.0956853Z 				# Uh-oh, a newline character.
2018-04-18T11:12:55.0956973Z 				# We cant reliably put a pathname
2018-04-18T11:12:55.0957086Z 				# containing a newline into COMPREPLY,
2018-04-18T11:12:55.0957193Z 				# and the newline would create a mess.
2018-04-18T11:12:55.0957300Z 				# Skip this path.
2018-04-18T11:12:55.0957413Z 				return ""
2018-04-18T11:12:55.0957510Z 			} else {
2018-04-18T11:12:55.0957808Z 				# Must be a \nnn octal value, then.
2018-04-18T11:12:55.0958070Z 				dec = esc * 64 + \
2018-04-18T11:12:55.0958184Z 				      substr(p, 1, 1) * 8  + \
2018-04-18T11:12:55.0958274Z 				      substr(p, 2, 1)
2018-04-18T11:12:55.0958369Z 				out = out sprintf("%c", dec)
2018-04-18T11:12:55.0958587Z 				p = substr(p, 3)
2018-04-18T11:12:55.0958692Z 			}
2018-04-18T11:12:55.0958769Z 		}
2018-04-18T11:12:55.0958862Z 		# Drop closing double quote, if there is one.
2018-04-18T11:12:55.0958969Z 		# (There isnt any if this is a directory, as it was
2018-04-18T11:12:55.0959153Z 		# already stripped with the trailing path components.)
2018-04-18T11:12:55.0959256Z 		if (substr(p, length(p), 1) == "\"")
2018-04-18T11:12:55.0959356Z 			out = out substr(p, 1, length(p) - 1)
2018-04-18T11:12:55.0959441Z 		else
2018-04-18T11:12:55.0959541Z 			out = out p
2018-04-18T11:12:55.0959598Z 
2018-04-18T11:12:55.0959682Z 		return out
2018-04-18T11:12:55.0959763Z 	}'
2018-04-18T11:12:55.1182135Z ++ __gitcomp_file_direct $'New∩\201╝Dir'
2018-04-18T11:12:55.1182355Z ++ local 'IFS=
2018-04-18T11:12:55.1182439Z '
2018-04-18T11:12:55.1182518Z ++ COMPREPLY=($1)
2018-04-18T11:12:55.1182622Z ++ compopt -o filenames +o nospace
2018-04-18T11:12:55.1182877Z ++ compgen -f /non-existing-dir/
2018-04-18T11:12:55.1182979Z ++ return 0
2018-04-18T11:12:55.1183055Z ++ return
2018-04-18T11:12:55.1183147Z ++ print_comp
2018-04-18T11:12:55.1183224Z ++ local 'IFS=
2018-04-18T11:12:55.1183300Z '
2018-04-18T11:12:55.1183398Z ++ echo $'New∩\201╝Dir'
2018-04-18T11:12:55.1183508Z ++ sort out
2018-04-18T11:12:55.1183605Z ++ /usr/bin/sort out
2018-04-18T11:12:55.1306331Z ++ test_cmp expected out_sorted
2018-04-18T11:12:55.1306825Z ++ mingw_test_cmp expected out_sorted
2018-04-18T11:12:55.1307024Z ++ local test_cmp_a= test_cmp_b=
2018-04-18T11:12:55.1307233Z ++ local stdin_for_diff=
2018-04-18T11:12:55.1307401Z ++ test -s expected
2018-04-18T11:12:55.1307568Z ++ test -s out_sorted
2018-04-18T11:12:55.1307742Z ++ mingw_read_file_strip_cr_ test_cmp_a
2018-04-18T11:12:55.1308083Z ++ local line
2018-04-18T11:12:55.1308424Z ++ :
2018-04-18T11:12:55.1308566Z ++ IFS=$'\r'
2018-04-18T11:12:55.1308717Z ++ read -r -d '
2018-04-18T11:12:55.1308852Z ' line
2018-04-18T11:12:55.1317521Z ++ line='New|Dir
2018-04-18T11:12:55.1317784Z '
2018-04-18T11:12:55.1318257Z ++ eval 'test_cmp_a=$test_cmp_a$line'
2018-04-18T11:12:55.1318424Z +++ test_cmp_a='New|Dir
2018-04-18T11:12:55.1318569Z '
2018-04-18T11:12:55.1318724Z ++ :
2018-04-18T11:12:55.1318871Z ++ IFS=$'\r'
2018-04-18T11:12:55.1319027Z ++ read -r -d '
2018-04-18T11:12:55.1319170Z ' line
2018-04-18T11:12:55.1319334Z ++ test -z ''
2018-04-18T11:12:55.1319476Z ++ break
2018-04-18T11:12:55.1319628Z ++ mingw_read_file_strip_cr_ test_cmp_b
2018-04-18T11:12:55.1319797Z ++ local line
2018-04-18T11:12:55.1319939Z ++ :
2018-04-18T11:12:55.1320081Z ++ IFS=$'\r'
2018-04-18T11:12:55.1320240Z ++ read -r -d '
2018-04-18T11:12:55.1320384Z ' line
2018-04-18T11:12:55.1320555Z ++ line='NewDir
2018-04-18T11:12:55.1320915Z '
2018-04-18T11:12:55.1321099Z ++ eval 'test_cmp_b=$test_cmp_b$line'
2018-04-18T11:12:55.1321266Z +++ test_cmp_b='NewDir
2018-04-18T11:12:55.1321422Z '
2018-04-18T11:12:55.1321570Z ++ :
2018-04-18T11:12:55.1321705Z ++ IFS=$'\r'
2018-04-18T11:12:55.1321859Z ++ read -r -d '
2018-04-18T11:12:55.1321994Z ' line
2018-04-18T11:12:55.1322219Z ++ test -z ''
2018-04-18T11:12:55.1322361Z ++ break
2018-04-18T11:12:55.1322497Z ++ test -n 'New|Dir
2018-04-18T11:12:55.1322649Z '
2018-04-18T11:12:55.1322828Z ++ test -n 'NewDir
2018-04-18T11:12:55.1322977Z '
2018-04-18T11:12:55.1323109Z ++ test 'New|Dir
2018-04-18T11:12:55.1323397Z ' = 'NewDir
2018-04-18T11:12:55.1323540Z '
2018-04-18T11:12:55.1323680Z ++ eval 'diff -u "$@" '
2018-04-18T11:12:55.1323840Z +++ diff -u expected out_sorted
2018-04-18T11:12:55.1454977Z --- expected	2018-04-18 11:12:55.065444100 +0000
2018-04-18T11:12:55.1455785Z error: last command exited with $?=1
2018-04-18T11:12:55.1456722Z +++ out_sorted	2018-04-18 11:12:55.127568400 +0000
2018-04-18T11:12:55.1457211Z @@ -1 +1 @@
2018-04-18T11:12:55.1457408Z -New|Dir
2018-04-18T11:12:55.1457752Z +NewDir
2018-04-18T11:12:55.1457975Z not ok 111 - complete files - escaped characters on cmdline
2018-04-18T11:12:55.1645995Z #	
2018-04-18T11:12:55.1646221Z #		test_when_finished "rm -rf \"New|Dir\"" &&
2018-04-18T11:12:55.1646380Z #		mkdir "New|Dir" &&
2018-04-18T11:12:55.1646487Z #		>"New|Dir/New&File.c" &&
2018-04-18T11:12:55.1646583Z #	
2018-04-18T11:12:55.1646865Z #		test_completion "git test-path-comp N" \
2018-04-18T11:12:55.1646986Z #				"New|Dir" &&	# Bash will turn this into "New\|Dir/"
2018-04-18T11:12:55.1647108Z #		test_completion "git test-path-comp New\\|D" \
2018-04-18T11:12:55.1647212Z #				"New|Dir" &&
2018-04-18T11:12:55.1647346Z #		test_completion "git test-path-comp New\\|Dir/N" \
2018-04-18T11:12:55.1647510Z # "New|Dir/New&File.c" &&	# Bash will turn this into
2018-04-18T11:12:55.1647636Z # # "New\|Dir/New\&File.c "
2018-04-18T11:12:55.1647775Z #		test_completion "git test-path-comp New\\|Dir/New\\&F" \
2018-04-18T11:12:55.1647886Z # "New|Dir/New&File.c"

2018-04-18T11:12:43.0459702Z     Your filesystem does not allow \ and " in filenames.
2018-04-18T11:12:43.0459823Z     skipped: complete files - C-style escapes in ls-files output (missing FUNNYNAMES_BS_DQ)

I followed suit of all existing FUNNYNAMES checks:

  $ git grep -B3 'test_set_prereq FUNNYNAMES' master t/
  master:t/t3600-rm.sh-if test_have_prereq !MINGW && touch -- 'tab
   embedded' 'newline
  master:t/t3600-rm.sh-embedded' 2>/dev/null
  master:t/t3600-rm.sh-then
  master:t/t3600-rm.sh:   test_set_prereq FUNNYNAMES
  --
  master:t/t4135-apply-weird-filenames.sh-        if test_have_prereq !MINGW &&
  master:t/t4135-apply-weird-filenames.sh-                touch --
"tab   embedded.txt" '\''"quoteembedded".txt'\''
  master:t/t4135-apply-weird-filenames.sh-        then
  master:t/t4135-apply-weird-filenames.sh:
test_set_prereq FUNNYNAMES
  --
  master:t/t9903-bash-prompt.sh-
  master:t/t9903-bash-prompt.sh-if test_have_prereq !MINGW && mkdir
"$repo_with_newline" 2>/dev/null
  master:t/t9903-bash-prompt.sh-then
  master:t/t9903-bash-prompt.sh:  test_set_prereq FUNNYNAMES

How am I supposed to check this, then?

It's much easier to read, understand and modify the functions related to git-aware path completion when they are right next to each other.

---
 contrib/completion/git-completion.bash | 39 +++++++++++++-------------
 1 file changed, 19 insertions(+), 20 deletions(-)

index b09c8a2362..36d3c6f928 100644
@@ -396,6 +396,25 @@ __git_index_files ()
 	done | sort | uniq
 }
 
+# __git_complete_index_file requires 1 argument:
+# 1: the options to pass to ls-file
+#
+# The exception is --committable, which finds the files appropriate commit.
+__git_complete_index_file ()
+{
+	local pfx="" cur_="$cur"
+
+	case "$cur_" in
+	?*/*)
+		pfx="${cur_%/*}"
+		cur_="${cur_##*/}"
+		pfx="${pfx}/"
+		;;
+	esac
+
+	__gitcomp_file "$(__git_index_files "$1" ${pfx:+"$pfx"})" "$pfx" "$cur_"
+}
+
 # Lists branches from the local repository.
 # 1: A prefix to be added to each listed branch (optional).
 # 2: List only branches matching this word (optional; list all branches if

@@ -712,26 +731,6 @@ __git_complete_revlist_file ()
 	esac
 }
 
-
-# __git_complete_index_file requires 1 argument:
-# 1: the options to pass to ls-file
-#
-# The exception is --committable, which finds the files appropriate commit.
-__git_complete_index_file ()
-{
-	local pfx="" cur_="$cur"
-
-	case "$cur_" in
-	?*/*)
-		pfx="${cur_%/*}"
-		cur_="${cur_##*/}"
-		pfx="${pfx}/"
-		;;
-	esac
-
-	__gitcomp_file "$(__git_index_files "$1" ${pfx:+"$pfx"})" "$pfx" "$cur_"
-}
-
 __git_complete_file ()
 {
 	__git_complete_revlist_file

2.17.0.366.gbe216a3084

Once upon a time 'git -C "" cmd' errored out with "Cannot change to '': No such file or directory", therefore the completion script took extra steps to run 'git -C "." cmd' instead; see fca416a41e (completion: use "git -C $there" instead of (cd $there && git ...), 2014-10-09).

Those extra steps are not needed since 6a536e2076 (git: treat "git -C '<path>'" as a no-op when <path> is empty, 2015-03-06), so remove them.

While at it, also simplify how the trailing '/' is appended to the variable holding the prefix path components.

---
 contrib/completion/git-completion.bash | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

index 36d3c6f928..72cd3add19 100644
@@ -385,7 +385,7 @@ __git_ls_files_helper ()
 #    slash.
 __git_index_files ()
 {
-	local root="${2-.}" file
+	local root="$2" file
 
 	__git_ls_files_helper "$root" "$1" |
 	while read -r file; do

@@ -406,13 +406,12 @@ __git_complete_index_file ()
 
 	case "$cur_" in
 	?*/*)
-		pfx="${cur_%/*}"
+		pfx="${cur_%/*}/"
 		cur_="${cur_##*/}"
-		pfx="${pfx}/"
 		;;
 	esac
 
-	__gitcomp_file "$(__git_index_files "$1" ${pfx:+"$pfx"})" "$pfx" "$cur_"
+	__gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
 }
 
 # Lists branches from the local repository.

2.17.0.366.gbe216a3084

Unless the user has 'core.quotePath=false' somewhere in the configuration, both 'git ls-files' and 'git diff-index' will by default quote any pathnames that contain bytes with values higher than 0x80, and escape those bytes as '\nnn' octal values. This prevents completing paths when the current path component to be completed contains any non-ASCII, most notably UTF-8, characters, because none of the listed quoted paths will match the current word on the command line.

Set 'core.quotePath=false' for those 'git ls-files' and 'git diff-index' invocations, so they won't consider bytes higher than 0x80 as "unusual", and won't quote pathnames containing such characters.

Note that pathnames containing backslash, double quote, or control characters will still be quoted; a later patch in this series will deal with those.

---
 contrib/completion/git-completion.bash | 6 ++++--
 t/t9902-completion.sh                  | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

index 72cd3add19..7072555960 100644
@@ -369,10 +369,12 @@ __gitcomp_file ()
 __git_ls_files_helper ()
 {
 	if [ "$2" == "--committable" ]; then
-		__git -C "$1" diff-index --name-only --relative HEAD
+		__git -C "$1" -c core.quotePath=false diff-index \
+			--name-only --relative HEAD
 	else
 		# NOTE: $2 is not quoted in order to support multiple options
-		__git -C "$1" ls-files --exclude-standard $2
+		__git -C "$1" -c core.quotePath=false ls-files \
+			--exclude-standard $2
 	fi
 }
 

index ff2e4a8f5f..a4f2c03b93 100755
@@ -1463,7 +1463,7 @@ test_expect_failure 'complete files - quoted characters on cmdline' '
 			"New(Dir/New)File.c"
 '
 
-test_expect_failure 'complete files - UTF-8 in ls-files output' '
+test_expect_success 'complete files - UTF-8 in ls-files output' '
 	test_when_finished "rm -r árvíztűrő" &&
 	mkdir árvíztűrő &&
 	>"árvíztűrő/Сайн яваарай" &&

2.17.0.366.gbe216a3084

Our git-aware path completion doesn't work when it has to complete a word already containing quoted and/or backslash-escaped characters on the command line. The root cause of the issue is that completion functions see all words on the command line verbatim, i.e. including all backslash, single and double quote characters that the shell would eventually remove when executing the finished command. These quoting/escaping characters cause different issues depending on which path component of the word to be completed contains them:

- The quoting/escaping is in the prefix path component(s).

Let's suppose we have a directory called 'New Dir', containing two untracked files 'file.c' and 'file.o', and we have a gitignore rule ignoring object files. In this case all of these:

git add New\ Dir/<TAB>
git add "New Dir/<TAB>
git add 'New Dir/<TAB>

should uniquely complete 'file.c' right away, but Bash offers both 'file.c' and 'file.o' instead. The reason for this behavior is that our completion script uses the prefix directory name like 'git -C "New\ Dir/" ls-files ...", i.e. with the backslash inside double quotes. Git then tries to enter a directory called 'New\ Dir', which (most likely) fails because such a directory doesn't exists. As a result our completion script doesn't list any files, leaves the COMPREPLY array empty, which in turn causes Bash to fall back to its simple filename completion and lists all files in that directory, i.e. both 'file.c' and 'file.o'.

- The quoting/escaping is in the path component to be completed.

Let's suppose we have two untracked files 'New File.c' and 'New File.o', and we have a gitignore rule ignoring object files. In this case all of these:

git add New\ Fi<TAB>
git add "New Fi<TAB>
git add 'New Fi<TAB>

should uniquely complete 'New File.c' right away, but Bash offers both 'New File.c' and 'New File.o' instead. The reason for this behavior is that our completion script uses this 'New\ Fi' or '"New Fi' etc. word to filter matching paths, and of course none of the potential filenames will match because of the included backslash or double quote. The end result is the same as above:
the completion script doesn't list any files, Bash falls back to its filename completion, which then lists the matching object file as well.

Add the new helper function __git_dequote() [1], which removes (most of[2]) the quoting and escaping from the word it gets as argument. To minimize the overhead of calling this function, store its result in the variable $dequoted_word, supposed to be declared local in the caller; simply printing the result would require a command substitution imposing the overhead of fork()ing a subshell. Use this function in __git_complete_index_file() to dequote the current word, i.e. the path, to be completed, to avoid the above described quoting-related issues, thereby fixing two of the failing quoted path completion tests.

[1] The bash-completion project already has a dequote() function, which I hoped I could borrow to deal with this, but unfortunately it doesn't work quite well for this purpose (perhaps that's why even the bash-completion project only rarely uses it). The main issue is that their dequote() is implemented as:

eval printf %s "$1" 2> /dev/null

where $1 would contain the word to be completed. While it's a short and sweet one-liner, the use of 'eval' requires that $1 is a syntactically valid string, which is not the case when quoting the path like 'git add "New Dir/<TAB>'. This causes 'eval' to fail, because it can't find the matching closing double quote, and the function returns nothing. The result is totally broken behavior, as if the current word were empty, and the completion script would then list all files from the current directory. This is why one of the quoted path completion tests specifically checks the completion of a path with an opening but without a corresponding closing double quote character. Furthermore, the 'eval' performs all kinds of expansions, which may or may not be desired; I think it's the latter. Finally, using this function would require a command substitution.

[2] Bash understands the $'string' quoting as well, which "expands to 'string', with backslash-escaped characters replaced as specified by the ANSI C standard" (quoted from Bash manpage). Since shell metacharacters, field separators, globbing, etc. can all be easily entered using standard shell escaping or quoting, this type of quoting comes in handly when dealing with control characters that are otherwise difficult both to "type" and to see on the command line. Because of this difficulty I would assume that people do avoid pathnames with such control characters anyway, so I didn't bother implementing it. This function is already way too long as it is.

---

Notes:
What if someone on Windows were to use backslash as directory separator during path completion? E.g. 'git add new-dir\subdir\<TAB>' Did that happen to work in the past? Does this patch break it?

 contrib/completion/git-completion.bash | 76 ++++++++++++++++++++++++--
 t/t9902-completion.sh                  | 46 +++++++++++++++-
 2 files changed, 116 insertions(+), 6 deletions(-)

index 7072555960..ae6127155e 100644
@@ -92,6 +92,70 @@ __git ()
 		${__git_dir:+--git-dir="$__git_dir"} "$@" 2>/dev/null
 }
 
+# Removes backslash escaping, single quotes and double quotes from a word,
+# stores the result in the variable $dequoted_word.
+# 1: The word to dequote.
+__git_dequote ()
+{
+	local rest="$1" len ch
+
+	dequoted_word=""
+
+	while test -n "$rest"; do
+		len=${#dequoted_word}
+		dequoted_word="$dequoted_word${rest%%[\\\'\"]*}"
+		rest="${rest:$((${#dequoted_word}-$len))}"
+
+		case "${rest:0:1}" in
+		\\)
+			ch="${rest:1:1}"
+			case "$ch" in
+			$'\n')
+				;;
+			*)
+				dequoted_word="$dequoted_word$ch"
+				;;
+			esac
+			rest="${rest:2}"
+			;;
+		\')
+			rest="${rest:1}"
+			len=${#dequoted_word}
+			dequoted_word="$dequoted_word${rest%%\'*}"
+			rest="${rest:$((${#dequoted_word}-$len+1))}"
+			;;
+		\")
+			rest="${rest:1}"
+			while test -n "$rest" ; do
+				len=${#dequoted_word}
+				dequoted_word="$dequoted_word${rest%%[\\\"]*}"
+				rest="${rest:$((${#dequoted_word}-$len))}"
+				case "${rest:0:1}" in
+				\\)
+					ch="${rest:1:1}"
+					case "$ch" in
+					\"|\\|\$|\`)
+						dequoted_word="$dequoted_word$ch"
+						;;
+					$'\n')
+						;;
+					*)
+						dequoted_word="$dequoted_word\\$ch"
+						;;
+					esac
+					rest="${rest:2}"
+					;;
+				\")
+					rest="${rest:1}"
+					break
+					;;
+				esac
+			done
+			;;
+		esac
+	done
+}
+
 # The following function is based on code from:
 #
 #   bash_completion - programmable completion functions for bash 3.2+

@@ -404,13 +468,17 @@ __git_index_files ()
 # The exception is --committable, which finds the files appropriate commit.
 __git_complete_index_file ()
 {
-	local pfx="" cur_="$cur"
+	local dequoted_word pfx="" cur_
 
-	case "$cur_" in
+	__git_dequote "$cur"
+
+	case "$dequoted_word" in
 	?*/*)
-		pfx="${cur_%/*}/"
-		cur_="${cur_##*/}"
+		pfx="${dequoted_word%/*}/"
+		cur_="${dequoted_word##*/}"
 		;;
+	*)
+		cur_="$dequoted_word"
 	esac
 
 	__gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"

2.17.0.366.gbe216a3084

During git-aware path completion, e.g. 'git rm dir/fil<TAB>', both 'git ls-files' and 'git diff-index' list all paths in the given 'dir/' matching certain criteria (cached, modified, untracked, etc.) appropriate for the given git command, even paths whose names don't begin with 'fil'. This comes with a considerable performance penalty when the directory in question contains a lot of paths, but the current word can be uniquely completed or when only a handful of those paths match the current word.

Reduce the number of iterations in this codepath from the number of paths to the number of matching paths by specifying an appropriate globbing pattern to 'git ls-files' and 'git diff-index' to list only paths that match the current word to be completed.

Note that both commands treat backslashes as escape characters in their file arguments, e.g. to preserve the literal meaning of globbing characters, so we have to double every backslash in the globbing pattern. This is why one of the path completion tests specifically checks the completion of a path containing a literal backslash character (that test still fails, though, because both commands output such paths enclosed in double quotes and the special characters escaped; a later patch in this series will deal with those).

This speeds up path completion considerably when there are a lot of non-matching paths to be filtered out. Uniquely completing a tracked filename at the top of the worktree in linux.git (over 62k files), i.e. what's doing all the hard work behind 'git rm Mak<TAB>' to complete 'Makefile':

Before this patch, best of five, on Linux:

    $ time cur=Mak __git_complete_index_file

    real    0m2.159s
    user    0m1.299s
    sys     0m1.089s

After:

    real    0m0.033s
    user    0m0.023s
    sys     0m0.015s

Difference: -98.5%
Speedup: 65.4x

---
 contrib/completion/git-completion.bash | 11 ++++++-----
 t/t9902-completion.sh                  |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

index ae6127155e..3948265d32 100644
@@ -434,11 +434,11 @@ __git_ls_files_helper ()
 {
 	if [ "$2" == "--committable" ]; then
 		__git -C "$1" -c core.quotePath=false diff-index \
-			--name-only --relative HEAD
+			--name-only --relative HEAD -- "${3//\\/\\\\}*"
 	else
 		# NOTE: $2 is not quoted in order to support multiple options
 		__git -C "$1" -c core.quotePath=false ls-files \
-			--exclude-standard $2
+			--exclude-standard $2 -- "${3//\\/\\\\}*"
 	fi
 }
 

@@ -449,11 +449,12 @@ __git_ls_files_helper ()
 #    If provided, only files within the specified directory are listed.
 #    Sub directories are never recursed.  Path must have a trailing
 #    slash.
+# 3: List only paths matching this path component (optional).
 __git_index_files ()
 {
-	local root="$2" file
+	local root="$2" match="$3" file
 
-	__git_ls_files_helper "$root" "$1" |
+	__git_ls_files_helper "$root" "$1" "$match" |
 	while read -r file; do
 		case "$file" in
 		?*/*) echo "${file%%/*}" ;;

@@ -481,7 +482,7 @@ __git_complete_index_file ()
 		cur_="$dequoted_word"
 	esac
 
-	__gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
+	__gitcomp_file "$(__git_index_files "$1" "$pfx" "$cur_")" "$pfx" "$cur_"
 }
 
 # Lists branches from the local repository.

index 6856b263f8..f7a9dd446d 100755
@@ -1515,6 +1515,7 @@ test_expect_success 'complete files - UTF-8 in ls-files output' '
 			"árvíztűrő/Сайн яваарай"
 '
 
+# Testing with a path containing a backslash is important.
 if test_have_prereq !MINGW &&
    mkdir 'New\Dir' 2>/dev/null &&
    touch 'New\Dir/New"File.c' 2>/dev/null

2.17.0.366.gbe216a3084

During git-aware path completion we complete one path component at a time, i.e. 'git add <TAB>' offers only 'dir/' at first, not 'dir/subdir/file' right away, just like Bash's own filename completion. However, since both 'git ls-files' and 'git diff-index' dive deep into subdirectories, we have to strip all trailing path components from the listed paths, keeping only the leading path component. This stripping is currently done in a shell loop in __git_index_files(), which can take a significant amount of time when it has to iterate through a large number of paths.

Replace this shell loop with a little 'awk' script using '/' as input field separator and printing the first field, which produces the same output much faster.

Listing all tracked files (12) and directories (23) at the top of the worktree in linux.git (over 62k files), i.e. what's doing all the hard work behind 'git rm <TAB>':

Before this patch, best of five, using GNU awk on Linux:

    $ time cur= __git_complete_index_file

    real    0m2.149s
    user    0m1.307s
    sys     0m1.086s

After:

    real    0m0.067s
    user    0m0.089s
    sys     0m0.023s

Difference: -96.9%
Speedup: 32.1x

Note that this could be done with 'sed', or even with 'cut', just as well, but the upcoming patches require 'awk's scriptability.

Note also that this change means one more fork()+exec()ed process during path completion, adding more overhead especially on Windows, but a later patch will more than make up for it by eliminating two other processes in the same function.

---
 contrib/completion/git-completion.bash | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

index 3948265d32..0abba88462 100644
@@ -452,15 +452,12 @@ __git_ls_files_helper ()
 # 3: List only paths matching this path component (optional).
 __git_index_files ()
 {
-	local root="$2" match="$3" file
+	local root="$2" match="$3"
 
 	__git_ls_files_helper "$root" "$1" "$match" |
-	while read -r file; do
-		case "$file" in
-		?*/*) echo "${file%%/*}" ;;
-		*) echo "$file" ;;
-		esac
-	done | sort | uniq
+	awk -F / '{
+		print $1
+	}' | sort | uniq
 }
 
 # __git_complete_index_file requires 1 argument:

2.17.0.366.gbe216a3084

The order or possible completion words in the COMPREPLY array doesn't actually matter, as long as all the right words are in there, because Bash will sort them anyway. Yet, our tests looking at the elements of COMPREPLY always expect them to be in a specific order.

Now, this hasn't been an issue before, but the next patch is about to optimize a bit more our git-aware path completion, and as a harmless side effect the order of elements in COMPREPLY will change. Worse, the order will be downright undefined, because after the next patch path components will come directly from iterating through an associative array in 'awk', and the order of iteration over the elements in those arrays is undefined, and indeed different 'awk' implementations produce different order. Consequently, we can't get away with simply adjusting the expected results in the affected tests.

Modify the 'test_completion' helper function to sort both the expected and the actual results, i.e. the elements in COMPREPLY, before comparing them, so the tests using this helper function will work regardless of the order of elements.

Note that this change still leaves a bunch of tests depending on the order of elements in COMPREPLY, tests that focus on a specific helper function and therefore don't use the 'test_completion' helper. I would rather deal with those later, when (if ever) the need actually arises, than create unnecessary code churn now.

---
 t/t9902-completion.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index f7a9dd446d..a747998d6c 100755
@@ -84,10 +84,11 @@ test_completion ()
 	then
 		printf '%s\n' "$2" >expected
 	else
-		sed -e 's/Z$//' >expected
+		sed -e 's/Z$//' |sort >expected
 	fi &&
 	run_completion "$1" &&
-	test_cmp expected out
+	sort out >out_sorted &&
+	test_cmp expected out_sorted
 }
 
 # Test __gitcomp.

@@ -1405,6 +1406,7 @@ test_expect_success 'complete files' '
 
 	echo "expected" > .gitignore &&
 	echo "out" >> .gitignore &&
+	echo "out_sorted" >> .gitignore &&
 
 	git add .gitignore &&
 	test_completion "git commit " ".gitignore" &&

2.17.0.366.gbe216a3084

During git-aware path completion, after all the trailing path components have been removed from the output of 'git ls-files' and 'git diff-index' (see previous patch), each directory name is repeated as many times as the number of listed paths it contains. This can be a lot of repetitions, especially when invoking path completion close to the root of a big worktree, which would cause a considerable overhead downstream of __git_index_files(), in particular in the shell loop that fills the COMPREPLY array. To reduce this overhead, __git_index_files() runs the classic '... |sort |uniq' pattern to remove those repetitions from the function's output.

While removing repeated directory names is effective in reducing the number of iterations in that shell loop, it still imposes the overhead of fork()+exec()ing two external processes, and two additional stages in the pipeline, where potentially relatively large amount of data can be passed between two subsequent pipeline stages.

Extend __git_index_files()'s 'awk' script to remove repeated path components by first creating and filling an associative array indexed by all encountered path components (after the trailing path components have been removed), and then iterating over this array and printing the indices, i.e. unique path components. This way we can remove the '|sort |uniq' pipeline stages, and their eliminated overhead results in faster path completion.

Listing all tracked files (12) and directories (23) at the top of the worktree in linux.git (over 62k files), i.e. what's doing all the hard work behind 'git rm <TAB>':

Before this patch, best of five, using GNU awk on Linux:

    real    0m0.069s
    user    0m0.089s
    sys     0m0.026s

After:

    real    0m0.052s
    user    0m0.072s
    sys     0m0.014s

Difference: -24.6%

Note that this changes order of elements in __git_index_files()'s output. This is not an issue, because this function was only ever intended to feed paths into the COMPREPLY array, and Bash will sort its elements (according to the users locale) anyway.

Note also that using 'awk' to remove repeated path components is also beneficial for the performance of the next two patches:

- The first will extend this 'awk' script to dequote quoted paths in the output of 'git ls-files' and 'git diff-index'. With this patch it will only have to dequote unique path components, not all.

- The second will, among other things, extend this 'awk' script to prepend prefix path components from the command line to the currently completed path component. Consequently, each line in 'awk's output will grow longer. Without this patch that '|sort |uniq' would have to exchange and process that much more data.

---
 contrib/completion/git-completion.bash | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index 0abba88462..70bc75dfc7 100644
@@ -456,8 +456,12 @@ __git_index_files ()
 
 	__git_ls_files_helper "$root" "$1" "$match" |
 	awk -F / '{
-		print $1
-	}' | sort | uniq
+		paths[$1] = 1
+	}
+	END {
+		for (p in paths)
+			print p
+	}'
 }
 
 # __git_complete_index_file requires 1 argument:

2.17.0.366.gbe216a3084

If any pathname contains backslash, double quote, tab, newline, or any control characters, 'git ls-files' and 'git diff-index' will enclose that pathname in double quotes and escape those special characters using C-style one-character escape sequences or \nnn octal values. This prevents those files from being listed during git-aware path completion, because due to the quoting they will never match the current word to be completed.

Extend __git_index_files()'s 'awk' script to remove all that quoting and escaping from unique path components, so even paths containing (almost all) such special characters can be completed.

Paths containing newline characters are still an issue, though. We use newlines as separator character when filling the COMPREPLY array, so a path with one or more newline will end up split to two or more elements in COMPREPLY, basically breaking completion. There is nothing we can do about it without a significant performance hit, so let's just ignore such paths for now. As far as paths with newlines are concerned, this isn't any different from the previous behavior, because those paths were always omitted, though in the past they were omitted because due to the quoting they didn't match the current word to be completed. Anyway, Bash's own filename completion (Meta-/) can complete even those paths, if need be.

Note:

- We don't dequote path components right away as they are coming in, because then we would have to dequote each directory name repeatedly, as many times as it appears in the input, i.e. as many times as the number of listed paths it contains. Instead, we dequote them at the end, as we print unique path components.

- Even when a directory name itself does not contain any special characters, it will still be quoted if any of its trailing path components do. If a directory contains paths both with and without special characters, then the name of that directory will appear both quoted and unquoted in the output of 'git ls-files' and 'git diff-index'. Consequently, we will add such a directory name to the deduplicating associative array twice: once quoted and once unquoted.

This means that we have to be careful after dequoting a directory name, and only print it if we haven't seen the same directory name unquoted.

- It would be wonderful if we could just pass '-z' to those git commands to output \0-separated unquoted paths, and use \0 as record separator in the 'awk' script processing their output... this patch would be so much simpler, almost trivial even. Unfortunately, however, POSIX and most 'awk' implementations don't support \0 as record separator (GNU awk does support it).

- This patch makes the earlier change to list paths with 'core.quotePath=false' basically redundant, because this could decode any \nnn-escaped non-ASCII character just fine, as well. However, I suspect that 'git ls-files' can deal with those non-ASCII characters faster than this updated 'awk' script; just in case someone is burdened with tons of pathnames containing non-ASCII characters.

---
 contrib/completion/git-completion.bash | 66 +++++++++++++++++++++++++-
 t/t9902-completion.sh                  | 17 ++++++-
 2 files changed, 79 insertions(+), 4 deletions(-)

index 70bc75dfc7..e97d57024b 100644
@@ -459,8 +459,70 @@ __git_index_files ()
 		paths[$1] = 1
 	}
 	END {
-		for (p in paths)
-			print p
+		for (p in paths) {
+			if (substr(p, 1, 1) != "\"") {
+				# No special characters, easy!
+				print p
+				continue
+			}
+
+			# The path is quoted.
+			p = dequote(p)
+			if (p == "")
+				continue
+
+			# Even when a directory name itself does not contain
+			# any special characters, it will still be quoted if
+			# any of its (stripped) trailing path components do.
+			# Because of this we may have seen the same direcory
+			# both quoted and unquoted.
+			if (p in paths)
+				# We have seen the same directory unquoted,
+				# skip it.
+				continue
+			else
+				print p
+		}
+	}
+	function dequote(p,    bs_idx, out, esc, esc_idx, dec) {
+		# Skip opening double quote.
+		p = substr(p, 2)
+
+		# Interpret backslash escape sequences.
+		while ((bs_idx = index(p, "\\")) != 0) {
+			out = out substr(p, 1, bs_idx - 1)
+			esc = substr(p, bs_idx + 1, 1)
+			p = substr(p, bs_idx + 2)
+
+			if ((esc_idx = index("abtvfr\"\\", esc)) != 0) {
+				# C-style one-character escape sequence.
+				out = out substr("\a\b\t\v\f\r\"\\",
+						 esc_idx, 1)
+			} else if (esc == "n") {
+				# Uh-oh, a newline character.
+				# We cant reliably put a pathname
+				# containing a newline into COMPREPLY,
+				# and the newline would create a mess.
+				# Skip this path.
+				return ""
+			} else {
+				# Must be a \nnn octal value, then.
+				dec = esc             * 64 + \
+				      substr(p, 1, 1) * 8  + \
+				      substr(p, 2, 1)
+				out = out sprintf("%c", dec)
+				p = substr(p, 3)
+			}
+		}
+		# Drop closing double quote, if there is one.
+		# (There isnt any if this is a directory, as it was
+		# already stripped with the trailing path components.)
+		if (substr(p, length(p), 1) == "\"")
+			out = out substr(p, 1, length(p) - 1)
+		else
+			out = out p
+
+		return out
 	}'
 }
 

index a747998d6c..b9879576ab 100755
@@ -1527,7 +1527,7 @@ else
 	say "Your filesystem does not allow \\ and \" in filenames."
 	rm -rf 'New\Dir'
 fi
-test_expect_failure FUNNYNAMES_BS_DQ \
+test_expect_success FUNNYNAMES_BS_DQ \
     'complete files - C-style escapes in ls-files output' '
 	test_when_finished "rm -r \"New\\\\Dir\"" &&
 

@@ -1548,7 +1548,7 @@ else
 	say 'Your filesystem does not allow special separator characters (FS, GS, RS, US) in filenames.'
 	rm -rf $'New\034Special\035Dir'
 fi
-test_expect_failure FUNNYNAMES_SEPARATORS \
+test_expect_success FUNNYNAMES_SEPARATORS \
     'complete files - \nnn-escaped control characters in ls-files output' '
 	test_when_finished "rm -r '$'New\034Special\035Dir''" &&
 

@@ -1562,6 +1562,19 @@ test_expect_failure FUNNYNAMES_SEPARATORS \
 			"'$'New\034Special\035Dir/New\036Special\037File''"
 '
 
+test_expect_success FUNNYNAMES_BS_DQ \
+    'complete files - removing repeated quoted path components' '
+	test_when_finished rm -rf NewDir &&
+	mkdir NewDir &&    # A dirname which in itself would not be quoted ...
+	>NewDir/0-file &&
+	>NewDir/1\"file && # ... but here the file makes the dirname quoted ...
+	>NewDir/2-file &&
+	>NewDir/3\"file && # ... and here, too.
+
+	# Still, we should only list it once.
+	test_completion "git test-path-comp New" "NewDir"
+'
+
 
 test_expect_success "completion uses <cmd> completion for alias: !sh -c 'git <cmd> ...'" '
 	test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" &&

2.17.0.366.gbe216a3084

During git-aware path completion, when a lot of path components have to be listed, a significant amount of time is spent in __gitcomp_file(), or more accurately in the shell loop of __gitcompappend(), iterating over all the path components filtering path components matching the current word to be completed, adding prefix path components, and placing the resulting matching paths into the COMPREPLY array.

Now, a previous patch in this series made 'git ls-files' and 'git diff-index' list only paths matching the current word to be completed, so an additional filtering in __gitcomp_file() is not necessary anymore. Adding the prefix path components could be done much more efficiently in __git_index_files()'s 'awk' script while stripping trailing path components and removing duplicates and quoting. And then the resulting paths won't require any more filtering or processing before being handed over to Bash, so we could fill the COMPREPLY array directly.

Unfortunately, we can't simply use the __gitcomp_direct() helper function to do that, because __gitcomp_file() does one additional
thing: it tells Bash that we are doing filename completion, so the shell will kindly do four important things for us:

1. Append a trailing space to all filenames.
2. Append a trailing '/' to all directory names.
3. Escape any meta, globbing, separator, etc. characters.
4. List only the current path component when listing possible

     completions (i.e. 'dir/subdir/f<TAB>' will list 'file1', 'file2',
     etc. instead of the whole 'dir/subdir/file1',
     'dir/subdir/file2').

While we could let __git_index_files()'s 'awk' script take care of the first two points, the third one gets tricky, and we absolutely need the shell's support for the fourth.

Add the helper function __gitcomp_file_direct(), which, just like __gitcomp_direct(), fills the COMPREPLY array with prefiltered and preprocessed paths without any additional processing, without a shell loop, with just one single compound assignment, and, similar to __gitcomp_file(), tells Bash and ZSH that we are doing filename completion. Extend __git_index_files()'s 'awk' script a bit to prepend any prefix path components to all listed paths. Finally, modify __git_complete_index_file() to feed __git_index_files()'s output to ___gitcomp_file_direct() instead of __gitcomp_file().

After this patch there is no shell loop left in the path completion code path.

This speeds up path completion when there are a lot of paths matching the current word to be completed. In a pathological repository with 100k files in a single directory, listing all those files:

Before this patch, best of five, using GNU awk on Linux:

    $ time cur=dir/ __git_complete_index_file

    real    0m0.983s
    user    0m1.004s
    sys     0m0.033s

After:

    real    0m0.313s
    user    0m0.341s
    sys     0m0.029s

Difference: -68.2%
Speedup: 3.1x

To see the benefits of the whole patch series, the same command with v2.17.0:

    real    0m2.736s
    user    0m2.472s
    sys     0m0.610s

Difference: -88.6%
Speedup: 8.7x

Note that this patch changes the output of the __git_index_files() helper function by unconditionally prepending the prefix path components to every listed path. This would break users' completion scriptlets that directly run:

  __gitcomp_file "$(__git_index_files ...)" "$pfx" "$cur_"

because that would add the prefix path components once more. However, __git_index_files() is kind of a "helper function of a helper function", and users' completion scriptlets should have been using __git_complete_index_file() for git-aware path completion in the first place, so this is likely doesn't worth worrying about.

---
 contrib/completion/git-completion.bash | 34 +++++++++++++++++++++++---
 contrib/completion/git-completion.zsh  |  9 +++++++
 2 files changed, 39 insertions(+), 4 deletions(-)

index e97d57024b..360ee9ca51 100644
@@ -404,6 +404,23 @@ __gitcomp_nl ()
 	__gitcomp_nl_append "$@"
 }
 
+# Fills the COMPREPLY array with prefiltered paths without any additional
+# processing.
+# Callers must take care of providing only paths that match the current path
+# to be completed and adding any prefix path components, if necessary.
+# 1: List of newline-separated matching paths, complete with all prefix
+#    path componens.
+__gitcomp_file_direct ()
+{
+	local IFS=$'\n'
+
+	COMPREPLY=($1)
+
+	# use a hack to enable file mode in bash < 4
+	compopt -o filenames +o nospace 2>/dev/null ||
+	compgen -f /non-existing-dir/ > /dev/null
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:

@@ -455,14 +472,14 @@ __git_index_files ()
 	local root="$2" match="$3"
 
 	__git_ls_files_helper "$root" "$1" "$match" |
-	awk -F / '{
+	awk -F / -v pfx="${2//\\/\\\\}" '{
 		paths[$1] = 1
 	}
 	END {
 		for (p in paths) {
 			if (substr(p, 1, 1) != "\"") {
 				# No special characters, easy!
-				print p
+				print pfx p
 				continue
 			}
 

@@ -481,7 +498,7 @@ __git_index_files ()
 				# skip it.
 				continue
 			else
-				print p
+				print pfx p
 		}
 	}
 	function dequote(p,    bs_idx, out, esc, esc_idx, dec) {

@@ -545,7 +562,7 @@ __git_complete_index_file ()
 		cur_="$dequoted_word"
 	esac
 
-	__gitcomp_file "$(__git_index_files "$1" "$pfx" "$cur_")" "$pfx" "$cur_"
+	__gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_")"
 }
 
 # Lists branches from the local repository.

@@ -3311,6 +3328,15 @@ if [[ -n ${ZSH_VERSION-} ]]; then
 		compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 	}
 
+	__gitcomp_file_direct ()
+	{
+		emulate -L zsh
+
+		local IFS=$'\n'
+		compset -P '*[=:]'
+		compadd -Q -f -- ${=1} && _ret=0
+	}
+
 	__gitcomp_file ()
 	{
 		emulate -L zsh

index c3521fbfc4..53cb0f934f 100644
@@ -93,6 +93,15 @@ __gitcomp_nl_append ()
 	compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 }
 
+__gitcomp_file_direct ()
+{
+	emulate -L zsh
+
+	local IFS=$'\n'
+	compset -P '*[=:]'
+	compadd -Q -f -- ${=1} && _ret=0
+}
+
 __gitcomp_file ()
 {
 	emulate -L zsh

2.17.0.366.gbe216a3084

Hi drizzd,

first of all: thank you so much for working on this. I am sure it will be noticeable to many Windows users, and also make my life easier.

Technically, it is not the *mingw64* bash, but it is an MSYS2 Bash. This does make a little bit of a difference because of the penalty incurred by the POSIX emulation layer provided by the MSYS2 runtime.

(And it also addresses Gabór's question whether you ran the test suite, I guess... it takes multiple hours to run it even once on a regular computer.)

Ciao,
Dscho