From 5bb534a620a1e7424e80cc89c53f9e3573fba0bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:05 +0200 Subject: t9902-completion: add tests demonstrating issues with quoted pathnames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 1b34caa..98a672c 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -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 completion for alias: !sh -c 'git ...'" ' test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" && test_completion "git co m" <<-\EOF -- cgit v0.10.2-6-g49f6 From 722e31c713a2f33dc483c0f17f208b2260faebe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:06 +0200 Subject: completion: move __git_complete_index_file() next to its helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index a757073..a87a8b2 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -398,6 +398,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 @@ -714,26 +733,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 -- cgit v0.10.2-6-g49f6 From 6bf0ced4e21bfc757641ec537b8bb9dd63ece904 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:07 +0200 Subject: completion: simplify prefix path component handling during path completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ''" as a no-op when 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index a87a8b2..57fc741 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -387,7 +387,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 @@ -408,13 +408,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. -- cgit v0.10.2-6-g49f6 From 3dfe23ba51664467d89ba937e607ffef6501c3f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:08 +0200 Subject: completion: support completing non-ASCII pathnames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 57fc741..2a8fe2a 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -371,10 +371,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 } diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 98a672c..f7d7bec 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -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ő/Сайн яваарай" && -- cgit v0.10.2-6-g49f6 From f12785a3a735c01e0d17e21f063c21ebe8bda200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:09 +0200 Subject: completion: improve handling quoted paths on the command line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/ git add "New Dir/ git add 'New Dir/ 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 git add "New Fi git add 'New Fi 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/'. 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 2a8fe2a..cdcf8b9 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -94,6 +94,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+ @@ -406,13 +470,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_" diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index f7d7bec..f8fceda 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -400,6 +400,46 @@ test_expect_success '__gitdir - remote as argument' ' test_cmp expected "$actual" ' + +test_expect_success '__git_dequote - plain unquoted word' ' + __git_dequote unquoted-word && + verbose test unquoted-word = "$dequoted_word" +' + +# input: b\a\c\k\'\\\"s\l\a\s\h\es +# expected: back'\"slashes +test_expect_success '__git_dequote - backslash escaped' ' + __git_dequote "b\a\c\k\\'\''\\\\\\\"s\l\a\s\h\es" && + verbose test "back'\''\\\"slashes" = "$dequoted_word" +' + +# input: sin'gle\' '"quo'ted +# expected: single\ "quoted +test_expect_success '__git_dequote - single quoted' ' + __git_dequote "'"sin'gle\\\\' '\\\"quo'ted"'" && + verbose test '\''single\ "quoted'\'' = "$dequoted_word" +' + +# input: dou"ble\\" "\"\quot"ed +# expected: double\ "\quoted +test_expect_success '__git_dequote - double quoted' ' + __git_dequote '\''dou"ble\\" "\"\quot"ed'\'' && + verbose test '\''double\ "\quoted'\'' = "$dequoted_word" +' + +# input: 'open single quote +test_expect_success '__git_dequote - open single quote' ' + __git_dequote "'\''open single quote" && + verbose test "open single quote" = "$dequoted_word" +' + +# input: "open double quote +test_expect_success '__git_dequote - open double quote' ' + __git_dequote "\"open double quote" && + verbose test "open double quote" = "$dequoted_word" +' + + test_expect_success '__gitcomp_direct - puts everything into COMPREPLY as-is' ' sed -e "s/Z$//g" >expected <<-EOF && with-trailing-space Z @@ -1437,7 +1477,7 @@ _git_test_path_comp () __git_complete_index_file --others } -test_expect_failure 'complete files - escaped characters on cmdline' ' +test_expect_success 'complete files - escaped characters on cmdline' ' test_when_finished "rm -rf \"New|Dir\"" && mkdir "New|Dir" && >"New|Dir/New&File.c" && @@ -1453,11 +1493,13 @@ test_expect_failure 'complete files - escaped characters on cmdline' ' "New|Dir/New&File.c" ' -test_expect_failure 'complete files - quoted characters on cmdline' ' +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" -- cgit v0.10.2-6-g49f6 From a364e984d1db4dddf5ddce494ed96db437bc025e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:10 +0200 Subject: completion: let 'ls-files' and 'diff-index' filter matching paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During git-aware path completion, e.g. 'git rm dir/fil', 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' 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 Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index cdcf8b9..a46c176 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -436,11 +436,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 } @@ -451,11 +451,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%%/*}" ;; @@ -483,7 +484,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. diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index f8fceda..562c88e 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -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 -- cgit v0.10.2-6-g49f6 From 105c0efff335bc813fe7bf970fbdf1b92fc49391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:11 +0200 Subject: completion: use 'awk' to strip trailing path components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During git-aware path completion we complete one path component at a time, i.e. 'git add ' 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 ': 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index a46c176..a71db47 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -454,15 +454,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: -- cgit v0.10.2-6-g49f6 From 9703797c9d6b4d1831bf32bc0ab721a9411c716a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:12 +0200 Subject: t9902-completion: ignore COMPREPLY element order in some tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 562c88e..9d46076 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -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" && -- cgit v0.10.2-6-g49f6 From c1bc0a0e927265c38aa6e4cf1c9b9e7866225b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:41:13 +0200 Subject: completion: remove repeated dirnames with 'awk' during path completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ': 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index a71db47..1cd019d 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -458,8 +458,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: -- cgit v0.10.2-6-g49f6 From 193757f8062f7b9a1e2f1137be4c6f720d74c4f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:42:35 +0200 Subject: completion: improve handling quoted paths in 'git ls-files's output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 1cd019d..63903f2 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -461,8 +461,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 }' } diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 9d46076..9559321 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -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 completion for alias: !sh -c 'git ...'" ' test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" && -- cgit v0.10.2-6-g49f6 From 7b0034206843c38dc96e700a527e06b88ac35d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 17 Apr 2018 00:42:36 +0200 Subject: completion: fill COMPREPLY directly when completing paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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' 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 63903f2..816901f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -406,6 +406,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: @@ -457,14 +474,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 } @@ -483,7 +500,7 @@ __git_index_files () # skip it. continue else - print p + print pfx p } } function dequote(p, bs_idx, out, esc, esc_idx, dec) { @@ -547,7 +564,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. @@ -3348,6 +3365,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 diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh index c3521fb..53cb0f9 100644 --- a/contrib/completion/git-completion.zsh +++ b/contrib/completion/git-completion.zsh @@ -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 -- cgit v0.10.2-6-g49f6 From 8b4c2e0b1c4ff47c07b2c1bbc937bb46e889755e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Fri, 18 May 2018 16:17:50 +0200 Subject: completion: don't return with error from __gitcomp_file_direct() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 816901f..8bc79a5 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -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 -- cgit v0.10.2-6-g49f6 From 7d314073488ae81b8f5cdecb4d00a78529fa7dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Fri, 18 May 2018 16:17:51 +0200 Subject: t9902-completion: exercise __git_complete_index_file() directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 tests 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. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 9559321..1b6d275 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -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 completion for alias: !sh -c 'git ...'" ' test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" && test_completion "git co m" <<-\EOF -- cgit v0.10.2-6-g49f6