{"thread":{"id":"48065","subject":"[PATCH 1/2] completion: improve ls-files filter performance","startedAt":"2018-03-17T08:17:51Z","lastAt":"2018-05-21T12:17:53Z","messageCount":36,"participants":["Clemens Buchacher","Junio C Hamano","SZEDER Gábor","Johannes Schindelin","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"342012","messageId":"1521274624-1370-1-git-send-email-drizzd@gmx.net","threadId":"48065","inReplyTo":null,"subject":"[PATCH 1/2] completion: improve ls-files filter performance","fromName":"Clemens Buchacher","fromEmail":"drizzd@gmx.net","sentAt":"2018-03-17T08:17:03Z","receivedAt":"2018-03-17T08:17:51Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"From the output of ls-files, we remove all but the leftmost path\ncomponent and then we eliminate duplicates. We do this in a while loop,\nwhich is a performance bottleneck when the number of iterations is large\n(e.g. for 60000 files in linux.git).\n\n$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n\nreal 0m11.876s\nuser 0m4.685s\nsys 0m6.808s\n\nUsing an equivalent sed script improves performance significantly:\n\n$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n\nreal 0m1.372s\nuser 0m0.263s\nsys 0m0.167s\n\nThe measurements were done with mingw64 bash, which is used by Git for\nWindows.\n\nSigned-off-by: Clemens Buchacher \n---\n contrib/completion/git-completion.bash | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 6da95b8..e3ddf27 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -384,12 +384,7 @@ __git_index_files ()\n \tlocal root=\"${2-.}\" file\n \n \t__git_ls_files_helper \"$root\" \"$1\" |\n-\twhile read -r file; do\n-\t\tcase \"$file\" in\n-\t\t?*/*) echo \"${file%%/*}\" ;;\n-\t\t*) echo \"$file\" ;;\n-\t\tesac\n-\tdone | sort | uniq\n+\tsed -e '/^\\//! s#/.*##' | sort | uniq\n }\n \n # Lists branches from the local repository.\n-- \n2.7.4\n\n"},{"id":"342013","messageId":"1521274624-1370-2-git-send-email-drizzd@gmx.net","threadId":"48065","inReplyTo":"1521274624-1370-1-git-send-email-drizzd@gmx.net","subject":"[PATCH 2/2] completion: simplify ls-files filter","fromName":"Clemens Buchacher","fromEmail":"drizzd@gmx.net","sentAt":"2018-03-17T08:17:04Z","receivedAt":"2018-03-17T08:17:55Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"When filtering the ls-files output we take care not to touch absolute\npaths. This is redundant, because ls-files will never output absolute\npaths. Furthermore, sorting the output is also redundant, because the\noutput of ls-files is already sorted.\n\nRemove the unnecessary operations.\n\nSigned-off-by: Clemens Buchacher \n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex e3ddf27..394c3df 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -384,7 +384,7 @@ __git_index_files ()\n \tlocal root=\"${2-.}\" file\n \n \t__git_ls_files_helper \"$root\" \"$1\" |\n-\tsed -e '/^\\//! s#/.*##' | sort | uniq\n+\tcut -f1 -d/ | uniq\n }\n \n # Lists branches from the local repository.\n-- \n2.7.4\n\n"},{"id":"342066","messageId":"xmqqo9jmxmt4.fsf@gitster-ct.c.googlers.com","threadId":"48065","inReplyTo":"1521274624-1370-1-git-send-email-drizzd@gmx.net","subject":"Re: [PATCH 1/2] completion: improve ls-files filter performance","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-18T00:13:59Z","receivedAt":"2018-03-18T00:14:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> From the output of ls-files, we remove all but the leftmost path\n> component and then we eliminate duplicates. We do this in a while loop,\n> which is a performance bottleneck when the number of iterations is large\n> (e.g. for 60000 files in linux.git).\n>\n> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n>\n> real 0m11.876s\n> user 0m4.685s\n> sys 0m6.808s\n>\n> Using an equivalent sed script improves performance significantly:\n>\n> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n>\n> real 0m1.372s\n> user 0m0.263s\n> sys 0m0.167s\n>\n> The measurements were done with mingw64 bash, which is used by Git for\n> Windows.\n>\n> Signed-off-by: Clemens Buchacher \n> ---\n> contrib/completion/git-completion.bash | 7 +------\n> 1 file changed, 1 insertion(+), 6 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 6da95b8..e3ddf27 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -384,12 +384,7 @@ __git_index_files ()\n> \tlocal root=\"${2-.}\" file\n> \n> \t__git_ls_files_helper \"$root\" \"$1\" |\n> -\twhile read -r file; do\n> -\t\tcase \"$file\" in\n> -\t\t?*/*) echo \"${file%%/*}\" ;;\n> -\t\t*) echo \"$file\" ;;\n> -\t\tesac\n> -\tdone | sort | uniq\n> +\tsed -e '/^\\//! s#/.*##' | sort | uniq\n\nMicronit: perhaps lose SP after '!'?\n\ncf. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n\n\"\"\"A function can be preceded by a '!' character, in which case the\nfunction shall be applied if the addresses do not select the pattern\nspace. Zero or more characters shall be accepted before the\n'!' character. It is unspecified whether characters can\nfollow the '!' character, and conforming applications shall not\nfollow the '!' character with characters.\"\"\"\n\n\n> }\n> \n> # Lists branches from the local repository.\n"},{"id":"342067","messageId":"xmqqk1uaxmor.fsf@gitster-ct.c.googlers.com","threadId":"48065","inReplyTo":"1521274624-1370-2-git-send-email-drizzd@gmx.net","subject":"Re: [PATCH 2/2] completion: simplify ls-files filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-18T00:16:36Z","receivedAt":"2018-03-18T00:16:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> When filtering the ls-files output we take care not to touch absolute\n> paths. This is redundant, because ls-files will never output absolute\n> paths. Furthermore, sorting the output is also redundant, because the\n> output of ls-files is already sorted.\n>\n> Remove the unnecessary operations.\n>\n> Signed-off-by: Clemens Buchacher \n> ---\n\nMakes sense, and I think you can and should just directly jump to\nthis concluding state without having an intermediate \"sed\" version.\nThe fact that the code does not have to worry about absolute paths\nand unsorted input is shared with the original version, too, so the\nproposed log message for this one applies equally well to such a\nsquashed patch.\n\n\n\n> contrib/completion/git-completion.bash | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index e3ddf27..394c3df 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -384,7 +384,7 @@ __git_index_files ()\n> \tlocal root=\"${2-.}\" file\n> \n> \t__git_ls_files_helper \"$root\" \"$1\" |\n> -\tsed -e '/^\\//! s#/.*##' | sort | uniq\n> +\tcut -f1 -d/ | uniq\n> }\n> \n> # Lists branches from the local repository.\n"},{"id":"342069","messageId":"20180318012618.32691-1-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"1521274624-1370-2-git-send-email-drizzd@gmx.net","subject":"Re: [PATCH 2/2] completion: simplify ls-files filter","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-03-18T01:26:18Z","receivedAt":"2018-03-18T01:27:25Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"> When filtering the ls-files output we take care not to touch absolute\n> paths. This is redundant, because ls-files will never output absolute\n> paths. Furthermore, sorting the output is also redundant, because the\n> output of ls-files is already sorted.\n> \n> Remove the unnecessary operations.\n\nYou didn't run the test suite, did you? ;)\n\nFirst, neither 'git ls-files' nor 'git diff-index' produce quite the\nsame order as the 'sort' utility does, e.g.:\n\n $ touch foo.c foo-zzz.c\n $ git add foo*\n $ git diff-index --name-only HEAD\n foo-zzz.c\n foo.c\n $ git diff-index --name-only HEAD |sort\n foo.c\n foo-zzz.c\n\nSecond, the output of 'git ls-files' is kind of \"block-sorted\": if you\nwere to invoke it with the options '--cached --modified --others',\nthen it will first list all untracked files in order, then all cached\nfiles in order, and finally all modified files in order. Note the\nimplications:\n\n - A file could theoretically be listed twice, because a modified\n file is inherently cached as well. I believe this doesn't happen\n currently, because no path completions use the combination of\n '--modified --cached', but we use a lot of options when completing\n paths for 'git status', and I haven't thought that through.\n\n - A directory name is repeated in two (or more) blocks, if it\n contains modified and untracked files as well. We do use the\n combination of '--modified --others' for 'git add', and '--cached\n --others' for 'git mv', so this does happen.\n\nNote also that there can be any number of other files between the same\ndirectory listed in two different blocks. That 'sort' that this patch\nis about to remove took care of this, but without that 'sort' the same\ndirectory name can be listed more than once even after 'uniq'.\nConsequently, the subsequent filtering of paths matching the current\nword to be completed might have twice as much work to do.\n\nAll this leads to the failure of an enormous test in t9902, hence my\nrethorical question at the beginning of my reply.\n\n\nI have a short patch series collecting dust somewhere for a long\nwhile, which pulls a couple more tricks to make git-aware path\ncompletion faster, but haven't submitted it yet, because it doesn't\nwork quite that well when filenames require quoting. Though, arguably\nthe current version doesn't work quite that well with quoted filenames\neither, so...\nWill try to dig up those patches.\n\n\n> Signed-off-by: Clemens Buchacher \n> ---\n> contrib/completion/git-completion.bash | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index e3ddf27..394c3df 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -384,7 +384,7 @@ __git_index_files ()\n> \tlocal root=\"${2-.}\" file\n> \n> \t__git_ls_files_helper \"$root\" \"$1\" |\n> -\tsed -e '/^\\//! s#/.*##' | sort | uniq\n> +\tcut -f1 -d/ | uniq\n> }\n> \n> # Lists branches from the local repository.\n> -- \n> 2.7.4\n> \n> \n"},{"id":"342076","messageId":"xmqq4llex88v.fsf@gitster-ct.c.googlers.com","threadId":"48065","inReplyTo":"20180318012618.32691-1-szeder.dev@gmail.com","subject":"Re: [PATCH 2/2] completion: simplify ls-files filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-18T05:28:32Z","receivedAt":"2018-03-18T05:28:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> First, neither 'git ls-files' nor 'git diff-index' produce quite the\n> same order as the 'sort' utility does, e.g.:\n>\n> $ touch foo.c foo-zzz.c\n> $ git add foo*\n> $ git diff-index --name-only HEAD\n> foo-zzz.c\n> foo.c\n> $ git diff-index --name-only HEAD |sort\n> foo.c\n> foo-zzz.c\n\nDoesn't this depend on your locale?\n\n $ printf \"foo%s\\n\" .c -zzz.c /c | LC_ALL=C sort\n foo-zzz.c\n foo.c\n foo/c\n\n $ printf \"foo%s\\n\" .c -zzz.c /c | LC_ALL=en_US.UTF-8 sort\n foo/c\n foo.c\n foo-zzz.c\n\n> Second, the output of 'git ls-files' is kind of \"block-sorted\": if you\n> were to invoke it with the options '--cached --modified --others',\n> then it will first list all untracked files in order, then all cached\n> files in order, and finally all modified files in order.\n\nThis is a lot more important consideration.\n\n> I have a short patch series collecting dust somewhere for a long\n> while, which pulls a couple more tricks to make git-aware path\n> completion faster, but haven't submitted it yet, because it doesn't\n> work quite that well when filenames require quoting. Though, arguably\n> the current version doesn't work quite that well with quoted filenames\n> either, so...\n> Will try to dig up those patches.\n\nThanks.\n"},{"id":"342206","messageId":"nycvar.QRO.7.76.6.1803191802140.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48065","inReplyTo":"1521274624-1370-1-git-send-email-drizzd@gmx.net","subject":"Re: [PATCH 1/2] completion: improve ls-files filter performance","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-19T17:12:19Z","receivedAt":"2018-03-19T17:12:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi drizzd,\n\nfirst of all: thank you so much for working on this. I am sure it will\nbe noticeable to many Windows users, and also make my life easier.\n\nOn Sat, 17 Mar 2018, Clemens Buchacher wrote:\n\n> From the output of ls-files, we remove all but the leftmost path\n> component and then we eliminate duplicates. We do this in a while loop,\n> which is a performance bottleneck when the number of iterations is large\n> (e.g. for 60000 files in linux.git).\n> \n> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n> \n> real 0m11.876s\n> user 0m4.685s\n> sys 0m6.808s\n> \n> Using an equivalent sed script improves performance significantly:\n> \n> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n> \n> real 0m1.372s\n> user 0m0.263s\n> sys 0m0.167s\n> \n> The measurements were done with mingw64 bash, which is used by Git for\n> Windows.\n\nTechnically, it is not the *mingw64* bash, but it is an MSYS2 Bash. This\ndoes make a little bit of a difference because of the penalty incurred by\nthe POSIX emulation layer provided by the MSYS2 runtime.\n\n(And it also addresses Gabór's question whether you ran the test suite, I\nguess... it takes multiple hours to run it even once on a regular\ncomputer.)\n\nCiao,\nDscho"},{"id":"343782","messageId":"20180404074658.GA5833@Sonnenschein.localdomain","threadId":"48065","inReplyTo":"20180318012618.32691-1-szeder.dev@gmail.com","subject":"[PATCH] completion: improve ls-files filter performance","fromName":"Clemens Buchacher","fromEmail":"drizzd@gmx.net","sentAt":"2018-04-04T07:46:58Z","receivedAt":"2018-04-04T07:47:04Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"From the output of ls-files, we remove all but the leftmost path\ncomponent and then we eliminate duplicates. We do this in a while loop,\nwhich is a performance bottleneck when the number of iterations is large\n(e.g. for 60000 files in linux.git).\n\n$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n\nreal 0m11.876s\nuser 0m4.685s\nsys 0m6.808s\n\nReplacing the loop with the cut command improves performance\nsignificantly:\n\n$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n\nreal 0m1.372s\nuser 0m0.263s\nsys 0m0.167s\n\nThe measurements were done with Msys2 bash, which is used by Git for\nWindows.\n\nWhen filtering the ls-files output we take care not to touch absolute\npaths. This is redundant, because ls-files will never output absolute\npaths. Remove the unnecessary operations.\n\nThe issue was reported here:\nhttps://github.com/git-for-windows/git/issues/1533\n\nSigned-off-by: Clemens Buchacher \n---\n\nOn Sun, Mar 18, 2018 at 02:26:18AM +0100, SZEDER Gábor wrote:\n> \n> You didn't run the test suite, did you? ;)\n\nMy bad. I put the sort back in. Test t9902 is now pass. I did not run\nthe other tests. I think the completion script is not used there.\n\nI also considered Junio's and Johannes' comments.\n\n> I have a short patch series collecting dust somewhere for a long\n> while, [...]\n> Will try to dig up those patches.\n\nCool. Bash completion can certainly use more performance improvements.\n\n contrib/completion/git-completion.bash | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 6da95b8..69a2d41 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -384,12 +384,7 @@ __git_index_files ()\n \tlocal root=\"${2-.}\" file\n \n \t__git_ls_files_helper \"$root\" \"$1\" |\n-\twhile read -r file; do\n-\t\tcase \"$file\" in\n-\t\t?*/*) echo \"${file%%/*}\" ;;\n-\t\t*) echo \"$file\" ;;\n-\t\tesac\n-\tdone | sort | uniq\n+\tcut -f1 -d/ | sort | uniq\n }\n \n # Lists branches from the local repository.\n-- \n2.7.4\n\n"},{"id":"343789","messageId":"nycvar.QRO.7.76.6.1804041805150.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48065","inReplyTo":"20180404074658.GA5833@Sonnenschein.localdomain","subject":"Re: [PATCH] completion: improve ls-files filter performance","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-04T16:16:13Z","receivedAt":"2018-04-04T16:16:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi drizzd,\n\nOn Wed, 4 Apr 2018, Clemens Buchacher wrote:\n\n> From the output of ls-files, we remove all but the leftmost path\n> component and then we eliminate duplicates. We do this in a while loop,\n> which is a performance bottleneck when the number of iterations is large\n> (e.g. for 60000 files in linux.git).\n> \n> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n> \n> real 0m11.876s\n> user 0m4.685s\n> sys 0m6.808s\n> \n> Replacing the loop with the cut command improves performance\n> significantly:\n> \n> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git\n> \n> real 0m1.372s\n> user 0m0.263s\n> sys 0m0.167s\n> \n> The measurements were done with Msys2 bash, which is used by Git for\n> Windows.\n\nThose are nice numbers right there, so I am eager to get this into Git for\nWindows as quickly as it stabilizes (i.e. when it hits `next` or so).\n\nI was wondering about one thing, though:\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 6da95b8..69a2d41 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -384,12 +384,7 @@ __git_index_files ()\n> \tlocal root=\"${2-.}\" file\n> \n> \t__git_ls_files_helper \"$root\" \"$1\" |\n> -\twhile read -r file; do\n> -\t\tcase \"$file\" in\n> -\t\t?*/*) echo \"${file%%/*}\" ;;\n\nThis is a bit different from the `cut -f1 -d/` logic, as it does *not\nnecessarily* strip a leading slash: for `/abc` the existing code would\nreturn the string unmodified, for `/abc/def` it would return an empty\nstring!\n\nNow, I think that this peculiar behavior is most likely bogus as `git\nls-files` outputs only relative paths (that I know of). In any case,\nreducing paths to an empty string seems fishy.\n\nI looked through the history of that code and tracked it all the way back\nto\nhttps://public-inbox.org/git/1357930123-26310-1-git-send-email-manlio.perillo@gmail.com/\n(that is the reason why you are Cc:ed, Manlio). Manlio, do you remember\nwhy you put the `?` in front of `?*/*` here? I know, it's been more than\nfive years...\n\nOut of curiosity, would the numbers change a lot if you replaced the `cut\n-f1 -d/` call by a `sed -e 's/^\\//' -e 's/\\/.*//'` one?\n\nI am not proposing to change the patch, though, because we really do not\nneed to expect `ls-files` to print lines with leading slashes.\n\n> -\t\t*) echo \"$file\" ;;\n> -\t\tesac\n> -\tdone | sort | uniq\n> +\tcut -f1 -d/ | sort | uniq\n> }\n> \n> # Lists branches from the local repository.\n\nCiao,\nDscho\n"},{"id":"344830","messageId":"20180416224113.16993-1-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180318012618.32691-1-szeder.dev@gmail.com","subject":"[PATCH 00/11] completion: path completion improvements: speedup and quoted paths","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:04Z","receivedAt":"2018-04-16T22:41:29Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"So, the highlights of this patch series are:\n\n - At the top of the worktree in linux.git with over 62k files the time\n needed for 'git rm ' goes down from 2.15s to 0.052s.\n\n - Same place, uniquely completing Makefile with 'git rm Mak' goes\n down from 2.16s to 0.033s.\n\n - Completing paths containing escaped or quoted characters (except\n newline) will work properly, and won't fall back to Bash's filename\n completion, e.g. git add File\\\\w' will list\n 'File\\with\\backslashes.c', but not the corresponding ignored object\n file.\n\nWell, at least it appears to be working for me, but dealing with quoting\nespecially is nuts. If someone knows simpler ways to remove escaping\nand quoting than in the functions added in patches 5 and 10, then\nplease, I would really love to learn something :)\n\nSomeone using ZSH regularly should have a look and test it.\n\nSZEDER Gábor (11):\n t9902-completion: add tests demonstrating issues with quoted pathnames\n completion: move __git_complete_index_file() next to its helpers\n completion: simplify prefix path component handling during path\n completion\n completion: support completing non-ASCII pathnames\n completion: improve handling quoted paths on the command line\n completion: let 'ls-files' and 'diff-index' filter matching paths\n completion: use 'awk' to strip trailing path components\n t9902-completion: ignore COMPREPLY element order in some tests\n completion: remove repeated dirnames with 'awk' during path completion\n completion: improve handling quoted paths in 'git ls-files's output\n completion: fill COMPREPLY directly when completing paths\n\n contrib/completion/git-completion.bash | 218 +++++++++++++++++++++----\n contrib/completion/git-completion.zsh | 9 +\n t/t9902-completion.sh | 153 ++++++++++++++++-\n 3 files changed, 348 insertions(+), 32 deletions(-)\n\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344831","messageId":"20180416224113.16993-2-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:05Z","receivedAt":"2018-04-16T22:41:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Completion functions see all words on the command line verbatim,\nincluding any backslash-escapes, single and double quotes that might\nbe there. Furthermore, git commands quote pathnames if they contain\ncertain special characters. All these create various issues when\ndoing git-aware path completion.\n\nAdd a couple of failing tests to demonstrate these issues.\n\nLater patches in this series will discuss these issues in detail as\nthey fix them.\n\nSigned-off-by: SZEDER Gábor \n---\n\nNotes:\n Do any more new tests need FUNNYNAMES* prereq?\n\n t/t9902-completion.sh | 91 +++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 91 insertions(+)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex b7f5b1e632..ff2e4a8f5f 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1427,6 +1427,97 @@ test_expect_success 'complete files' '\n \ttest_completion \"git add mom\" \"momified\"\n '\n \n+# The next tests only care about how the completion script deals with\n+# unusual characters in path names. By defining a custom completion\n+# function to list untracked files they won't be influenced by future\n+# changes of the completion functions of real git commands, and we\n+# don't have to bother with adding files to the index in these tests.\n+_git_test_path_comp ()\n+{\n+\t__git_complete_index_file --others\n+}\n+\n+test_expect_failure 'complete files - escaped characters on cmdline' '\n+\ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n+\tmkdir \"New|Dir\" &&\n+\t>\"New|Dir/New&File.c\" &&\n+\n+\ttest_completion \"git test-path-comp N\" \\\n+\t\t\t\"New|Dir\" &&\t# Bash will turn this into \"New\\|Dir/\"\n+\ttest_completion \"git test-path-comp New\\\\|D\" \\\n+\t\t\t\"New|Dir\" &&\n+\ttest_completion \"git test-path-comp New\\\\|Dir/N\" \\\n+\t\t\t\"New|Dir/New&File.c\" &&\t# Bash will turn this into\n+\t\t\t\t\t\t# \"New\\|Dir/New\\&File.c \"\n+\ttest_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n+\t\t\t\"New|Dir/New&File.c\"\n+'\n+\n+test_expect_failure 'complete files - quoted characters on cmdline' '\n+\ttest_when_finished \"rm -r \\\"New(Dir\\\"\" &&\n+\tmkdir \"New(Dir\" &&\n+\t>\"New(Dir/New)File.c\" &&\n+\n+\ttest_completion \"git test-path-comp \\\"New(D\" \"New(Dir\" &&\n+\ttest_completion \"git test-path-comp \\\"New(Dir/New)F\" \\\n+\t\t\t\"New(Dir/New)File.c\"\n+'\n+\n+test_expect_failure 'complete files - UTF-8 in ls-files output' '\n+\ttest_when_finished \"rm -r árvíztűrő\" &&\n+\tmkdir árvíztűrő &&\n+\t>\"árvíztűrő/Сайн яваарай\" &&\n+\n+\ttest_completion \"git test-path-comp á\" \"árvíztűrő\" &&\n+\ttest_completion \"git test-path-comp árvíztűrő/С\" \\\n+\t\t\t\"árvíztűrő/Сайн яваарай\"\n+'\n+\n+if test_have_prereq !MINGW &&\n+ mkdir 'New\\Dir' 2>/dev/null &&\n+ touch 'New\\Dir/New\"File.c' 2>/dev/null\n+then\n+\ttest_set_prereq FUNNYNAMES_BS_DQ\n+else\n+\tsay \"Your filesystem does not allow \\\\ and \\\" in filenames.\"\n+\trm -rf 'New\\Dir'\n+fi\n+test_expect_failure FUNNYNAMES_BS_DQ \\\n+ 'complete files - C-style escapes in ls-files output' '\n+\ttest_when_finished \"rm -r \\\"New\\\\\\\\Dir\\\"\" &&\n+\n+\ttest_completion \"git test-path-comp N\" \"New\\\\Dir\" &&\n+\ttest_completion \"git test-path-comp New\\\\\\\\D\" \"New\\\\Dir\" &&\n+\ttest_completion \"git test-path-comp New\\\\\\\\Dir/N\" \\\n+\t\t\t\"New\\\\Dir/New\\\"File.c\" &&\n+\ttest_completion \"git test-path-comp New\\\\\\\\Dir/New\\\\\\\"F\" \\\n+\t\t\t\"New\\\\Dir/New\\\"File.c\"\n+'\n+\n+if test_have_prereq !MINGW &&\n+ mkdir $'New\\034Special\\035Dir' 2>/dev/null &&\n+ touch $'New\\034Special\\035Dir/New\\036Special\\037File' 2>/dev/null\n+then\n+\ttest_set_prereq FUNNYNAMES_SEPARATORS\n+else\n+\tsay 'Your filesystem does not allow special separator characters (FS, GS, RS, US) in filenames.'\n+\trm -rf $'New\\034Special\\035Dir'\n+fi\n+test_expect_failure FUNNYNAMES_SEPARATORS \\\n+ 'complete files - \\nnn-escaped control characters in ls-files output' '\n+\ttest_when_finished \"rm -r '$'New\\034Special\\035Dir''\" &&\n+\n+\t# Note: these will be literal separator characters on the cmdline.\n+\ttest_completion \"git test-path-comp N\" \"'$'New\\034Special\\035Dir''\" &&\n+\ttest_completion \"git test-path-comp '$'New\\034S''\" \\\n+\t\t\t\"'$'New\\034Special\\035Dir''\" &&\n+\ttest_completion \"git test-path-comp '$'New\\034Special\\035Dir/''\" \\\n+\t\t\t\"'$'New\\034Special\\035Dir/New\\036Special\\037File''\" &&\n+\ttest_completion \"git test-path-comp '$'New\\034Special\\035Dir/New\\036S''\" \\\n+\t\t\t\"'$'New\\034Special\\035Dir/New\\036Special\\037File''\"\n+'\n+\n+\n test_expect_success \"completion uses completion for alias: !sh -c 'git ...'\" '\n \ttest_config alias.co \"!sh -c '\"'\"'git checkout ...'\"'\"'\" &&\n \ttest_completion \"git co m\" <<-\\EOF\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344832","messageId":"20180416224113.16993-3-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 02/11] completion: move __git_complete_index_file() next to its helpers","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:06Z","receivedAt":"2018-04-16T22:41:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"It's much easier to read, understand and modify the functions related\nto git-aware path completion when they are right next to each other.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 39 +++++++++++++-------------\n 1 file changed, 19 insertions(+), 20 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b09c8a2362..36d3c6f928 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -396,6 +396,25 @@ __git_index_files ()\n \tdone | sort | uniq\n }\n \n+# __git_complete_index_file requires 1 argument:\n+# 1: the options to pass to ls-file\n+#\n+# The exception is --committable, which finds the files appropriate commit.\n+__git_complete_index_file ()\n+{\n+\tlocal pfx=\"\" cur_=\"$cur\"\n+\n+\tcase \"$cur_\" in\n+\t?*/*)\n+\t\tpfx=\"${cur_%/*}\"\n+\t\tcur_=\"${cur_##*/}\"\n+\t\tpfx=\"${pfx}/\"\n+\t\t;;\n+\tesac\n+\n+\t__gitcomp_file \"$(__git_index_files \"$1\" ${pfx:+\"$pfx\"})\" \"$pfx\" \"$cur_\"\n+}\n+\n # Lists branches from the local repository.\n # 1: A prefix to be added to each listed branch (optional).\n # 2: List only branches matching this word (optional; list all branches if\n@@ -712,26 +731,6 @@ __git_complete_revlist_file ()\n \tesac\n }\n \n-\n-# __git_complete_index_file requires 1 argument:\n-# 1: the options to pass to ls-file\n-#\n-# The exception is --committable, which finds the files appropriate commit.\n-__git_complete_index_file ()\n-{\n-\tlocal pfx=\"\" cur_=\"$cur\"\n-\n-\tcase \"$cur_\" in\n-\t?*/*)\n-\t\tpfx=\"${cur_%/*}\"\n-\t\tcur_=\"${cur_##*/}\"\n-\t\tpfx=\"${pfx}/\"\n-\t\t;;\n-\tesac\n-\n-\t__gitcomp_file \"$(__git_index_files \"$1\" ${pfx:+\"$pfx\"})\" \"$pfx\" \"$cur_\"\n-}\n-\n __git_complete_file ()\n {\n \t__git_complete_revlist_file\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344833","messageId":"20180416224113.16993-5-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 04/11] completion: support completing non-ASCII pathnames","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:08Z","receivedAt":"2018-04-16T22:41:33Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Unless the user has 'core.quotePath=false' somewhere in the\nconfiguration, both 'git ls-files' and 'git diff-index' will by\ndefault quote any pathnames that contain bytes with values higher than\n0x80, and escape those bytes as '\\nnn' octal values. This prevents\ncompleting paths when the current path component to be completed\ncontains any non-ASCII, most notably UTF-8, characters, because none\nof the listed quoted paths will match the current word on the command\nline.\n\nSet 'core.quotePath=false' for those 'git ls-files' and 'git\ndiff-index' invocations, so they won't consider bytes higher than 0x80\nas \"unusual\", and won't quote pathnames containing such characters.\n\nNote that pathnames containing backslash, double quote, or control\ncharacters will still be quoted; a later patch in this series will\ndeal with those.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 6 ++++--\n t/t9902-completion.sh | 2 +-\n 2 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 72cd3add19..7072555960 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -369,10 +369,12 @@ __gitcomp_file ()\n __git_ls_files_helper ()\n {\n \tif [ \"$2\" == \"--committable\" ]; then\n-\t\t__git -C \"$1\" diff-index --name-only --relative HEAD\n+\t\t__git -C \"$1\" -c core.quotePath=false diff-index \\\n+\t\t\t--name-only --relative HEAD\n \telse\n \t\t# NOTE: $2 is not quoted in order to support multiple options\n-\t\t__git -C \"$1\" ls-files --exclude-standard $2\n+\t\t__git -C \"$1\" -c core.quotePath=false ls-files \\\n+\t\t\t--exclude-standard $2\n \tfi\n }\n \ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex ff2e4a8f5f..a4f2c03b93 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1463,7 +1463,7 @@ test_expect_failure 'complete files - quoted characters on cmdline' '\n \t\t\t\"New(Dir/New)File.c\"\n '\n \n-test_expect_failure 'complete files - UTF-8 in ls-files output' '\n+test_expect_success 'complete files - UTF-8 in ls-files output' '\n \ttest_when_finished \"rm -r árvíztűrő\" &&\n \tmkdir árvíztűrő &&\n \t>\"árvíztűrő/Сайн яваарай\" &&\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344834","messageId":"20180416224113.16993-9-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 08/11] t9902-completion: ignore COMPREPLY element order in some tests","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:12Z","receivedAt":"2018-04-16T22:41:36Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The order or possible completion words in the COMPREPLY array doesn't\nactually matter, as long as all the right words are in there, because\nBash will sort them anyway. Yet, our tests looking at the elements of\nCOMPREPLY always expect them to be in a specific order.\n\nNow, this hasn't been an issue before, but the next patch is about to\noptimize a bit more our git-aware path completion, and as a harmless\nside effect the order of elements in COMPREPLY will change. Worse,\nthe order will be downright undefined, because after the next patch\npath components will come directly from iterating through an\nassociative array in 'awk', and the order of iteration over the\nelements in those arrays is undefined, and indeed different 'awk'\nimplementations produce different order. Consequently, we can't get\naway with simply adjusting the expected results in the affected tests.\n\nModify the 'test_completion' helper function to sort both the expected\nand the actual results, i.e. the elements in COMPREPLY, before\ncomparing them, so the tests using this helper function will work\nregardless of the order of elements.\n\nNote that this change still leaves a bunch of tests depending on the\norder of elements in COMPREPLY, tests that focus on a specific helper\nfunction and therefore don't use the 'test_completion' helper. I\nwould rather deal with those later, when (if ever) the need actually\narises, than create unnecessary code churn now.\n\nSigned-off-by: SZEDER Gábor \n---\n t/t9902-completion.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex f7a9dd446d..a747998d6c 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -84,10 +84,11 @@ test_completion ()\n \tthen\n \t\tprintf '%s\\n' \"$2\" >expected\n \telse\n-\t\tsed -e 's/Z$//' >expected\n+\t\tsed -e 's/Z$//' |sort >expected\n \tfi &&\n \trun_completion \"$1\" &&\n-\ttest_cmp expected out\n+\tsort out >out_sorted &&\n+\ttest_cmp expected out_sorted\n }\n \n # Test __gitcomp.\n@@ -1405,6 +1406,7 @@ test_expect_success 'complete files' '\n \n \techo \"expected\" > .gitignore &&\n \techo \"out\" >> .gitignore &&\n+\techo \"out_sorted\" >> .gitignore &&\n \n \tgit add .gitignore &&\n \ttest_completion \"git commit \" \".gitignore\" &&\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344835","messageId":"20180416224113.16993-10-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 09/11] completion: remove repeated dirnames with 'awk' during path completion","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:13Z","receivedAt":"2018-04-16T22:41:38Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"During git-aware path completion, after all the trailing path\ncomponents have been removed from the output of 'git ls-files' and\n'git diff-index' (see previous patch), each directory name is repeated\nas many times as the number of listed paths it contains. This can be\na lot of repetitions, especially when invoking path completion close\nto the root of a big worktree, which would cause a considerable\noverhead downstream of __git_index_files(), in particular in the shell\nloop that fills the COMPREPLY array. To reduce this overhead,\n__git_index_files() runs the classic '... |sort |uniq' pattern to\nremove those repetitions from the function's output.\n\nWhile removing repeated directory names is effective in reducing the\nnumber of iterations in that shell loop, it still imposes the overhead\nof fork()+exec()ing two external processes, and two additional stages\nin the pipeline, where potentially relatively large amount of data can\nbe passed between two subsequent pipeline stages.\n\nExtend __git_index_files()'s 'awk' script to remove repeated path\ncomponents by first creating and filling an associative array indexed\nby all encountered path components (after the trailing path components\nhave been removed), and then iterating over this array and printing\nthe indices, i.e. unique path components. This way we can remove the\n'|sort |uniq' pipeline stages, and their eliminated overhead results\nin faster path completion.\n\nListing all tracked files (12) and directories (23) at the top of the\nworktree in linux.git (over 62k files), i.e. what's doing all the hard\nwork behind 'git rm ':\n\n Before this patch, best of five, using GNU awk on Linux:\n\n real 0m0.069s\n user 0m0.089s\n sys 0m0.026s\n\n After:\n\n real 0m0.052s\n user 0m0.072s\n sys 0m0.014s\n\n Difference: -24.6%\n\nNote that this changes order of elements in __git_index_files()'s\noutput. This is not an issue, because this function was only ever\nintended to feed paths into the COMPREPLY array, and Bash will sort\nits elements (according to the users locale) anyway.\n\nNote also that using 'awk' to remove repeated path components is also\nbeneficial for the performance of the next two patches:\n\n - The first will extend this 'awk' script to dequote quoted paths in\n the output of 'git ls-files' and 'git diff-index'. With this\n patch it will only have to dequote unique path components, not\n all.\n\n - The second will, among other things, extend this 'awk' script to\n prepend prefix path components from the command line to the\n currently completed path component. Consequently, each line in\n 'awk's output will grow longer. Without this patch that '|sort\n |uniq' would have to exchange and process that much more data.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 0abba88462..70bc75dfc7 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -456,8 +456,12 @@ __git_index_files ()\n \n \t__git_ls_files_helper \"$root\" \"$1\" \"$match\" |\n \tawk -F / '{\n-\t\tprint $1\n-\t}' | sort | uniq\n+\t\tpaths[$1] = 1\n+\t}\n+\tEND {\n+\t\tfor (p in paths)\n+\t\t\tprint p\n+\t}'\n }\n \n # __git_complete_index_file requires 1 argument:\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344836","messageId":"20180416224113.16993-7-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 06/11] completion: let 'ls-files' and 'diff-index' filter matching paths","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:10Z","receivedAt":"2018-04-16T22:41:40Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"During git-aware path completion, e.g. 'git rm dir/fil', both\n'git ls-files' and 'git diff-index' list all paths in the given 'dir/'\nmatching certain criteria (cached, modified, untracked, etc.)\nappropriate for the given git command, even paths whose names don't\nbegin with 'fil'. This comes with a considerable performance\npenalty when the directory in question contains a lot of paths, but\nthe current word can be uniquely completed or when only a handful of\nthose paths match the current word.\n\nReduce the number of iterations in this codepath from the number of\npaths to the number of matching paths by specifying an appropriate\nglobbing pattern to 'git ls-files' and 'git diff-index' to list only\npaths that match the current word to be completed.\n\nNote that both commands treat backslashes as escape characters in\ntheir file arguments, e.g. to preserve the literal meaning of globbing\ncharacters, so we have to double every backslash in the globbing\npattern. This is why one of the path completion tests specifically\nchecks the completion of a path containing a literal backslash\ncharacter (that test still fails, though, because both commands output\nsuch paths enclosed in double quotes and the special characters\nescaped; a later patch in this series will deal with those).\n\nThis speeds up path completion considerably when there are a lot of\nnon-matching paths to be filtered out. Uniquely completing a tracked\nfilename at the top of the worktree in linux.git (over 62k files),\ni.e. what's doing all the hard work behind 'git rm Mak' to\ncomplete 'Makefile':\n\n Before this patch, best of five, on Linux:\n\n $ time cur=Mak __git_complete_index_file\n\n real 0m2.159s\n user 0m1.299s\n sys 0m1.089s\n\n After:\n\n real 0m0.033s\n user 0m0.023s\n sys 0m0.015s\n\n Difference: -98.5%\n Speedup: 65.4x\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 11 ++++++-----\n t/t9902-completion.sh | 1 +\n 2 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex ae6127155e..3948265d32 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -434,11 +434,11 @@ __git_ls_files_helper ()\n {\n \tif [ \"$2\" == \"--committable\" ]; then\n \t\t__git -C \"$1\" -c core.quotePath=false diff-index \\\n-\t\t\t--name-only --relative HEAD\n+\t\t\t--name-only --relative HEAD -- \"${3//\\\\/\\\\\\\\}*\"\n \telse\n \t\t# NOTE: $2 is not quoted in order to support multiple options\n \t\t__git -C \"$1\" -c core.quotePath=false ls-files \\\n-\t\t\t--exclude-standard $2\n+\t\t\t--exclude-standard $2 -- \"${3//\\\\/\\\\\\\\}*\"\n \tfi\n }\n \n@@ -449,11 +449,12 @@ __git_ls_files_helper ()\n # If provided, only files within the specified directory are listed.\n # Sub directories are never recursed. Path must have a trailing\n # slash.\n+# 3: List only paths matching this path component (optional).\n __git_index_files ()\n {\n-\tlocal root=\"$2\" file\n+\tlocal root=\"$2\" match=\"$3\" file\n \n-\t__git_ls_files_helper \"$root\" \"$1\" |\n+\t__git_ls_files_helper \"$root\" \"$1\" \"$match\" |\n \twhile read -r file; do\n \t\tcase \"$file\" in\n \t\t?*/*) echo \"${file%%/*}\" ;;\n@@ -481,7 +482,7 @@ __git_complete_index_file ()\n \t\tcur_=\"$dequoted_word\"\n \tesac\n \n-\t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\n+\t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\" \"$cur_\")\" \"$pfx\" \"$cur_\"\n }\n \n # Lists branches from the local repository.\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 6856b263f8..f7a9dd446d 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1515,6 +1515,7 @@ test_expect_success 'complete files - UTF-8 in ls-files output' '\n \t\t\t\"árvíztűrő/Сайн яваарай\"\n '\n \n+# Testing with a path containing a backslash is important.\n if test_have_prereq !MINGW &&\n mkdir 'New\\Dir' 2>/dev/null &&\n touch 'New\\Dir/New\"File.c' 2>/dev/null\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344837","messageId":"20180416224113.16993-8-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 07/11] completion: use 'awk' to strip trailing path components","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:11Z","receivedAt":"2018-04-16T22:41:43Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"During git-aware path completion we complete one path component at a\ntime, i.e. 'git add ' offers only 'dir/' at first, not\n'dir/subdir/file' right away, just like Bash's own filename\ncompletion. However, since both 'git ls-files' and 'git diff-index'\ndive deep into subdirectories, we have to strip all trailing path\ncomponents from the listed paths, keeping only the leading path\ncomponent. This stripping is currently done in a shell loop in\n__git_index_files(), which can take a significant amount of time when\nit has to iterate through a large number of paths.\n\nReplace this shell loop with a little 'awk' script using '/' as input\nfield separator and printing the first field, which produces the same\noutput much faster.\n\nListing all tracked files (12) and directories (23) at the top of the\nworktree in linux.git (over 62k files), i.e. what's doing all the hard\nwork behind 'git rm ':\n\n Before this patch, best of five, using GNU awk on Linux:\n\n $ time cur= __git_complete_index_file\n\n real 0m2.149s\n user 0m1.307s\n sys 0m1.086s\n\n After:\n\n real 0m0.067s\n user 0m0.089s\n sys 0m0.023s\n\n Difference: -96.9%\n Speedup: 32.1x\n\nNote that this could be done with 'sed', or even with 'cut', just as\nwell, but the upcoming patches require 'awk's scriptability.\n\nNote also that this change means one more fork()+exec()ed process\nduring path completion, adding more overhead especially on Windows,\nbut a later patch will more than make up for it by eliminating two\nother processes in the same function.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 11 ++++-------\n 1 file changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 3948265d32..0abba88462 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -452,15 +452,12 @@ __git_ls_files_helper ()\n # 3: List only paths matching this path component (optional).\n __git_index_files ()\n {\n-\tlocal root=\"$2\" match=\"$3\" file\n+\tlocal root=\"$2\" match=\"$3\"\n \n \t__git_ls_files_helper \"$root\" \"$1\" \"$match\" |\n-\twhile read -r file; do\n-\t\tcase \"$file\" in\n-\t\t?*/*) echo \"${file%%/*}\" ;;\n-\t\t*) echo \"$file\" ;;\n-\t\tesac\n-\tdone | sort | uniq\n+\tawk -F / '{\n+\t\tprint $1\n+\t}' | sort | uniq\n }\n \n # __git_complete_index_file requires 1 argument:\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344838","messageId":"20180416224113.16993-6-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 05/11] completion: improve handling quoted paths on the command line","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:09Z","receivedAt":"2018-04-16T22:41:47Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Our git-aware path completion doesn't work when it has to complete a\nword already containing quoted and/or backslash-escaped characters on\nthe command line. The root cause of the issue is that completion\nfunctions see all words on the command line verbatim, i.e. including\nall backslash, single and double quote characters that the shell would\neventually remove when executing the finished command. These\nquoting/escaping characters cause different issues depending on which\npath component of the word to be completed contains them:\n\n - The quoting/escaping is in the prefix path component(s).\n\n Let's suppose we have a directory called 'New Dir', containing two\n untracked files 'file.c' and 'file.o', and we have a gitignore\n rule ignoring object files. In this case all of these:\n\n git add New\\ Dir/\n git add \"New Dir/\n git add 'New Dir/\n\n should uniquely complete 'file.c' right away, but Bash offers both\n 'file.c' and 'file.o' instead. The reason for this behavior is\n that our completion script uses the prefix directory name like\n 'git -C \"New\\ Dir/\" ls-files ...\", i.e. with the backslash inside\n double quotes. Git then tries to enter a directory called\n 'New\\ Dir', which (most likely) fails because such a directory\n doesn't exists. As a result our completion script doesn't list\n any files, leaves the COMPREPLY array empty, which in turn causes\n Bash to fall back to its simple filename completion and lists all\n files in that directory, i.e. both 'file.c' and 'file.o'.\n\n - The quoting/escaping is in the path component to be completed.\n\n Let's suppose we have two untracked files 'New File.c' and\n 'New File.o', and we have a gitignore rule ignoring object files.\n In this case all of these:\n\n git add New\\ Fi\n git add \"New Fi\n git add 'New Fi\n\n should uniquely complete 'New File.c' right away, but Bash offers\n both 'New File.c' and 'New File.o' instead. The reason for this\n behavior is that our completion script uses this 'New\\ Fi' or\n '\"New Fi' etc. word to filter matching paths, and of course none\n of the potential filenames will match because of the included\n backslash or double quote. The end result is the same as above:\n the completion script doesn't list any files, Bash falls back to\n its filename completion, which then lists the matching object file\n as well.\n\nAdd the new helper function __git_dequote() [1], which removes (most\nof[2]) the quoting and escaping from the word it gets as argument. To\nminimize the overhead of calling this function, store its result in\nthe variable $dequoted_word, supposed to be declared local in the\ncaller; simply printing the result would require a command\nsubstitution imposing the overhead of fork()ing a subshell. Use this\nfunction in __git_complete_index_file() to dequote the current word,\ni.e. the path, to be completed, to avoid the above described\nquoting-related issues, thereby fixing two of the failing quoted path\ncompletion tests.\n\n[1] The bash-completion project already has a dequote() function,\n which I hoped I could borrow to deal with this, but unfortunately\n it doesn't work quite well for this purpose (perhaps that's why\n even the bash-completion project only rarely uses it). The main\n issue is that their dequote() is implemented as:\n\n eval printf %s \"$1\" 2> /dev/null\n\n where $1 would contain the word to be completed. While it's a\n short and sweet one-liner, the use of 'eval' requires that $1 is a\n syntactically valid string, which is not the case when quoting the\n path like 'git add \"New Dir/'. This causes 'eval' to fail,\n because it can't find the matching closing double quote, and the\n function returns nothing. The result is totally broken behavior,\n as if the current word were empty, and the completion script would\n then list all files from the current directory. This is why one\n of the quoted path completion tests specifically checks the\n completion of a path with an opening but without a corresponding\n closing double quote character. Furthermore, the 'eval' performs\n all kinds of expansions, which may or may not be desired; I think\n it's the latter. Finally, using this function would require a\n command substitution.\n\n[2] Bash understands the $'string' quoting as well, which \"expands to\n 'string', with backslash-escaped characters replaced as specified\n by the ANSI C standard\" (quoted from Bash manpage). Since shell\n metacharacters, field separators, globbing, etc. can all be easily\n entered using standard shell escaping or quoting, this type of\n quoting comes in handly when dealing with control characters that\n are otherwise difficult both to \"type\" and to see on the command\n line. Because of this difficulty I would assume that people do\n avoid pathnames with such control characters anyway, so I didn't\n bother implementing it. This function is already way too long as\n it is.\n\nSigned-off-by: SZEDER Gábor \n---\n\nNotes:\n What if someone on Windows were to use backslash as directory\n separator during path completion? E.g. 'git add new-dir\\subdir\\'\n Did that happen to work in the past? Does this patch break it?\n\n contrib/completion/git-completion.bash | 76 ++++++++++++++++++++++++--\n t/t9902-completion.sh | 46 +++++++++++++++-\n 2 files changed, 116 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7072555960..ae6127155e 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -92,6 +92,70 @@ __git ()\n \t\t${__git_dir:+--git-dir=\"$__git_dir\"} \"$@\" 2>/dev/null\n }\n \n+# Removes backslash escaping, single quotes and double quotes from a word,\n+# stores the result in the variable $dequoted_word.\n+# 1: The word to dequote.\n+__git_dequote ()\n+{\n+\tlocal rest=\"$1\" len ch\n+\n+\tdequoted_word=\"\"\n+\n+\twhile test -n \"$rest\"; do\n+\t\tlen=${#dequoted_word}\n+\t\tdequoted_word=\"$dequoted_word${rest%%[\\\\\\'\\\"]*}\"\n+\t\trest=\"${rest:$((${#dequoted_word}-$len))}\"\n+\n+\t\tcase \"${rest:0:1}\" in\n+\t\t\\\\)\n+\t\t\tch=\"${rest:1:1}\"\n+\t\t\tcase \"$ch\" in\n+\t\t\t$'\\n')\n+\t\t\t\t;;\n+\t\t\t*)\n+\t\t\t\tdequoted_word=\"$dequoted_word$ch\"\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\t\trest=\"${rest:2}\"\n+\t\t\t;;\n+\t\t\\')\n+\t\t\trest=\"${rest:1}\"\n+\t\t\tlen=${#dequoted_word}\n+\t\t\tdequoted_word=\"$dequoted_word${rest%%\\'*}\"\n+\t\t\trest=\"${rest:$((${#dequoted_word}-$len+1))}\"\n+\t\t\t;;\n+\t\t\\\")\n+\t\t\trest=\"${rest:1}\"\n+\t\t\twhile test -n \"$rest\" ; do\n+\t\t\t\tlen=${#dequoted_word}\n+\t\t\t\tdequoted_word=\"$dequoted_word${rest%%[\\\\\\\"]*}\"\n+\t\t\t\trest=\"${rest:$((${#dequoted_word}-$len))}\"\n+\t\t\t\tcase \"${rest:0:1}\" in\n+\t\t\t\t\\\\)\n+\t\t\t\t\tch=\"${rest:1:1}\"\n+\t\t\t\t\tcase \"$ch\" in\n+\t\t\t\t\t\\\"|\\\\|\\$|\\`)\n+\t\t\t\t\t\tdequoted_word=\"$dequoted_word$ch\"\n+\t\t\t\t\t\t;;\n+\t\t\t\t\t$'\\n')\n+\t\t\t\t\t\t;;\n+\t\t\t\t\t*)\n+\t\t\t\t\t\tdequoted_word=\"$dequoted_word\\\\$ch\"\n+\t\t\t\t\t\t;;\n+\t\t\t\t\tesac\n+\t\t\t\t\trest=\"${rest:2}\"\n+\t\t\t\t\t;;\n+\t\t\t\t\\\")\n+\t\t\t\t\trest=\"${rest:1}\"\n+\t\t\t\t\tbreak\n+\t\t\t\t\t;;\n+\t\t\t\tesac\n+\t\t\tdone\n+\t\t\t;;\n+\t\tesac\n+\tdone\n+}\n+\n # The following function is based on code from:\n #\n # bash_completion - programmable completion functions for bash 3.2+\n@@ -404,13 +468,17 @@ __git_index_files ()\n # The exception is --committable, which finds the files appropriate commit.\n __git_complete_index_file ()\n {\n-\tlocal pfx=\"\" cur_=\"$cur\"\n+\tlocal dequoted_word pfx=\"\" cur_\n \n-\tcase \"$cur_\" in\n+\t__git_dequote \"$cur\"\n+\n+\tcase \"$dequoted_word\" in\n \t?*/*)\n-\t\tpfx=\"${cur_%/*}/\"\n-\t\tcur_=\"${cur_##*/}\"\n+\t\tpfx=\"${dequoted_word%/*}/\"\n+\t\tcur_=\"${dequoted_word##*/}\"\n \t\t;;\n+\t*)\n+\t\tcur_=\"$dequoted_word\"\n \tesac\n \n \t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex a4f2c03b93..6856b263f8 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -400,6 +400,46 @@ test_expect_success '__gitdir - remote as argument' '\n \ttest_cmp expected \"$actual\"\n '\n \n+\n+test_expect_success '__git_dequote - plain unquoted word' '\n+\t__git_dequote unquoted-word &&\n+\tverbose test unquoted-word = \"$dequoted_word\"\n+'\n+\n+# input: b\\a\\c\\k\\'\\\\\\\"s\\l\\a\\s\\h\\es\n+# expected: back'\\\"slashes\n+test_expect_success '__git_dequote - backslash escaped' '\n+\t__git_dequote \"b\\a\\c\\k\\\\'\\''\\\\\\\\\\\\\\\"s\\l\\a\\s\\h\\es\" &&\n+\tverbose test \"back'\\''\\\\\\\"slashes\" = \"$dequoted_word\"\n+'\n+\n+# input: sin'gle\\' '\"quo'ted\n+# expected: single\\ \"quoted\n+test_expect_success '__git_dequote - single quoted' '\n+\t__git_dequote \"'\"sin'gle\\\\\\\\' '\\\\\\\"quo'ted\"'\" &&\n+\tverbose test '\\''single\\ \"quoted'\\'' = \"$dequoted_word\"\n+'\n+\n+# input: dou\"ble\\\\\" \"\\\"\\quot\"ed\n+# expected: double\\ \"\\quoted\n+test_expect_success '__git_dequote - double quoted' '\n+\t__git_dequote '\\''dou\"ble\\\\\" \"\\\"\\quot\"ed'\\'' &&\n+\tverbose test '\\''double\\ \"\\quoted'\\'' = \"$dequoted_word\"\n+'\n+\n+# input: 'open single quote\n+test_expect_success '__git_dequote - open single quote' '\n+\t__git_dequote \"'\\''open single quote\" &&\n+\tverbose test \"open single quote\" = \"$dequoted_word\"\n+'\n+\n+# input: \"open double quote\n+test_expect_success '__git_dequote - open double quote' '\n+\t__git_dequote \"\\\"open double quote\" &&\n+\tverbose test \"open double quote\" = \"$dequoted_word\"\n+'\n+\n+\n test_expect_success '__gitcomp_direct - puts everything into COMPREPLY as-is' '\n \tsed -e \"s/Z$//g\" >expected <<-EOF &&\n \twith-trailing-space Z\n@@ -1437,7 +1477,7 @@ _git_test_path_comp ()\n \t__git_complete_index_file --others\n }\n \n-test_expect_failure 'complete files - escaped characters on cmdline' '\n+test_expect_success 'complete files - escaped characters on cmdline' '\n \ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n \tmkdir \"New|Dir\" &&\n \t>\"New|Dir/New&File.c\" &&\n@@ -1453,11 +1493,13 @@ test_expect_failure 'complete files - escaped characters on cmdline' '\n \t\t\t\"New|Dir/New&File.c\"\n '\n \n-test_expect_failure 'complete files - quoted characters on cmdline' '\n+test_expect_success 'complete files - quoted characters on cmdline' '\n \ttest_when_finished \"rm -r \\\"New(Dir\\\"\" &&\n \tmkdir \"New(Dir\" &&\n \t>\"New(Dir/New)File.c\" &&\n \n+\t# Testing with an opening but without a corresponding closing\n+\t# double quote is important.\n \ttest_completion \"git test-path-comp \\\"New(D\" \"New(Dir\" &&\n \ttest_completion \"git test-path-comp \\\"New(Dir/New)F\" \\\n \t\t\t\"New(Dir/New)File.c\"\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344839","messageId":"20180416224113.16993-4-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 03/11] completion: simplify prefix path component handling during path completion","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:41:07Z","receivedAt":"2018-04-16T22:41:52Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Once upon a time 'git -C \"\" cmd' errored out with \"Cannot change to\n'': No such file or directory\", therefore the completion script took\nextra steps to run 'git -C \".\" cmd' instead; see fca416a41e\n(completion: use \"git -C $there\" instead of (cd $there && git ...),\n2014-10-09).\n\nThose extra steps are not needed since 6a536e2076 (git: treat \"git -C\n''\" as a no-op when is empty, 2015-03-06), so remove\nthem.\n\nWhile at it, also simplify how the trailing '/' is appended to the\nvariable holding the prefix path components.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 36d3c6f928..72cd3add19 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -385,7 +385,7 @@ __git_ls_files_helper ()\n # slash.\n __git_index_files ()\n {\n-\tlocal root=\"${2-.}\" file\n+\tlocal root=\"$2\" file\n \n \t__git_ls_files_helper \"$root\" \"$1\" |\n \twhile read -r file; do\n@@ -406,13 +406,12 @@ __git_complete_index_file ()\n \n \tcase \"$cur_\" in\n \t?*/*)\n-\t\tpfx=\"${cur_%/*}\"\n+\t\tpfx=\"${cur_%/*}/\"\n \t\tcur_=\"${cur_##*/}\"\n-\t\tpfx=\"${pfx}/\"\n \t\t;;\n \tesac\n \n-\t__gitcomp_file \"$(__git_index_files \"$1\" ${pfx:+\"$pfx\"})\" \"$pfx\" \"$cur_\"\n+\t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\n }\n \n # Lists branches from the local repository.\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344841","messageId":"20180416224236.17078-1-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224113.16993-1-szeder.dev@gmail.com","subject":"[PATCH 10/11] completion: improve handling quoted paths in 'git ls-files's output","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:42:35Z","receivedAt":"2018-04-16T22:42:56Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"If any pathname contains backslash, double quote, tab, newline, or any\ncontrol characters, 'git ls-files' and 'git diff-index' will enclose\nthat pathname in double quotes and escape those special characters\nusing C-style one-character escape sequences or \\nnn octal values.\nThis prevents those files from being listed during git-aware path\ncompletion, because due to the quoting they will never match the\ncurrent word to be completed.\n\nExtend __git_index_files()'s 'awk' script to remove all that quoting\nand escaping from unique path components, so even paths containing\n(almost all) such special characters can be completed.\n\nPaths containing newline characters are still an issue, though. We\nuse newlines as separator character when filling the COMPREPLY array,\nso a path with one or more newline will end up split to two or more\nelements in COMPREPLY, basically breaking completion. There is\nnothing we can do about it without a significant performance hit, so\nlet's just ignore such paths for now. As far as paths with newlines\nare concerned, this isn't any different from the previous behavior,\nbecause those paths were always omitted, though in the past they were\nomitted because due to the quoting they didn't match the current word\nto be completed. Anyway, Bash's own filename completion (Meta-/) can\ncomplete even those paths, if need be.\n\nNote:\n\n - We don't dequote path components right away as they are coming in,\n because then we would have to dequote each directory name\n repeatedly, as many times as it appears in the input, i.e. as many\n times as the number of listed paths it contains. Instead, we\n dequote them at the end, as we print unique path components.\n\n - Even when a directory name itself does not contain any special\n characters, it will still be quoted if any of its trailing path\n components do. If a directory contains paths both with and\n without special characters, then the name of that directory will\n appear both quoted and unquoted in the output of 'git ls-files'\n and 'git diff-index'. Consequently, we will add such a directory\n name to the deduplicating associative array twice: once quoted and\n once unquoted.\n\n This means that we have to be careful after dequoting a directory\n name, and only print it if we haven't seen the same directory name\n unquoted.\n\n - It would be wonderful if we could just pass '-z' to those git\n commands to output \\0-separated unquoted paths, and use \\0 as\n record separator in the 'awk' script processing their output...\n this patch would be so much simpler, almost trivial even.\n Unfortunately, however, POSIX and most 'awk' implementations don't\n support \\0 as record separator (GNU awk does support it).\n\n - This patch makes the earlier change to list paths with\n 'core.quotePath=false' basically redundant, because this could\n decode any \\nnn-escaped non-ASCII character just fine, as well.\n However, I suspect that 'git ls-files' can deal with those\n non-ASCII characters faster than this updated 'awk' script; just\n in case someone is burdened with tons of pathnames containing\n non-ASCII characters.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 66 +++++++++++++++++++++++++-\n t/t9902-completion.sh | 17 ++++++-\n 2 files changed, 79 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 70bc75dfc7..e97d57024b 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -459,8 +459,70 @@ __git_index_files ()\n \t\tpaths[$1] = 1\n \t}\n \tEND {\n-\t\tfor (p in paths)\n-\t\t\tprint p\n+\t\tfor (p in paths) {\n+\t\t\tif (substr(p, 1, 1) != \"\\\"\") {\n+\t\t\t\t# No special characters, easy!\n+\t\t\t\tprint p\n+\t\t\t\tcontinue\n+\t\t\t}\n+\n+\t\t\t# The path is quoted.\n+\t\t\tp = dequote(p)\n+\t\t\tif (p == \"\")\n+\t\t\t\tcontinue\n+\n+\t\t\t# Even when a directory name itself does not contain\n+\t\t\t# any special characters, it will still be quoted if\n+\t\t\t# any of its (stripped) trailing path components do.\n+\t\t\t# Because of this we may have seen the same direcory\n+\t\t\t# both quoted and unquoted.\n+\t\t\tif (p in paths)\n+\t\t\t\t# We have seen the same directory unquoted,\n+\t\t\t\t# skip it.\n+\t\t\t\tcontinue\n+\t\t\telse\n+\t\t\t\tprint p\n+\t\t}\n+\t}\n+\tfunction dequote(p, bs_idx, out, esc, esc_idx, dec) {\n+\t\t# Skip opening double quote.\n+\t\tp = substr(p, 2)\n+\n+\t\t# Interpret backslash escape sequences.\n+\t\twhile ((bs_idx = index(p, \"\\\\\")) != 0) {\n+\t\t\tout = out substr(p, 1, bs_idx - 1)\n+\t\t\tesc = substr(p, bs_idx + 1, 1)\n+\t\t\tp = substr(p, bs_idx + 2)\n+\n+\t\t\tif ((esc_idx = index(\"abtvfr\\\"\\\\\", esc)) != 0) {\n+\t\t\t\t# C-style one-character escape sequence.\n+\t\t\t\tout = out substr(\"\\a\\b\\t\\v\\f\\r\\\"\\\\\",\n+\t\t\t\t\t\t esc_idx, 1)\n+\t\t\t} else if (esc == \"n\") {\n+\t\t\t\t# Uh-oh, a newline character.\n+\t\t\t\t# We cant reliably put a pathname\n+\t\t\t\t# containing a newline into COMPREPLY,\n+\t\t\t\t# and the newline would create a mess.\n+\t\t\t\t# Skip this path.\n+\t\t\t\treturn \"\"\n+\t\t\t} else {\n+\t\t\t\t# Must be a \\nnn octal value, then.\n+\t\t\t\tdec = esc * 64 + \\\n+\t\t\t\t substr(p, 1, 1) * 8 + \\\n+\t\t\t\t substr(p, 2, 1)\n+\t\t\t\tout = out sprintf(\"%c\", dec)\n+\t\t\t\tp = substr(p, 3)\n+\t\t\t}\n+\t\t}\n+\t\t# Drop closing double quote, if there is one.\n+\t\t# (There isnt any if this is a directory, as it was\n+\t\t# already stripped with the trailing path components.)\n+\t\tif (substr(p, length(p), 1) == \"\\\"\")\n+\t\t\tout = out substr(p, 1, length(p) - 1)\n+\t\telse\n+\t\t\tout = out p\n+\n+\t\treturn out\n \t}'\n }\n \ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex a747998d6c..b9879576ab 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1527,7 +1527,7 @@ else\n \tsay \"Your filesystem does not allow \\\\ and \\\" in filenames.\"\n \trm -rf 'New\\Dir'\n fi\n-test_expect_failure FUNNYNAMES_BS_DQ \\\n+test_expect_success FUNNYNAMES_BS_DQ \\\n 'complete files - C-style escapes in ls-files output' '\n \ttest_when_finished \"rm -r \\\"New\\\\\\\\Dir\\\"\" &&\n \n@@ -1548,7 +1548,7 @@ else\n \tsay 'Your filesystem does not allow special separator characters (FS, GS, RS, US) in filenames.'\n \trm -rf $'New\\034Special\\035Dir'\n fi\n-test_expect_failure FUNNYNAMES_SEPARATORS \\\n+test_expect_success FUNNYNAMES_SEPARATORS \\\n 'complete files - \\nnn-escaped control characters in ls-files output' '\n \ttest_when_finished \"rm -r '$'New\\034Special\\035Dir''\" &&\n \n@@ -1562,6 +1562,19 @@ test_expect_failure FUNNYNAMES_SEPARATORS \\\n \t\t\t\"'$'New\\034Special\\035Dir/New\\036Special\\037File''\"\n '\n \n+test_expect_success FUNNYNAMES_BS_DQ \\\n+ 'complete files - removing repeated quoted path components' '\n+\ttest_when_finished rm -rf NewDir &&\n+\tmkdir NewDir && # A dirname which in itself would not be quoted ...\n+\t>NewDir/0-file &&\n+\t>NewDir/1\\\"file && # ... but here the file makes the dirname quoted ...\n+\t>NewDir/2-file &&\n+\t>NewDir/3\\\"file && # ... and here, too.\n+\n+\t# Still, we should only list it once.\n+\ttest_completion \"git test-path-comp New\" \"NewDir\"\n+'\n+\n \n test_expect_success \"completion uses completion for alias: !sh -c 'git ...'\" '\n \ttest_config alias.co \"!sh -c '\"'\"'git checkout ...'\"'\"'\" &&\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344842","messageId":"20180416224236.17078-2-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180416224236.17078-1-szeder.dev@gmail.com","subject":"[PATCH 11/11] completion: fill COMPREPLY directly when completing paths","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-16T22:42:36Z","receivedAt":"2018-04-16T22:42:59Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"During git-aware path completion, when a lot of path components have\nto be listed, a significant amount of time is spent in\n__gitcomp_file(), or more accurately in the shell loop of\n__gitcompappend(), iterating over all the path components filtering\npath components matching the current word to be completed, adding\nprefix path components, and placing the resulting matching paths into\nthe COMPREPLY array.\n\nNow, a previous patch in this series made 'git ls-files' and 'git\ndiff-index' list only paths matching the current word to be completed,\nso an additional filtering in __gitcomp_file() is not necessary\nanymore. Adding the prefix path components could be done much more\nefficiently in __git_index_files()'s 'awk' script while stripping\ntrailing path components and removing duplicates and quoting. And\nthen the resulting paths won't require any more filtering or\nprocessing before being handed over to Bash, so we could fill the\nCOMPREPLY array directly.\n\nUnfortunately, we can't simply use the __gitcomp_direct() helper\nfunction to do that, because __gitcomp_file() does one additional\nthing: it tells Bash that we are doing filename completion, so the\nshell will kindly do four important things for us:\n\n 1. Append a trailing space to all filenames.\n 2. Append a trailing '/' to all directory names.\n 3. Escape any meta, globbing, separator, etc. characters.\n 4. List only the current path component when listing possible\n completions (i.e. 'dir/subdir/f' will list 'file1', 'file2',\n etc. instead of the whole 'dir/subdir/file1',\n 'dir/subdir/file2').\n\nWhile we could let __git_index_files()'s 'awk' script take care of the\nfirst two points, the third one gets tricky, and we absolutely need\nthe shell's support for the fourth.\n\nAdd the helper function __gitcomp_file_direct(), which, just like\n__gitcomp_direct(), fills the COMPREPLY array with prefiltered and\npreprocessed paths without any additional processing, without a shell\nloop, with just one single compound assignment, and, similar to\n__gitcomp_file(), tells Bash and ZSH that we are doing filename\ncompletion. Extend __git_index_files()'s 'awk' script a bit to\nprepend any prefix path components to all listed paths. Finally,\nmodify __git_complete_index_file() to feed __git_index_files()'s\noutput to ___gitcomp_file_direct() instead of __gitcomp_file().\n\nAfter this patch there is no shell loop left in the path completion\ncode path.\n\nThis speeds up path completion when there are a lot of paths matching\nthe current word to be completed. In a pathological repository with\n100k files in a single directory, listing all those files:\n\n Before this patch, best of five, using GNU awk on Linux:\n\n $ time cur=dir/ __git_complete_index_file\n\n real 0m0.983s\n user 0m1.004s\n sys 0m0.033s\n\n After:\n\n real 0m0.313s\n user 0m0.341s\n sys 0m0.029s\n\n Difference: -68.2%\n Speedup: 3.1x\n\n To see the benefits of the whole patch series, the same command with\n v2.17.0:\n\n real 0m2.736s\n user 0m2.472s\n sys 0m0.610s\n\n Difference: -88.6%\n Speedup: 8.7x\n\nNote that this patch changes the output of the __git_index_files()\nhelper function by unconditionally prepending the prefix path\ncomponents to every listed path. This would break users' completion\nscriptlets that directly run:\n\n __gitcomp_file \"$(__git_index_files ...)\" \"$pfx\" \"$cur_\"\n\nbecause that would add the prefix path components once more.\nHowever, __git_index_files() is kind of a \"helper function of a helper\nfunction\", and users' completion scriptlets should have been using\n__git_complete_index_file() for git-aware path completion in the first\nplace, so this is likely doesn't worth worrying about.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 34 +++++++++++++++++++++++---\n contrib/completion/git-completion.zsh | 9 +++++++\n 2 files changed, 39 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex e97d57024b..360ee9ca51 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -404,6 +404,23 @@ __gitcomp_nl ()\n \t__gitcomp_nl_append \"$@\"\n }\n \n+# Fills the COMPREPLY array with prefiltered paths without any additional\n+# processing.\n+# Callers must take care of providing only paths that match the current path\n+# to be completed and adding any prefix path components, if necessary.\n+# 1: List of newline-separated matching paths, complete with all prefix\n+# path componens.\n+__gitcomp_file_direct ()\n+{\n+\tlocal IFS=$'\\n'\n+\n+\tCOMPREPLY=($1)\n+\n+\t# use a hack to enable file mode in bash < 4\n+\tcompopt -o filenames +o nospace 2>/dev/null ||\n+\tcompgen -f /non-existing-dir/ > /dev/null\n+}\n+\n # Generates completion reply with compgen from newline-separated possible\n # completion filenames.\n # It accepts 1 to 3 arguments:\n@@ -455,14 +472,14 @@ __git_index_files ()\n \tlocal root=\"$2\" match=\"$3\"\n \n \t__git_ls_files_helper \"$root\" \"$1\" \"$match\" |\n-\tawk -F / '{\n+\tawk -F / -v pfx=\"${2//\\\\/\\\\\\\\}\" '{\n \t\tpaths[$1] = 1\n \t}\n \tEND {\n \t\tfor (p in paths) {\n \t\t\tif (substr(p, 1, 1) != \"\\\"\") {\n \t\t\t\t# No special characters, easy!\n-\t\t\t\tprint p\n+\t\t\t\tprint pfx p\n \t\t\t\tcontinue\n \t\t\t}\n \n@@ -481,7 +498,7 @@ __git_index_files ()\n \t\t\t\t# skip it.\n \t\t\t\tcontinue\n \t\t\telse\n-\t\t\t\tprint p\n+\t\t\t\tprint pfx p\n \t\t}\n \t}\n \tfunction dequote(p, bs_idx, out, esc, esc_idx, dec) {\n@@ -545,7 +562,7 @@ __git_complete_index_file ()\n \t\tcur_=\"$dequoted_word\"\n \tesac\n \n-\t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\" \"$cur_\")\" \"$pfx\" \"$cur_\"\n+\t__gitcomp_file_direct \"$(__git_index_files \"$1\" \"$pfx\" \"$cur_\")\"\n }\n \n # Lists branches from the local repository.\n@@ -3311,6 +3328,15 @@ if [[ -n ${ZSH_VERSION-} ]]; then\n \t\tcompadd -Q -S \"${4- }\" -p \"${2-}\" -- ${=1} && _ret=0\n \t}\n \n+\t__gitcomp_file_direct ()\n+\t{\n+\t\temulate -L zsh\n+\n+\t\tlocal IFS=$'\\n'\n+\t\tcompset -P '*[=:]'\n+\t\tcompadd -Q -f -- ${=1} && _ret=0\n+\t}\n+\n \t__gitcomp_file ()\n \t{\n \t\temulate -L zsh\ndiff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh\nindex c3521fbfc4..53cb0f934f 100644\n--- a/contrib/completion/git-completion.zsh\n+++ b/contrib/completion/git-completion.zsh\n@@ -93,6 +93,15 @@ __gitcomp_nl_append ()\n \tcompadd -Q -S \"${4- }\" -p \"${2-}\" -- ${=1} && _ret=0\n }\n \n+__gitcomp_file_direct ()\n+{\n+\temulate -L zsh\n+\n+\tlocal IFS=$'\\n'\n+\tcompset -P '*[=:]'\n+\tcompadd -Q -f -- ${=1} && _ret=0\n+}\n+\n __gitcomp_file ()\n {\n \temulate -L zsh\n-- \n2.17.0.366.gbe216a3084\n\n"},{"id":"344862","messageId":"xmqq7ep6v6ft.fsf@gitster-ct.c.googlers.com","threadId":"48065","inReplyTo":"20180416224113.16993-2-szeder.dev@gmail.com","subject":"Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-17T03:48:54Z","receivedAt":"2018-04-17T03:49:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> Do any more new tests need FUNNYNAMES* prereq?\n\nHmph, all of these look like they involve some funnynames ;-)\n\n> +test_expect_failure 'complete files - escaped characters on cmdline' '\n> +\ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n> +\tmkdir \"New|Dir\" &&\n> +\t>\"New|Dir/New&File.c\" &&\n> +\n> +\ttest_completion \"git test-path-comp N\" \\\n> +\t\t\t\"New|Dir\" &&\t# Bash will turn this into \"New\\|Dir/\"\n> +\ttest_completion \"git test-path-comp New\\\\|D\" \\\n> +\t\t\t\"New|Dir\" &&\n> +\ttest_completion \"git test-path-comp New\\\\|Dir/N\" \\\n> +\t\t\t\"New|Dir/New&File.c\" &&\t# Bash will turn this into\n> +\t\t\t\t\t\t# \"New\\|Dir/New\\&File.c \"\n> +\ttest_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n> +\t\t\t\"New|Dir/New&File.c\"\n> +'\n> +\n> +test_expect_failure 'complete files - quoted characters on cmdline' '\n> +\ttest_when_finished \"rm -r \\\"New(Dir\\\"\" &&\n\nThis does not use -rf unlike the previous one?\n\n> +\tmkdir \"New(Dir\" &&\n> +\t>\"New(Dir/New)File.c\" &&\n> +\n> +\ttest_completion \"git test-path-comp \\\"New(D\" \"New(Dir\" &&\n> +\ttest_completion \"git test-path-comp \\\"New(Dir/New)F\" \\\n> +\t\t\t\"New(Dir/New)File.c\"\n> +'\n\nOK.\n\n> +test_expect_failure 'complete files - UTF-8 in ls-files output' '\n> +\ttest_when_finished \"rm -r árvíztűrő\" &&\n> +\tmkdir árvíztűrő &&\n> +\t>\"árvíztűrő/Сайн яваарай\" &&\n> +\n> +\ttest_completion \"git test-path-comp á\" \"árvíztűrő\" &&\n> +\ttest_completion \"git test-path-comp árvíztűrő/С\" \\\n> +\t\t\t\"árvíztűrő/Сайн яваарай\"\n> +'\n> +\n> +if test_have_prereq !MINGW &&\n> + mkdir 'New\\Dir' 2>/dev/null &&\n> + touch 'New\\Dir/New\"File.c' 2>/dev/null\n> +then\n> +\ttest_set_prereq FUNNYNAMES_BS_DQ\n> +else\n> +\tsay \"Your filesystem does not allow \\\\ and \\\" in filenames.\"\n> +\trm -rf 'New\\Dir'\n> +fi\n> +test_expect_failure FUNNYNAMES_BS_DQ \\\n> + 'complete files - C-style escapes in ls-files output' '\n> +\ttest_when_finished \"rm -r \\\"New\\\\\\\\Dir\\\"\" &&\n> +\n> +\ttest_completion \"git test-path-comp N\" \"New\\\\Dir\" &&\n> +\ttest_completion \"git test-path-comp New\\\\\\\\D\" \"New\\\\Dir\" &&\n> +\ttest_completion \"git test-path-comp New\\\\\\\\Dir/N\" \\\n> +\t\t\t\"New\\\\Dir/New\\\"File.c\" &&\n> +\ttest_completion \"git test-path-comp New\\\\\\\\Dir/New\\\\\\\"F\" \\\n> +\t\t\t\"New\\\\Dir/New\\\"File.c\"\n> +'\n> +\n> +if test_have_prereq !MINGW &&\n> + mkdir $'New\\034Special\\035Dir' 2>/dev/null &&\n> + touch $'New\\034Special\\035Dir/New\\036Special\\037File' 2>/dev/null\n\nThe $'...' quote is bash-ism, but this is about testing bash\ncompletion, so as long as we make sure non-bash shells won't touch\nthis part of the test, it is OK.\n\nDo we want to test a more common case of a filename that is two\nwords with SP in between, i.e. \n\n\t$ >'hello world' && git add hel\n\nor is it known to work just fine without quoting/escaping (because\nthe funny we care about is output from ls-files and SP is not special\nin its one-item-at-a-time-on-a-line output) and not worth checking?\n"},{"id":"344954","messageId":"CAM0VKjk=JtdoduywJ4t5OPhLGgt90yxJA_Zif6R803XHA=Sfbg@mail.gmail.com","threadId":"48065","inReplyTo":"xmqq7ep6v6ft.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-17T23:32:43Z","receivedAt":"2018-04-17T23:32:49Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Apr 17, 2018 at 5:48 AM, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n>\n>> Do any more new tests need FUNNYNAMES* prereq?\n>\n> Hmph, all of these look like they involve some funnynames ;-)\n\nWell, I can' create a directory with a '|' in its name on FAT32 (on\nLinux), so this needs FUNNYNAMES prereq, too.\n\n>> +test_expect_failure 'complete files - escaped characters on cmdline' '\n>> + test_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n>> + mkdir \"New|Dir\" &&\n>> + >\"New|Dir/New&File.c\" &&\n>> +\n>> + test_completion \"git test-path-comp N\" \\\n>> + \"New|Dir\" && # Bash will turn this into \"New\\|Dir/\"\n>> + test_completion \"git test-path-comp New\\\\|D\" \\\n>> + \"New|Dir\" &&\n>> + test_completion \"git test-path-comp New\\\\|Dir/N\" \\\n>> + \"New|Dir/New&File.c\" && # Bash will turn this into\n>> + # \"New\\|Dir/New\\&File.c \"\n>> + test_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n>> + \"New|Dir/New&File.c\"\n>> +'\n>> +\n>> +test_expect_failure 'complete files - quoted characters on cmdline' '\n>> + test_when_finished \"rm -r \\\"New(Dir\\\"\" &&\n>\n> This does not use -rf unlike the previous one?\n\nNoted.\n\nThe lack of '-f' is leftover from early versions of these tests, when I\nhad a hard time getting the quoting-escaping right. Without the '-f'\n'rm' errored out when I messed up, and the error message helpfully\ncontained the path it wasn't able to delete.\n\n>> + mkdir \"New(Dir\" &&\n>> + >\"New(Dir/New)File.c\" &&\n>> +\n>> + test_completion \"git test-path-comp \\\"New(D\" \"New(Dir\" &&\n>> + test_completion \"git test-path-comp \\\"New(Dir/New)F\" \\\n>> + \"New(Dir/New)File.c\"\n>> +'\n>\n> OK.\n>\n>> +test_expect_failure 'complete files - UTF-8 in ls-files output' '\n>> + test_when_finished \"rm -r árvíztűrő\" &&\n>> + mkdir árvíztűrő &&\n>> + >\"árvíztűrő/Сайн яваарай\" &&\n>> +\n>> + test_completion \"git test-path-comp á\" \"árvíztűrő\" &&\n>> + test_completion \"git test-path-comp árvíztűrő/С\" \\\n>> + \"árvíztűrő/Сайн яваарай\"\n>> +'\n>> +\n>> +if test_have_prereq !MINGW &&\n>> + mkdir 'New\\Dir' 2>/dev/null &&\n>> + touch 'New\\Dir/New\"File.c' 2>/dev/null\n>> +then\n>> + test_set_prereq FUNNYNAMES_BS_DQ\n>> +else\n>> + say \"Your filesystem does not allow \\\\ and \\\" in filenames.\"\n>> + rm -rf 'New\\Dir'\n>> +fi\n>> +test_expect_failure FUNNYNAMES_BS_DQ \\\n>> + 'complete files - C-style escapes in ls-files output' '\n>> + test_when_finished \"rm -r \\\"New\\\\\\\\Dir\\\"\" &&\n>> +\n>> + test_completion \"git test-path-comp N\" \"New\\\\Dir\" &&\n>> + test_completion \"git test-path-comp New\\\\\\\\D\" \"New\\\\Dir\" &&\n>> + test_completion \"git test-path-comp New\\\\\\\\Dir/N\" \\\n>> + \"New\\\\Dir/New\\\"File.c\" &&\n>> + test_completion \"git test-path-comp New\\\\\\\\Dir/New\\\\\\\"F\" \\\n>> + \"New\\\\Dir/New\\\"File.c\"\n>> +'\n>> +\n>> +if test_have_prereq !MINGW &&\n>> + mkdir $'New\\034Special\\035Dir' 2>/dev/null &&\n>> + touch $'New\\034Special\\035Dir/New\\036Special\\037File' 2>/dev/null\n>\n> The $'...' quote is bash-ism, but this is about testing bash\n> completion, so as long as we make sure non-bash shells won't touch\n> this part of the test, it is OK.\n>\n> Do we want to test a more common case of a filename that is two\n> words with SP in between, i.e.\n>\n> $ >'hello world' && git add hel\n>\n> or is it known to work just fine without quoting/escaping (because\n> the funny we care about is output from ls-files and SP is not special\n> in its one-item-at-a-time-on-a-line output) and not worth checking?\n\nThis particular case already works, even without this patch series.\n\nThe problems start when you want to complete the filename after a space,\ne.g. 'hello\\ w wrote:\n> On Tue, Apr 17, 2018 at 5:48 AM, Junio C Hamano wrote:\n>> SZEDER Gábor writes:\n>>\n>>> Do any more new tests need FUNNYNAMES* prereq?\n>>\n>> Hmph, all of these look like they involve some funnynames ;-)\n>\n> Well, I can' create a directory with a '|' in its name on FAT32 (on\n> Linux), so this needs FUNNYNAMES prereq, too.\n\nOr, on second thought, using a different, more widely usable character\nwould be better, so the test can be run on more platforms.\n\n\n>> Do we want to test a more common case of a filename that is two\n>> words with SP in between, i.e.\n>>\n>> $ >'hello world' && git add hel\n>>\n>> or is it known to work just fine without quoting/escaping (because\n>> the funny we care about is output from ls-files and SP is not special\n>> in its one-item-at-a-time-on-a-line output) and not worth checking?\n>\n> This particular case already works, even without this patch series.\n>\n> The problems start when you want to complete the filename after a space,\n> e.g. 'hello\\ w was the first thing I tried to write a test for, but it didn't work out:\n> inside the 'test_completion' helper function the space acts as\n> separator, and the completion script then sees 'hello\\' and 'w' as two\n> separate words.\n\nOn another second thought, a test for the already working 'hel'\ncase could make sure that we won't mess up the value of IFS when filling\nCOMPREPLY.\n"},{"id":"344964","messageId":"xmqqlgdljok5.fsf@gitster-ct.c.googlers.com","threadId":"48065","inReplyTo":"CAM0VKjk=JtdoduywJ4t5OPhLGgt90yxJA_Zif6R803XHA=Sfbg@mail.gmail.com","subject":"Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-18T01:22:50Z","receivedAt":"2018-04-18T01:22:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>>> +test_expect_failure 'complete files - quoted characters on cmdline' '\n>>> + test_when_finished \"rm -r \\\"New(Dir\\\"\" &&\n>>\n>> This does not use -rf unlike the previous one?\n>\n> Noted.\n>\n> The lack of '-f' is leftover from early versions of these tests, when I\n> had a hard time getting the quoting-escaping right. Without the '-f'\n> 'rm' errored out when I messed up, and the error message helpfully\n> contained the path it wasn't able to delete.\n\nSounds like we do not want 'f' from both tests, then? I think it is\nOK either way.\n\n>>> +if test_have_prereq !MINGW &&\n>>> + mkdir $'New\\034Special\\035Dir' 2>/dev/null &&\n>>> + touch $'New\\034Special\\035Dir/New\\036Special\\037File' 2>/dev/null\n>>\n>> The $'...' quote is bash-ism, but this is about testing bash\n>> completion, so as long as we make sure non-bash shells won't touch\n>> this part of the test, it is OK.\n>>\n>> Do we want to test a more common case of a filename that is two\n>> words with SP in between, i.e.\n>>\n>> $ >'hello world' && git add hel\n>>\n>> or is it known to work just fine without quoting/escaping (because\n>> the funny we care about is output from ls-files and SP is not special\n>> in its one-item-at-a-time-on-a-line output) and not worth checking?\n>\n> This particular case already works, even without this patch series.\n\nI was more wondering about preventing regressions---\"it worked\nwithout this patch series, but now it is broken\" is what I was\nworried about.\n\n> The problems start when you want to complete the filename after a space,\n> e.g. 'hello\\ w was the first thing I tried to write a test for, but it didn't work out:\n> inside the 'test_completion' helper function the space acts as\n> separator, and the completion script then sees 'hello\\' and 'w' as two\n> separate words.\n\nHmph. That is somewhat unfortunate.\n"},{"id":"344980","messageId":"nycvar.QRO.7.76.6.1804181421590.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48065","inReplyTo":"20180416224113.16993-2-szeder.dev@gmail.com","subject":"Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-18T12:31:04Z","receivedAt":"2018-04-18T12:31:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Tue, 17 Apr 2018, SZEDER Gábor wrote:\n\n> Completion functions see all words on the command line verbatim,\n> including any backslash-escapes, single and double quotes that might\n> be there. Furthermore, git commands quote pathnames if they contain\n> certain special characters. All these create various issues when\n> doing git-aware path completion.\n> \n> Add a couple of failing tests to demonstrate these issues.\n> \n> Later patches in this series will discuss these issues in detail as\n> they fix them.\n> \n> Signed-off-by: SZEDER Gábor \n> ---\n> \n> Notes:\n> Do any more new tests need FUNNYNAMES* prereq?\n\nYes.\n\n> t/t9902-completion.sh | 91 +++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 91 insertions(+)\n> \n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index b7f5b1e632..ff2e4a8f5f 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -1427,6 +1427,97 @@ test_expect_success 'complete files' '\n> \ttest_completion \"git add mom\" \"momified\"\n> '\n> \n> +# The next tests only care about how the completion script deals with\n> +# unusual characters in path names. By defining a custom completion\n> +# function to list untracked files they won't be influenced by future\n> +# changes of the completion functions of real git commands, and we\n> +# don't have to bother with adding files to the index in these tests.\n> +_git_test_path_comp ()\n> +{\n> +\t__git_complete_index_file --others\n> +}\n> +\n> +test_expect_failure 'complete files - escaped characters on cmdline' '\n> +\ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n> +\tmkdir \"New|Dir\" &&\n> +\t>\"New|Dir/New&File.c\" &&\n> +\n> +\ttest_completion \"git test-path-comp N\" \\\n> +\t\t\t\"New|Dir\" &&\t# Bash will turn this into \"New\\|Dir/\"\n> +\ttest_completion \"git test-path-comp New\\\\|D\" \\\n> +\t\t\t\"New|Dir\" &&\n> +\ttest_completion \"git test-path-comp New\\\\|Dir/N\" \\\n> +\t\t\t\"New|Dir/New&File.c\" &&\t# Bash will turn this into\n> +\t\t\t\t\t\t# \"New\\|Dir/New\\&File.c \"\n> +\ttest_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n> +\t\t\t\"New|Dir/New&File.c\"\n> +'\n\nThis fails with:\n\n2018-04-18T11:12:55.0436371Z expecting success: \n2018-04-18T11:12:55.0436665Z \ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n2018-04-18T11:12:55.0436799Z \tmkdir \"New|Dir\" &&\n2018-04-18T11:12:55.0436904Z \t>\"New|Dir/New&File.c\" &&\n2018-04-18T11:12:55.0436972Z \n2018-04-18T11:12:55.0437158Z \ttest_completion \"git test-path-comp N\" \\\n2018-04-18T11:12:55.0437296Z \t\t\t\"New|Dir\" &&\t# Bash will turn this into \"New\\|Dir/\"\n2018-04-18T11:12:55.0437413Z \ttest_completion \"git test-path-comp New\\\\|D\" \\\n2018-04-18T11:12:55.0437522Z \t\t\t\"New|Dir\" &&\n2018-04-18T11:12:55.0437629Z \ttest_completion \"git test-path-comp New\\\\|Dir/N\" \\\n2018-04-18T11:12:55.0437767Z \t\t\t\"New|Dir/New&File.c\" &&\t# Bash will turn this into\n2018-04-18T11:12:55.0438040Z \t\t\t\t\t\t# \"New\\|Dir/New\\&File.c \"\n2018-04-18T11:12:55.0438152Z \ttest_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n2018-04-18T11:12:55.0438504Z \t\t\t\"New|Dir/New&File.c\"\n2018-04-18T11:12:55.0438742Z \n2018-04-18T11:12:55.0590984Z ++ test_when_finished 'rm -rf \"New|Dir\"'\n2018-04-18T11:12:55.0591722Z ++ test 0 = 0\n2018-04-18T11:12:55.0592001Z ++ test_cleanup='{ rm -rf \"New|Dir\"\n2018-04-18T11:12:55.0592290Z \t\t} && (exit \"$eval_ret\"); eval_ret=$?; :'\n2018-04-18T11:12:55.0592472Z ++ mkdir 'New|Dir'\n2018-04-18T11:12:55.0717255Z ++ test_completion 'git test-path-comp N' 'New|Dir'\n2018-04-18T11:12:55.0717680Z ++ test 2 -gt 1\n2018-04-18T11:12:55.0718062Z ++ printf '%s\\n' 'New|Dir'\n2018-04-18T11:12:55.0718275Z ++ run_completion 'git test-path-comp N'\n2018-04-18T11:12:55.0718447Z ++ local -a COMPREPLY _words\n2018-04-18T11:12:55.0718631Z ++ local _cword\n2018-04-18T11:12:55.0718806Z ++ _words=($1)\n2018-04-18T11:12:55.0718965Z ++ test N = ' '\n2018-04-18T11:12:55.0719124Z ++ (( _cword = 3 - 1 ))\n2018-04-18T11:12:55.0719286Z ++ __git_wrap__git_main\n2018-04-18T11:12:55.0719467Z ++ __git_func_wrap __git_main\n2018-04-18T11:12:55.0719633Z ++ local cur words cword prev\n2018-04-18T11:12:55.0719801Z ++ _get_comp_words_by_ref -n =: cur words cword prev\n2018-04-18T11:12:55.0720074Z ++ '[' 6 -gt 0 ']'\n2018-04-18T11:12:55.0720239Z ++ case \"$1\" in\n2018-04-18T11:12:55.0720406Z ++ shift\n2018-04-18T11:12:55.0720584Z ++ '[' 5 -gt 0 ']'\n2018-04-18T11:12:55.0720742Z ++ case \"$1\" in\n2018-04-18T11:12:55.0720899Z ++ shift\n2018-04-18T11:12:55.0721054Z ++ '[' 4 -gt 0 ']'\n2018-04-18T11:12:55.0721240Z ++ case \"$1\" in\n2018-04-18T11:12:55.0721392Z ++ cur=N\n2018-04-18T11:12:55.0721547Z ++ shift\n2018-04-18T11:12:55.0721717Z ++ '[' 3 -gt 0 ']'\n2018-04-18T11:12:55.0721879Z ++ case \"$1\" in\n2018-04-18T11:12:55.0722040Z ++ words=(\"${_words[@]}\")\n2018-04-18T11:12:55.0722201Z ++ shift\n2018-04-18T11:12:55.0722396Z ++ '[' 2 -gt 0 ']'\n2018-04-18T11:12:55.0722931Z ++ case \"$1\" in\n2018-04-18T11:12:55.0723070Z ++ cword=2\n2018-04-18T11:12:55.0723221Z ++ shift\n2018-04-18T11:12:55.0723357Z ++ '[' 1 -gt 0 ']'\n2018-04-18T11:12:55.0723575Z ++ case \"$1\" in\n2018-04-18T11:12:55.0723735Z ++ prev=test-path-comp\n2018-04-18T11:12:55.0723874Z ++ shift\n2018-04-18T11:12:55.0724009Z ++ '[' 0 -gt 0 ']'\n2018-04-18T11:12:55.0724397Z ++ __git_main\n2018-04-18T11:12:55.0724984Z ++ local i c=1 command __git_dir __git_repo_path\n2018-04-18T11:12:55.0725183Z ++ local __git_C_args C_args_count=0\n2018-04-18T11:12:55.0725353Z ++ '[' 1 -lt 2 ']'\n2018-04-18T11:12:55.0725537Z ++ i=test-path-comp\n2018-04-18T11:12:55.0725712Z ++ case \"$i\" in\n2018-04-18T11:12:55.0725882Z ++ command=test-path-comp\n2018-04-18T11:12:55.0726057Z ++ break\n2018-04-18T11:12:55.0726270Z ++ '[' -z test-path-comp ']'\n2018-04-18T11:12:55.0726446Z ++ __git_complete_command test-path-comp\n2018-04-18T11:12:55.0726621Z ++ local command=test-path-comp\n2018-04-18T11:12:55.0726816Z ++ local completion_func=_git_test_path_comp\n2018-04-18T11:12:55.0726992Z ++ declare -f _git_test_path_comp\n2018-04-18T11:12:55.0727353Z ++ declare -f _git_test_path_comp\n2018-04-18T11:12:55.0727547Z ++ _git_test_path_comp\n2018-04-18T11:12:55.0727716Z ++ __git_complete_index_file --others\n2018-04-18T11:12:55.0727890Z ++ local dequoted_word pfx= cur_\n2018-04-18T11:12:55.0728234Z ++ __git_dequote N\n2018-04-18T11:12:55.0728418Z ++ local rest=N len ch\n2018-04-18T11:12:55.0728869Z ++ dequoted_word=\n2018-04-18T11:12:55.0729020Z ++ test -n N\n2018-04-18T11:12:55.0729152Z ++ len=0\n2018-04-18T11:12:55.0729309Z ++ dequoted_word=N\n2018-04-18T11:12:55.0729440Z ++ rest=\n2018-04-18T11:12:55.0729666Z ++ case \"${rest:0:1}\" in\n2018-04-18T11:12:55.0729822Z ++ test -n ''\n2018-04-18T11:12:55.0729993Z ++ case \"$dequoted_word\" in\n2018-04-18T11:12:55.0730133Z ++ cur_=N\n2018-04-18T11:12:55.0782504Z +++ __git_index_files --others '' N\n2018-04-18T11:12:55.0782805Z +++ local root= match=N\n2018-04-18T11:12:55.0845235Z +++ __git_ls_files_helper '' --others N\n2018-04-18T11:12:55.0845440Z +++ '[' --others == --committable ']'\n2018-04-18T11:12:55.0845567Z +++ __git -C '' -c core.quotePath=false ls-files --exclude-standard --others -- 'N*'\n2018-04-18T11:12:55.0845706Z +++ git -C '' -c core.quotePath=false ls-files --exclude-standard --others -- 'N*'\n2018-04-18T11:12:55.0907632Z +++ awk -F / -v pfx= '{\n2018-04-18T11:12:55.0907806Z \t\tpaths[$1] = 1\n2018-04-18T11:12:55.0908985Z \t}\n2018-04-18T11:12:55.0942839Z \tEND {\n2018-04-18T11:12:55.0943072Z \t\tfor (p in paths) {\n2018-04-18T11:12:55.0949175Z \t\t\tif (substr(p, 1, 1) != \"\\\"\") {\n2018-04-18T11:12:55.0949458Z \t\t\t\t# No special characters, easy!\n2018-04-18T11:12:55.0949659Z \t\t\t\tprint pfx p\n2018-04-18T11:12:55.0949823Z \t\t\t\tcontinue\n2018-04-18T11:12:55.0949999Z \t\t\t}\n2018-04-18T11:12:55.0950121Z \n2018-04-18T11:12:55.0950335Z \t\t\t# The path is quoted.\n2018-04-18T11:12:55.0950829Z \t\t\tp = dequote(p)\n2018-04-18T11:12:55.0951171Z \t\t\tif (p == \"\")\n2018-04-18T11:12:55.0951555Z \t\t\t\tcontinue\n2018-04-18T11:12:55.0951672Z \n2018-04-18T11:12:55.0951856Z \t\t\t# Even when a directory name itself does not contain\n2018-04-18T11:12:55.0952038Z \t\t\t# any special characters, it will still be quoted if\n2018-04-18T11:12:55.0952213Z \t\t\t# any of its (stripped) trailing path components do.\n2018-04-18T11:12:55.0952407Z \t\t\t# Because of this we may have seen the same direcory\n2018-04-18T11:12:55.0952583Z \t\t\t# both quoted and unquoted.\n2018-04-18T11:12:55.0952762Z \t\t\tif (p in paths)\n2018-04-18T11:12:55.0952948Z \t\t\t\t# We have seen the same directory unquoted,\n2018-04-18T11:12:55.0953117Z \t\t\t\t# skip it.\n2018-04-18T11:12:55.0953276Z \t\t\t\tcontinue\n2018-04-18T11:12:55.0953441Z \t\t\telse\n2018-04-18T11:12:55.0953613Z \t\t\t\tprint pfx p\n2018-04-18T11:12:55.0953766Z \t\t}\n2018-04-18T11:12:55.0953914Z \t}\n2018-04-18T11:12:55.0954461Z \tfunction dequote(p, bs_idx, out, esc, esc_idx, dec) {\n2018-04-18T11:12:55.0954650Z \t\t# Skip opening double quote.\n2018-04-18T11:12:55.0954813Z \t\tp = substr(p, 2)\n2018-04-18T11:12:55.0954935Z \n2018-04-18T11:12:55.0955237Z \t\t# Interpret backslash escape sequences.\n2018-04-18T11:12:55.0955415Z \t\twhile ((bs_idx = index(p, \"\\\\\")) != 0) {\n2018-04-18T11:12:55.0955533Z \t\t\tout = out substr(p, 1, bs_idx - 1)\n2018-04-18T11:12:55.0955638Z \t\t\tesc = substr(p, bs_idx + 1, 1)\n2018-04-18T11:12:55.0955743Z \t\t\tp = substr(p, bs_idx + 2)\n2018-04-18T11:12:55.0955830Z \n2018-04-18T11:12:55.0955939Z \t\t\tif ((esc_idx = index(\"abtvfr\\\"\\\\\", esc)) != 0) {\n2018-04-18T11:12:55.0956079Z \t\t\t\t# C-style one-character escape sequence.\n2018-04-18T11:12:55.0956513Z \t\t\t\tout = out substr(\"\\a\\b\\t\\v\\f\\r\\\"\\\\\",\n2018-04-18T11:12:55.0956631Z esc_idx, 1)\n2018-04-18T11:12:55.0956745Z \t\t\t} else if (esc == \"n\") {\n2018-04-18T11:12:55.0956853Z \t\t\t\t# Uh-oh, a newline character.\n2018-04-18T11:12:55.0956973Z \t\t\t\t# We cant reliably put a pathname\n2018-04-18T11:12:55.0957086Z \t\t\t\t# containing a newline into COMPREPLY,\n2018-04-18T11:12:55.0957193Z \t\t\t\t# and the newline would create a mess.\n2018-04-18T11:12:55.0957300Z \t\t\t\t# Skip this path.\n2018-04-18T11:12:55.0957413Z \t\t\t\treturn \"\"\n2018-04-18T11:12:55.0957510Z \t\t\t} else {\n2018-04-18T11:12:55.0957808Z \t\t\t\t# Must be a \\nnn octal value, then.\n2018-04-18T11:12:55.0958070Z \t\t\t\tdec = esc * 64 + \\\n2018-04-18T11:12:55.0958184Z \t\t\t\t substr(p, 1, 1) * 8 + \\\n2018-04-18T11:12:55.0958274Z \t\t\t\t substr(p, 2, 1)\n2018-04-18T11:12:55.0958369Z \t\t\t\tout = out sprintf(\"%c\", dec)\n2018-04-18T11:12:55.0958587Z \t\t\t\tp = substr(p, 3)\n2018-04-18T11:12:55.0958692Z \t\t\t}\n2018-04-18T11:12:55.0958769Z \t\t}\n2018-04-18T11:12:55.0958862Z \t\t# Drop closing double quote, if there is one.\n2018-04-18T11:12:55.0958969Z \t\t# (There isnt any if this is a directory, as it was\n2018-04-18T11:12:55.0959153Z \t\t# already stripped with the trailing path components.)\n2018-04-18T11:12:55.0959256Z \t\tif (substr(p, length(p), 1) == \"\\\"\")\n2018-04-18T11:12:55.0959356Z \t\t\tout = out substr(p, 1, length(p) - 1)\n2018-04-18T11:12:55.0959441Z \t\telse\n2018-04-18T11:12:55.0959541Z \t\t\tout = out p\n2018-04-18T11:12:55.0959598Z \n2018-04-18T11:12:55.0959682Z \t\treturn out\n2018-04-18T11:12:55.0959763Z \t}'\n2018-04-18T11:12:55.1182135Z ++ __gitcomp_file_direct $'New∩\\201╝Dir'\n2018-04-18T11:12:55.1182355Z ++ local 'IFS=\n2018-04-18T11:12:55.1182439Z '\n2018-04-18T11:12:55.1182518Z ++ COMPREPLY=($1)\n2018-04-18T11:12:55.1182622Z ++ compopt -o filenames +o nospace\n2018-04-18T11:12:55.1182877Z ++ compgen -f /non-existing-dir/\n2018-04-18T11:12:55.1182979Z ++ return 0\n2018-04-18T11:12:55.1183055Z ++ return\n2018-04-18T11:12:55.1183147Z ++ print_comp\n2018-04-18T11:12:55.1183224Z ++ local 'IFS=\n2018-04-18T11:12:55.1183300Z '\n2018-04-18T11:12:55.1183398Z ++ echo $'New∩\\201╝Dir'\n2018-04-18T11:12:55.1183508Z ++ sort out\n2018-04-18T11:12:55.1183605Z ++ /usr/bin/sort out\n2018-04-18T11:12:55.1306331Z ++ test_cmp expected out_sorted\n2018-04-18T11:12:55.1306825Z ++ mingw_test_cmp expected out_sorted\n2018-04-18T11:12:55.1307024Z ++ local test_cmp_a= test_cmp_b=\n2018-04-18T11:12:55.1307233Z ++ local stdin_for_diff=\n2018-04-18T11:12:55.1307401Z ++ test -s expected\n2018-04-18T11:12:55.1307568Z ++ test -s out_sorted\n2018-04-18T11:12:55.1307742Z ++ mingw_read_file_strip_cr_ test_cmp_a\n2018-04-18T11:12:55.1308083Z ++ local line\n2018-04-18T11:12:55.1308424Z ++ :\n2018-04-18T11:12:55.1308566Z ++ IFS=$'\\r'\n2018-04-18T11:12:55.1308717Z ++ read -r -d '\n2018-04-18T11:12:55.1308852Z ' line\n2018-04-18T11:12:55.1317521Z ++ line='New|Dir\n2018-04-18T11:12:55.1317784Z '\n2018-04-18T11:12:55.1318257Z ++ eval 'test_cmp_a=$test_cmp_a$line'\n2018-04-18T11:12:55.1318424Z +++ test_cmp_a='New|Dir\n2018-04-18T11:12:55.1318569Z '\n2018-04-18T11:12:55.1318724Z ++ :\n2018-04-18T11:12:55.1318871Z ++ IFS=$'\\r'\n2018-04-18T11:12:55.1319027Z ++ read -r -d '\n2018-04-18T11:12:55.1319170Z ' line\n2018-04-18T11:12:55.1319334Z ++ test -z ''\n2018-04-18T11:12:55.1319476Z ++ break\n2018-04-18T11:12:55.1319628Z ++ mingw_read_file_strip_cr_ test_cmp_b\n2018-04-18T11:12:55.1319797Z ++ local line\n2018-04-18T11:12:55.1319939Z ++ :\n2018-04-18T11:12:55.1320081Z ++ IFS=$'\\r'\n2018-04-18T11:12:55.1320240Z ++ read -r -d '\n2018-04-18T11:12:55.1320384Z ' line\n2018-04-18T11:12:55.1320555Z ++ line='New∩ü╝Dir\n2018-04-18T11:12:55.1320915Z '\n2018-04-18T11:12:55.1321099Z ++ eval 'test_cmp_b=$test_cmp_b$line'\n2018-04-18T11:12:55.1321266Z +++ test_cmp_b='New∩ü╝Dir\n2018-04-18T11:12:55.1321422Z '\n2018-04-18T11:12:55.1321570Z ++ :\n2018-04-18T11:12:55.1321705Z ++ IFS=$'\\r'\n2018-04-18T11:12:55.1321859Z ++ read -r -d '\n2018-04-18T11:12:55.1321994Z ' line\n2018-04-18T11:12:55.1322219Z ++ test -z ''\n2018-04-18T11:12:55.1322361Z ++ break\n2018-04-18T11:12:55.1322497Z ++ test -n 'New|Dir\n2018-04-18T11:12:55.1322649Z '\n2018-04-18T11:12:55.1322828Z ++ test -n 'New∩ü╝Dir\n2018-04-18T11:12:55.1322977Z '\n2018-04-18T11:12:55.1323109Z ++ test 'New|Dir\n2018-04-18T11:12:55.1323397Z ' = 'New∩ü╝Dir\n2018-04-18T11:12:55.1323540Z '\n2018-04-18T11:12:55.1323680Z ++ eval 'diff -u \"$@\" '\n2018-04-18T11:12:55.1323840Z +++ diff -u expected out_sorted\n2018-04-18T11:12:55.1454977Z --- expected\t2018-04-18 11:12:55.065444100 +0000\n2018-04-18T11:12:55.1455785Z error: last command exited with $?=1\n2018-04-18T11:12:55.1456722Z +++ out_sorted\t2018-04-18 11:12:55.127568400 +0000\n2018-04-18T11:12:55.1457211Z @@ -1 +1 @@\n2018-04-18T11:12:55.1457408Z -New|Dir\n2018-04-18T11:12:55.1457752Z +New∩ü╝Dir\n2018-04-18T11:12:55.1457975Z not ok 111 - complete files - escaped characters on cmdline\n2018-04-18T11:12:55.1645995Z #\t\n2018-04-18T11:12:55.1646221Z #\t\ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n2018-04-18T11:12:55.1646380Z #\t\tmkdir \"New|Dir\" &&\n2018-04-18T11:12:55.1646487Z #\t\t>\"New|Dir/New&File.c\" &&\n2018-04-18T11:12:55.1646583Z #\t\n2018-04-18T11:12:55.1646865Z #\t\ttest_completion \"git test-path-comp N\" \\\n2018-04-18T11:12:55.1646986Z #\t\t\t\t\"New|Dir\" &&\t# Bash will turn this into \"New\\|Dir/\"\n2018-04-18T11:12:55.1647108Z #\t\ttest_completion \"git test-path-comp New\\\\|D\" \\\n2018-04-18T11:12:55.1647212Z #\t\t\t\t\"New|Dir\" &&\n2018-04-18T11:12:55.1647346Z #\t\ttest_completion \"git test-path-comp New\\\\|Dir/N\" \\\n2018-04-18T11:12:55.1647510Z # \"New|Dir/New&File.c\" &&\t# Bash will turn this into\n2018-04-18T11:12:55.1647636Z # # \"New\\|Dir/New\\&File.c \"\n2018-04-18T11:12:55.1647775Z #\t\ttest_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n2018-04-18T11:12:55.1647886Z # \"New|Dir/New&File.c\"\n\nI suspect that the culprit is once again Cygwin's trick where illegal\ncharacters are mapped into a private Unicode page. Cygwin (and therefore\nMSYS2 runtime, and therefore the Bash used to run the test script) can use\nthose filenames all right, but Git cannot.\n\nSo even testing whether you could write an illegal file name via shell\nscript is *not* enough to determine whether the file system supports funny\ncharacters.\n\nAs far as I can tell from a *really* cursory glance, this is the only\naffected test case. Apparently your prereq catches, somehow, on Windows:\n\n2018-04-18T11:12:43.0459702Z Your filesystem does not allow \\ and \" in filenames.\n2018-04-18T11:12:43.0459823Z skipped: complete files - C-style escapes in ls-files output (missing FUNNYNAMES_BS_DQ)\n\nCiao,\nDscho"},{"id":"345158","messageId":"CAM0VKjmJTBxBke7YNHGeqiUyqM5drqjs7ceuv19uhMqOPt5jiQ@mail.gmail.com","threadId":"48065","inReplyTo":"nycvar.QRO.7.76.6.1804181421590.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-19T19:08:44Z","receivedAt":"2018-04-19T19:08:48Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Apr 18, 2018 at 2:31 PM, Johannes Schindelin\n wrote:\n\n> I suspect that the culprit is once again Cygwin's trick where illegal\n> characters are mapped into a private Unicode page. Cygwin (and therefore\n> MSYS2 runtime, and therefore the Bash used to run the test script) can use\n> those filenames all right, but Git cannot.\n>\n> So even testing whether you could write an illegal file name via shell\n> script is *not* enough to determine whether the file system supports funny\n> characters.\n\nI followed suit of all existing FUNNYNAMES checks:\n\n $ git grep -B3 'test_set_prereq FUNNYNAMES' master t/\n master:t/t3600-rm.sh-if test_have_prereq !MINGW && touch -- 'tab\n embedded' 'newline\n master:t/t3600-rm.sh-embedded' 2>/dev/null\n master:t/t3600-rm.sh-then\n master:t/t3600-rm.sh: test_set_prereq FUNNYNAMES\n --\n master:t/t4135-apply-weird-filenames.sh- if test_have_prereq !MINGW &&\n master:t/t4135-apply-weird-filenames.sh- touch --\n\"tab embedded.txt\" '\\''\"quoteembedded\".txt'\\''\n master:t/t4135-apply-weird-filenames.sh- then\n master:t/t4135-apply-weird-filenames.sh:\ntest_set_prereq FUNNYNAMES\n --\n master:t/t9903-bash-prompt.sh-\n master:t/t9903-bash-prompt.sh-if test_have_prereq !MINGW && mkdir\n\"$repo_with_newline\" 2>/dev/null\n master:t/t9903-bash-prompt.sh-then\n master:t/t9903-bash-prompt.sh: test_set_prereq FUNNYNAMES\n\nHow am I supposed to check this, then?\n\n\n> As far as I can tell from a *really* cursory glance, this is the only\n> affected test case. Apparently your prereq catches, somehow, on Windows:\n>\n> 2018-04-18T11:12:43.0459702Z Your filesystem does not allow \\ and \" in filenames.\n> 2018-04-18T11:12:43.0459823Z skipped: complete files - C-style escapes in ls-files output (missing FUNNYNAMES_BS_DQ)\n"},{"id":"345848","messageId":"CAM0VKjkQfTm+qnurvZ_545VXJH2PwuPfkhXaa1sLj5ePSPjBwA@mail.gmail.com","threadId":"48065","inReplyTo":"xmqqlgdljok5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-26T00:25:34Z","receivedAt":"2018-04-26T00:25:38Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Apr 18, 2018 at 3:22 AM, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n>>> Do we want to test a more common case of a filename that is two\n>>> words with SP in between, i.e.\n>>>\n>>> $ >'hello world' && git add hel\n>>>\n>>> or is it known to work just fine without quoting/escaping (because\n>>> the funny we care about is output from ls-files and SP is not special\n>>> in its one-item-at-a-time-on-a-line output) and not worth checking?\n>>\n>> This particular case already works, even without this patch series.\n>\n> I was more wondering about preventing regressions---\"it worked\n> without this patch series, but now it is broken\" is what I was\n> worried about.\n>\n>> The problems start when you want to complete the filename after a space,\n>> e.g. 'hello\\ w> was the first thing I tried to write a test for, but it didn't work out:\n>> inside the 'test_completion' helper function the space acts as\n>> separator, and the completion script then sees 'hello\\' and 'w' as two\n>> separate words.\n>\n> Hmph. That is somewhat unfortunate.\n\nActually, I used 'test_completion' in these new tests, because there\nis that big test checking file completion for various commands, and it\nalready uses 'test_completion', so I just followed suit. Now, that\ntest checks that the right type(s) of files are listed for various git\ncommands, e.g. modified and untracked for 'git add', IOW that the\ncaller of __git_complete_index_file() specifies the appropriate 'git\nls-files' options. For those kind of checks 'test_completion' is\ngreat.\n\nThese new tests, however, are primarily interested in the inner\nworkings of __git_complete_index_file() in the presence of escapes\nand/or quotes in the path to be completed and/or in the output of 'git\nls-files'. For these kind of tests we could simply invoke\n__git_complete_index_file() directly, like we call __git_refs()\ndirectly to test refs completion. Then we could set the current path\nto be completed to whatever we want, including spaces, because it\nwon't be subject to field splitting like the command line given to\n'test_completion'.\n\nSo, I think for v2 I will rewrite these tests to call\n__git_complete_index_file() directly instead of using\n'test_completion', and will include a test with spaces in path names.\n"},{"id":"345853","messageId":"xmqq1sf24syg.fsf@gitster-ct.c.googlers.com","threadId":"48065","inReplyTo":"CAM0VKjkQfTm+qnurvZ_545VXJH2PwuPfkhXaa1sLj5ePSPjBwA@mail.gmail.com","subject":"Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-26T02:11:51Z","receivedAt":"2018-04-26T02:11:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> These new tests, however, are primarily interested in the inner\n> workings of __git_complete_index_file() in the presence of escapes\n> and/or quotes in the path to be completed and/or in the output of 'git\n> ls-files'. For these kind of tests we could simply invoke\n> __git_complete_index_file() directly, like we call __git_refs()\n> directly to test refs completion. Then we could set the current path\n> to be completed to whatever we want, including spaces, because it\n> won't be subject to field splitting like the command line given to\n> 'test_completion'.\n>\n> So, I think for v2 I will rewrite these tests to call\n> __git_complete_index_file() directly instead of using\n> 'test_completion', and will include a test with spaces in path names.\n\nQuite well thought-out reasoning. Thanks.\n"},{"id":"347983","messageId":"20180518141751.16350-1-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"xmqq1sf24syg.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 0/2] Test improvements for 'sg/complete-paths'","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-05-18T14:17:49Z","receivedAt":"2018-05-18T14:18:13Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"> > So, I think for v2 I will rewrite these tests to call\n> > __git_complete_index_file() directly instead of using\n> > 'test_completion', and will include a test with spaces in path names.\n> \n> Quite well thought-out reasoning. Thanks.\n\nUnfortunately I couldn't get around to it soon enough, and now the\ntopic 'sg/complete-paths' is already in next, so here are those test\nimprovements on top.\n\n\nSZEDER Gábor (2):\n completion: don't return with error from __gitcomp_file_direct()\n t9902-completion: exercise __git_complete_index_file() directly\n\n contrib/completion/git-completion.bash | 6 +-\n t/t9902-completion.sh | 225 +++++++++++++------------\n 2 files changed, 122 insertions(+), 109 deletions(-)\n\n-- \n2.17.0.799.gd371044c7c\n\n"},{"id":"347984","messageId":"20180518141751.16350-2-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180518141751.16350-1-szeder.dev@gmail.com","subject":"[PATCH 1/2] completion: don't return with error from __gitcomp_file_direct()","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-05-18T14:17:50Z","receivedAt":"2018-05-18T14:18:14Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"In __gitcomp_file_direct() we tell Bash that it should handle our\npossible completion words as filenames with the following piece of\ncleverness:\n\n # use a hack to enable file mode in bash < 4\n compopt -o filenames +o nospace 2>/dev/null ||\n compgen -f /non-existing-dir/ > /dev/null\n\nUnfortunately, this makes this function always return with error\nwhen it is not invoked in real completion, but e.g. in tests of\n't9902-completion.sh':\n\n - First the 'compopt' line errors out\n - either because in Bash v3.x there is no such command,\n - or because in Bash v4.x it complains about \"not currently\n executing completion function\",\n\n - then 'compgen' just silently returns with error because of the\n non-existing directory.\n\nSince __gitcomp_file_direct() is now the last command executed in\n__git_complete_index_file(), that function returns with error as well,\nwhich prevents it from being invoked in tests directly as is, and\nwould require extra steps in test to hide its error code.\n\nSo let's make sure that __gitcomp_file_direct() doesn't return with\nerror, because in the tests coming in the following patch we do want\nto exercise __git_complete_index_file() directly,\n\n__gitcomp_file() contains the same construct, and thus it, too, always\nreturns with error. Update that function accordingly as well.\n\nWhile at it, also remove the space from between the redirection\noperator and the filename in both functions.\n\nSigned-off-by: SZEDER Gábor \n---\n contrib/completion/git-completion.bash | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 816901f0f0..8bc79a5226 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -420,7 +420,8 @@ __gitcomp_file_direct ()\n \n \t# use a hack to enable file mode in bash < 4\n \tcompopt -o filenames +o nospace 2>/dev/null ||\n-\tcompgen -f /non-existing-dir/ > /dev/null\n+\tcompgen -f /non-existing-dir/ >/dev/null ||\n+\ttrue\n }\n \n # Generates completion reply with compgen from newline-separated possible\n@@ -442,7 +443,8 @@ __gitcomp_file ()\n \n \t# use a hack to enable file mode in bash < 4\n \tcompopt -o filenames +o nospace 2>/dev/null ||\n-\tcompgen -f /non-existing-dir/ > /dev/null\n+\tcompgen -f /non-existing-dir/ >/dev/null ||\n+\ttrue\n }\n \n # Execute 'git ls-files', unless the --committable option is specified, in\n-- \n2.17.0.799.gd371044c7c\n\n"},{"id":"347985","messageId":"20180518141751.16350-3-szeder.dev@gmail.com","threadId":"48065","inReplyTo":"20180518141751.16350-1-szeder.dev@gmail.com","subject":"[PATCH 2/2] t9902-completion: exercise __git_complete_index_file() directly","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-05-18T14:17:51Z","receivedAt":"2018-05-18T14:18:17Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The tests added in 2f271cd9cf (t9902-completion: add tests\ndemonstrating issues with quoted pathnames, 2018-05-08) and in\n2ab6eab4fe (completion: improve handling quoted paths in 'git\nls-files's output, 2018-03-28) have a few shortcomings:\n\n - All these test use the 'test_completion' helper function, thus\n they are exercising the whole completion machinery, although they\n are only interested in how git-aware path completion, specifically\n the __git_complete_index_file() function deals with unusual\n characters in pathnames and on the command line.\n\n - These tests can't satisfactorily test the case of pathnames\n containing spaces, because 'test_completion' gets the words on the\n command line as a single argument and it uses space as word\n separator.\n\n - Some of the tests are protected by different FUNNYNAMES_* prereqs\n depending on whether they put backslashes and double quotes or\n separator characters (FS, GS, RS, US) in pathnames, although a\n filesystem not allowing one likely doesn't allow the others\n either.\n\n - One of the tests operates on paths containing '|' and '&'\n characters without being protected by a FUNNYNAMES prereq, but\n some filesystems (notably on Windows) don't allow these characters\n in pathnames, either.\n\nReplace these tests with basically equivalent, more focused tests that\ncall __git_complete_index_file() directly. Since this function only\nlooks at the current word to be completed, i.e. the $cur variable, we\ncan easily include pathnames containing spaces in the tests, so use\nsuch pathnames instead of pathnames containing '|' and '&'. Finally,\nuse only a single FUNNYNAMES prereq for all kinds of special\ncharacters.\n\nSigned-off-by: SZEDER Gábor \n---\n t/t9902-completion.sh | 225 ++++++++++++++++++++++--------------------\n 1 file changed, 118 insertions(+), 107 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 955932174c..1b6d275254 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1209,6 +1209,124 @@ test_expect_success 'teardown after ref completion' '\n \tgit remote remove other\n '\n \n+\n+test_path_completion ()\n+{\n+\ttest $# = 2 || error \"bug in the test script: not 2 parameters to test_path_completion\"\n+\n+\tlocal cur=\"$1\" expected=\"$2\"\n+\techo \"$expected\" >expected &&\n+\t(\n+\t\t# In the following tests calling this function we only\n+\t\t# care about how __git_complete_index_file() deals with\n+\t\t# unusual characters in path names. By requesting only\n+\t\t# untracked files we dont have to bother adding any\n+\t\t# paths to the index in those tests.\n+\t\t__git_complete_index_file --others &&\n+\t\tprint_comp\n+\t) &&\n+\ttest_cmp expected out\n+}\n+\n+test_expect_success 'setup for path completion tests' '\n+\tmkdir simple-dir \\\n+\t \"spaces in dir\" \\\n+\t árvíztűrő &&\n+\ttouch simple-dir/simple-file \\\n+\t \"spaces in dir/spaces in file\" \\\n+\t \"árvíztűrő/Сайн яваарай\" &&\n+\tif test_have_prereq !MINGW &&\n+\t mkdir BS\\\\dir \\\n+\t\t '$'separators\\034in\\035dir'' &&\n+\t touch BS\\\\dir/DQ\\\"file \\\n+\t\t '$'separators\\034in\\035dir/sep\\036in\\037file''\n+\tthen\n+\t\ttest_set_prereq FUNNYNAMES\n+\telse\n+\t\trm -rf BS\\\\dir '$'separators\\034in\\035dir''\n+\tfi\n+'\n+\n+test_expect_success '__git_complete_index_file - simple' '\n+\ttest_path_completion simple simple-dir && # Bash is supposed to\n+\t\t\t\t\t\t # add the trailing /.\n+\ttest_path_completion simple-dir/simple simple-dir/simple-file\n+'\n+\n+test_expect_success \\\n+ '__git_complete_index_file - escaped characters on cmdline' '\n+\ttest_path_completion spac \"spaces in dir\" && # Bash will turn this\n+\t\t\t\t\t\t # into \"spaces\\ in\\ dir\"\n+\ttest_path_completion \"spaces\\\\ i\" \\\n+\t\t\t \"spaces in dir\" &&\n+\ttest_path_completion \"spaces\\\\ in\\\\ dir/s\" \\\n+\t\t\t \"spaces in dir/spaces in file\" &&\n+\ttest_path_completion \"spaces\\\\ in\\\\ dir/spaces\\\\ i\" \\\n+\t\t\t \"spaces in dir/spaces in file\"\n+'\n+\n+test_expect_success \\\n+ '__git_complete_index_file - quoted characters on cmdline' '\n+\t# Testing with an opening but without a corresponding closing\n+\t# double quote is important.\n+\ttest_path_completion \\\"spac \"spaces in dir\" &&\n+\ttest_path_completion \"\\\"spaces i\" \\\n+\t\t\t \"spaces in dir\" &&\n+\ttest_path_completion \"\\\"spaces in dir/s\" \\\n+\t\t\t \"spaces in dir/spaces in file\" &&\n+\ttest_path_completion \"\\\"spaces in dir/spaces i\" \\\n+\t\t\t \"spaces in dir/spaces in file\"\n+'\n+\n+test_expect_success '__git_complete_index_file - UTF-8 in ls-files output' '\n+\ttest_path_completion á árvíztűrő &&\n+\ttest_path_completion árvíztűrő/С \"árvíztűrő/Сайн яваарай\"\n+'\n+\n+test_expect_success FUNNYNAMES \\\n+ '__git_complete_index_file - C-style escapes in ls-files output' '\n+\ttest_path_completion BS \\\n+\t\t\t BS\\\\dir &&\n+\ttest_path_completion BS\\\\\\\\d \\\n+\t\t\t BS\\\\dir &&\n+\ttest_path_completion BS\\\\\\\\dir/DQ \\\n+\t\t\t BS\\\\dir/DQ\\\"file &&\n+\ttest_path_completion BS\\\\\\\\dir/DQ\\\\\\\"f \\\n+\t\t\t BS\\\\dir/DQ\\\"file\n+'\n+\n+test_expect_success FUNNYNAMES \\\n+ '__git_complete_index_file - \\nnn-escaped characters in ls-files output' '\n+\ttest_path_completion sep '$'separators\\034in\\035dir'' &&\n+\ttest_path_completion '$'separators\\034i'' \\\n+\t\t\t '$'separators\\034in\\035dir'' &&\n+\ttest_path_completion '$'separators\\034in\\035dir/sep'' \\\n+\t\t\t '$'separators\\034in\\035dir/sep\\036in\\037file'' &&\n+\ttest_path_completion '$'separators\\034in\\035dir/sep\\036i'' \\\n+\t\t\t '$'separators\\034in\\035dir/sep\\036in\\037file''\n+'\n+\n+test_expect_success FUNNYNAMES \\\n+ '__git_complete_index_file - removing repeated quoted path components' '\n+\ttest_when_finished rm -r repeated-quoted &&\n+\tmkdir repeated-quoted && # A directory whose name in itself\n+\t\t\t\t # would not be quoted ...\n+\t>repeated-quoted/0-file &&\n+\t>repeated-quoted/1\\\"file && # ... but here the file makes the\n+\t\t\t\t # dirname quoted ...\n+\t>repeated-quoted/2-file &&\n+\t>repeated-quoted/3\\\"file && # ... and here, too.\n+\n+\t# Still, we shold only list the directory name only once.\n+\ttest_path_completion repeated repeated-quoted\n+'\n+\n+test_expect_success 'teardown after path completion tests' '\n+\trm -rf simple-dir \"spaces in dir\" árvíztűrő \\\n+\t BS\\\\dir '$'separators\\034in\\035dir''\n+'\n+\n+\n test_expect_success '__git_get_config_variables' '\n \tcat >expect <<-EOF &&\n \tname-1\n@@ -1469,113 +1587,6 @@ test_expect_success 'complete files' '\n \ttest_completion \"git add mom\" \"momified\"\n '\n \n-# The next tests only care about how the completion script deals with\n-# unusual characters in path names. By defining a custom completion\n-# function to list untracked files they won't be influenced by future\n-# changes of the completion functions of real git commands, and we\n-# don't have to bother with adding files to the index in these tests.\n-_git_test_path_comp ()\n-{\n-\t__git_complete_index_file --others\n-}\n-\n-test_expect_success 'complete files - escaped characters on cmdline' '\n-\ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n-\tmkdir \"New|Dir\" &&\n-\t>\"New|Dir/New&File.c\" &&\n-\n-\ttest_completion \"git test-path-comp N\" \\\n-\t\t\t\"New|Dir\" &&\t# Bash will turn this into \"New\\|Dir/\"\n-\ttest_completion \"git test-path-comp New\\\\|D\" \\\n-\t\t\t\"New|Dir\" &&\n-\ttest_completion \"git test-path-comp New\\\\|Dir/N\" \\\n-\t\t\t\"New|Dir/New&File.c\" &&\t# Bash will turn this into\n-\t\t\t\t\t\t# \"New\\|Dir/New\\&File.c \"\n-\ttest_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n-\t\t\t\"New|Dir/New&File.c\"\n-'\n-\n-test_expect_success 'complete files - quoted characters on cmdline' '\n-\ttest_when_finished \"rm -r \\\"New(Dir\\\"\" &&\n-\tmkdir \"New(Dir\" &&\n-\t>\"New(Dir/New)File.c\" &&\n-\n-\t# Testing with an opening but without a corresponding closing\n-\t# double quote is important.\n-\ttest_completion \"git test-path-comp \\\"New(D\" \"New(Dir\" &&\n-\ttest_completion \"git test-path-comp \\\"New(Dir/New)F\" \\\n-\t\t\t\"New(Dir/New)File.c\"\n-'\n-\n-test_expect_success 'complete files - UTF-8 in ls-files output' '\n-\ttest_when_finished \"rm -r árvíztűrő\" &&\n-\tmkdir árvíztűrő &&\n-\t>\"árvíztűrő/Сайн яваарай\" &&\n-\n-\ttest_completion \"git test-path-comp á\" \"árvíztűrő\" &&\n-\ttest_completion \"git test-path-comp árvíztűrő/С\" \\\n-\t\t\t\"árvíztűrő/Сайн яваарай\"\n-'\n-\n-# Testing with a path containing a backslash is important.\n-if test_have_prereq !MINGW &&\n- mkdir 'New\\Dir' 2>/dev/null &&\n- touch 'New\\Dir/New\"File.c' 2>/dev/null\n-then\n-\ttest_set_prereq FUNNYNAMES_BS_DQ\n-else\n-\tsay \"Your filesystem does not allow \\\\ and \\\" in filenames.\"\n-\trm -rf 'New\\Dir'\n-fi\n-test_expect_success FUNNYNAMES_BS_DQ \\\n- 'complete files - C-style escapes in ls-files output' '\n-\ttest_when_finished \"rm -r \\\"New\\\\\\\\Dir\\\"\" &&\n-\n-\ttest_completion \"git test-path-comp N\" \"New\\\\Dir\" &&\n-\ttest_completion \"git test-path-comp New\\\\\\\\D\" \"New\\\\Dir\" &&\n-\ttest_completion \"git test-path-comp New\\\\\\\\Dir/N\" \\\n-\t\t\t\"New\\\\Dir/New\\\"File.c\" &&\n-\ttest_completion \"git test-path-comp New\\\\\\\\Dir/New\\\\\\\"F\" \\\n-\t\t\t\"New\\\\Dir/New\\\"File.c\"\n-'\n-\n-if test_have_prereq !MINGW &&\n- mkdir $'New\\034Special\\035Dir' 2>/dev/null &&\n- touch $'New\\034Special\\035Dir/New\\036Special\\037File' 2>/dev/null\n-then\n-\ttest_set_prereq FUNNYNAMES_SEPARATORS\n-else\n-\tsay 'Your filesystem does not allow special separator characters (FS, GS, RS, US) in filenames.'\n-\trm -rf $'New\\034Special\\035Dir'\n-fi\n-test_expect_success FUNNYNAMES_SEPARATORS \\\n- 'complete files - \\nnn-escaped control characters in ls-files output' '\n-\ttest_when_finished \"rm -r '$'New\\034Special\\035Dir''\" &&\n-\n-\t# Note: these will be literal separator characters on the cmdline.\n-\ttest_completion \"git test-path-comp N\" \"'$'New\\034Special\\035Dir''\" &&\n-\ttest_completion \"git test-path-comp '$'New\\034S''\" \\\n-\t\t\t\"'$'New\\034Special\\035Dir''\" &&\n-\ttest_completion \"git test-path-comp '$'New\\034Special\\035Dir/''\" \\\n-\t\t\t\"'$'New\\034Special\\035Dir/New\\036Special\\037File''\" &&\n-\ttest_completion \"git test-path-comp '$'New\\034Special\\035Dir/New\\036S''\" \\\n-\t\t\t\"'$'New\\034Special\\035Dir/New\\036Special\\037File''\"\n-'\n-\n-test_expect_success FUNNYNAMES_BS_DQ \\\n- 'complete files - removing repeated quoted path components' '\n-\ttest_when_finished rm -rf NewDir &&\n-\tmkdir NewDir && # A dirname which in itself would not be quoted ...\n-\t>NewDir/0-file &&\n-\t>NewDir/1\\\"file && # ... but here the file makes the dirname quoted ...\n-\t>NewDir/2-file &&\n-\t>NewDir/3\\\"file && # ... and here, too.\n-\n-\t# Still, we should only list it once.\n-\ttest_completion \"git test-path-comp New\" \"NewDir\"\n-'\n-\n-\n test_expect_success \"completion uses completion for alias: !sh -c 'git ...'\" '\n \ttest_config alias.co \"!sh -c '\"'\"'git checkout ...'\"'\"'\" &&\n \ttest_completion \"git co m\" <<-\\EOF\n-- \n2.17.0.799.gd371044c7c\n\n"},{"id":"347997","messageId":"CAPig+cR3phOhx79n6Z2bA2O=PrUPPnbcgHgHOYYYmyOiwjVdEA@mail.gmail.com","threadId":"48065","inReplyTo":"20180518141751.16350-3-szeder.dev@gmail.com","subject":"Re: [PATCH 2/2] t9902-completion: exercise __git_complete_index_file() directly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-05-18T19:25:48Z","receivedAt":"2018-05-18T19:25:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 18, 2018 at 10:17 AM, SZEDER Gábor wrote:\n> The tests added in 2f271cd9cf (t9902-completion: add tests\n> demonstrating issues with quoted pathnames, 2018-05-08) and in\n> 2ab6eab4fe (completion: improve handling quoted paths in 'git\n> ls-files's output, 2018-03-28) have a few shortcomings:\n>\n> - All these test use the 'test_completion' helper function, thus\n\ns/these test/&s/\n\n> they are exercising the whole completion machinery, although they\n> are only interested in how git-aware path completion, specifically\n> the __git_complete_index_file() function deals with unusual\n> characters in pathnames and on the command line.\n>\n> - These tests can't satisfactorily test the case of pathnames\n> containing spaces, because 'test_completion' gets the words on the\n> command line as a single argument and it uses space as word\n> separator.\n>\n> - Some of the tests are protected by different FUNNYNAMES_* prereqs\n> depending on whether they put backslashes and double quotes or\n> separator characters (FS, GS, RS, US) in pathnames, although a\n> filesystem not allowing one likely doesn't allow the others\n> either.\n>\n> - One of the tests operates on paths containing '|' and '&'\n> characters without being protected by a FUNNYNAMES prereq, but\n> some filesystems (notably on Windows) don't allow these characters\n> in pathnames, either.\n>\n> Replace these tests with basically equivalent, more focused tests that\n> call __git_complete_index_file() directly. Since this function only\n> looks at the current word to be completed, i.e. the $cur variable, we\n> can easily include pathnames containing spaces in the tests, so use\n> such pathnames instead of pathnames containing '|' and '&'. Finally,\n> use only a single FUNNYNAMES prereq for all kinds of special\n> characters.\n>\n> Signed-off-by: SZEDER Gábor \n"},{"id":"348192","messageId":"nycvar.QRO.7.76.6.1805211334220.77@tvgsbejvaqbjf.bet","threadId":"48065","inReplyTo":"20180518141751.16350-1-szeder.dev@gmail.com","subject":"Re: [PATCH 0/2] Test improvements for 'sg/complete-paths'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-21T11:35:26Z","receivedAt":"2018-05-21T11:35:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Fri, 18 May 2018, SZEDER Gábor wrote:\n\n> > > So, I think for v2 I will rewrite these tests to call\n> > > __git_complete_index_file() directly instead of using\n> > > 'test_completion', and will include a test with spaces in path\n> > > names.\n> > \n> > Quite well thought-out reasoning. Thanks.\n> \n> Unfortunately I couldn't get around to it soon enough, and now the topic\n> 'sg/complete-paths' is already in next, so here are those test\n> improvements on top.\n\nI can verify that the weeks-long breakage of `pu` on Windows has been\naddressed, probably by this patch series.\n\nCiao,\nDscho"},{"id":"348194","messageId":"nycvar.QRO.7.76.6.1805211340490.77@tvgsbejvaqbjf.bet","threadId":"48065","inReplyTo":"20180518141751.16350-3-szeder.dev@gmail.com","subject":"Re: [PATCH 2/2] t9902-completion: exercise __git_complete_index_file() directly","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-21T12:14:11Z","receivedAt":"2018-05-21T12:14:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Fri, 18 May 2018, SZEDER Gábor wrote:\n\n> The tests added in 2f271cd9cf (t9902-completion: add tests\n> demonstrating issues with quoted pathnames, 2018-05-08) and in\n> 2ab6eab4fe (completion: improve handling quoted paths in 'git\n> ls-files's output, 2018-03-28) have a few shortcomings:\n> \n> - All these test use the 'test_completion' helper function, thus\n> they are exercising the whole completion machinery, although they\n> are only interested in how git-aware path completion, specifically\n> the __git_complete_index_file() function deals with unusual\n> characters in pathnames and on the command line.\n> \n> - These tests can't satisfactorily test the case of pathnames\n> containing spaces, because 'test_completion' gets the words on the\n> command line as a single argument and it uses space as word\n> separator.\n> \n> - Some of the tests are protected by different FUNNYNAMES_* prereqs\n> depending on whether they put backslashes and double quotes or\n> separator characters (FS, GS, RS, US) in pathnames, although a\n> filesystem not allowing one likely doesn't allow the others\n> either.\n> \n> - One of the tests operates on paths containing '|' and '&'\n> characters without being protected by a FUNNYNAMES prereq, but\n> some filesystems (notably on Windows) don't allow these characters\n> in pathnames, either.\n> \n> Replace these tests with basically equivalent, more focused tests that\n> call __git_complete_index_file() directly. Since this function only\n> looks at the current word to be completed, i.e. the $cur variable, we\n> can easily include pathnames containing spaces in the tests, so use\n> such pathnames instead of pathnames containing '|' and '&'. Finally,\n> use only a single FUNNYNAMES prereq for all kinds of special\n> characters.\n\nMakes sense.\n\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 955932174c..1b6d275254 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -1209,6 +1209,124 @@ test_expect_success 'teardown after ref completion' '\n> \tgit remote remove other\n> '\n> \n> +\n> +test_path_completion ()\n> +{\n> +\ttest $# = 2 || error \"bug in the test script: not 2 parameters to test_path_completion\"\n\nMaybe shorten this to\n\n\ttest $# = 2 || error \"BUG: test_path_completion requires 2 parameters\"\n\nin order to keep to the 80 columns/line limit?\n\n> +\n> +\tlocal cur=\"$1\" expected=\"$2\"\n\nI thought `local` was a Bash-ism we tried to avoid in the test scripts.\nBut I guess this file is already littered with `local` keywords...\n\n> +\techo \"$expected\" >expected &&\n> +\t(\n> +\t\t# In the following tests calling this function we only\n> +\t\t# care about how __git_complete_index_file() deals with\n> +\t\t# unusual characters in path names. By requesting only\n> +\t\t# untracked files we dont have to bother adding any\n> +\t\t# paths to the index in those tests.\n> +\t\t__git_complete_index_file --others &&\n> +\t\tprint_comp\n> +\t) &&\n> +\ttest_cmp expected out\n> +}\n> +\n> +test_expect_success 'setup for path completion tests' '\n> +\tmkdir simple-dir \\\n> +\t \"spaces in dir\" \\\n> +\t árvíztűrő &&\n> +\ttouch simple-dir/simple-file \\\n> +\t \"spaces in dir/spaces in file\" \\\n> +\t \"árvíztűrő/Сайн яваарай\" &&\n> +\tif test_have_prereq !MINGW &&\n> +\t mkdir BS\\\\dir \\\n> +\t\t '$'separators\\034in\\035dir'' &&\n> +\t touch BS\\\\dir/DQ\\\"file \\\n> +\t\t '$'separators\\034in\\035dir/sep\\036in\\037file''\n> +\tthen\n> +\t\ttest_set_prereq FUNNYNAMES\n> +\telse\n> +\t\trm -rf BS\\\\dir '$'separators\\034in\\035dir''\n> +\tfi\n> +'\n> +\n> +test_expect_success '__git_complete_index_file - simple' '\n> +\ttest_path_completion simple simple-dir && # Bash is supposed to\n> +\t\t\t\t\t\t # add the trailing /.\n> +\ttest_path_completion simple-dir/simple simple-dir/simple-file\n> +'\n> +\n> +test_expect_success \\\n> + '__git_complete_index_file - escaped characters on cmdline' '\n> +\ttest_path_completion spac \"spaces in dir\" && # Bash will turn this\n> +\t\t\t\t\t\t # into \"spaces\\ in\\ dir\"\n> +\ttest_path_completion \"spaces\\\\ i\" \\\n> +\t\t\t \"spaces in dir\" &&\n> +\ttest_path_completion \"spaces\\\\ in\\\\ dir/s\" \\\n> +\t\t\t \"spaces in dir/spaces in file\" &&\n> +\ttest_path_completion \"spaces\\\\ in\\\\ dir/spaces\\\\ i\" \\\n> +\t\t\t \"spaces in dir/spaces in file\"\n> +'\n> +\n> +test_expect_success \\\n> + '__git_complete_index_file - quoted characters on cmdline' '\n> +\t# Testing with an opening but without a corresponding closing\n> +\t# double quote is important.\n> +\ttest_path_completion \\\"spac \"spaces in dir\" &&\n> +\ttest_path_completion \"\\\"spaces i\" \\\n> +\t\t\t \"spaces in dir\" &&\n> +\ttest_path_completion \"\\\"spaces in dir/s\" \\\n> +\t\t\t \"spaces in dir/spaces in file\" &&\n> +\ttest_path_completion \"\\\"spaces in dir/spaces i\" \\\n> +\t\t\t \"spaces in dir/spaces in file\"\n> +'\n> +\n> +test_expect_success '__git_complete_index_file - UTF-8 in ls-files output' '\n> +\ttest_path_completion á árvíztűrő &&\n> +\ttest_path_completion árvíztűrő/С \"árvíztűrő/Сайн яваарай\"\n> +'\n> +\n> +test_expect_success FUNNYNAMES \\\n> + '__git_complete_index_file - C-style escapes in ls-files output' '\n> +\ttest_path_completion BS \\\n> +\t\t\t BS\\\\dir &&\n> +\ttest_path_completion BS\\\\\\\\d \\\n> +\t\t\t BS\\\\dir &&\n> +\ttest_path_completion BS\\\\\\\\dir/DQ \\\n> +\t\t\t BS\\\\dir/DQ\\\"file &&\n> +\ttest_path_completion BS\\\\\\\\dir/DQ\\\\\\\"f \\\n> +\t\t\t BS\\\\dir/DQ\\\"file\n> +'\n> +\n> +test_expect_success FUNNYNAMES \\\n> + '__git_complete_index_file - \\nnn-escaped characters in ls-files output' '\n> +\ttest_path_completion sep '$'separators\\034in\\035dir'' &&\n> +\ttest_path_completion '$'separators\\034i'' \\\n> +\t\t\t '$'separators\\034in\\035dir'' &&\n> +\ttest_path_completion '$'separators\\034in\\035dir/sep'' \\\n> +\t\t\t '$'separators\\034in\\035dir/sep\\036in\\037file'' &&\n> +\ttest_path_completion '$'separators\\034in\\035dir/sep\\036i'' \\\n> +\t\t\t '$'separators\\034in\\035dir/sep\\036in\\037file''\n> +'\n> +\n> +test_expect_success FUNNYNAMES \\\n> + '__git_complete_index_file - removing repeated quoted path components' '\n> +\ttest_when_finished rm -r repeated-quoted &&\n> +\tmkdir repeated-quoted && # A directory whose name in itself\n> +\t\t\t\t # would not be quoted ...\n> +\t>repeated-quoted/0-file &&\n> +\t>repeated-quoted/1\\\"file && # ... but here the file makes the\n> +\t\t\t\t # dirname quoted ...\n> +\t>repeated-quoted/2-file &&\n> +\t>repeated-quoted/3\\\"file && # ... and here, too.\n> +\n> +\t# Still, we shold only list the directory name only once.\n> +\ttest_path_completion repeated repeated-quoted\n> +'\n> +\n> +test_expect_success 'teardown after path completion tests' '\n> +\trm -rf simple-dir \"spaces in dir\" árvíztűrő \\\n> +\t BS\\\\dir '$'separators\\034in\\035dir''\n> +'\n> +\n> +\n> test_expect_success '__git_get_config_variables' '\n> \tcat >expect <<-EOF &&\n> \tname-1\n> @@ -1469,113 +1587,6 @@ test_expect_success 'complete files' '\n\nIt made it a bit awkward to review this patch that the code was\nmove-edited. In this case, I cannot blame exclusively the hostile review\nenvironment that is an email reader, but also sadly, `git show --color-moved\n7d314073488ae81b8f5cdecb4d00a78529fa7dc3` helped only *so* much\n*almost-touches-thumb-with-first-finger*.\n\n> \ttest_completion \"git add mom\" \"momified\"\n> '\n> \n> -# The next tests only care about how the completion script deals with\n> -# unusual characters in path names. By defining a custom completion\n> -# function to list untracked files they won't be influenced by future\n> -# changes of the completion functions of real git commands, and we\n> -# don't have to bother with adding files to the index in these tests.\n\nWe should keep the corresponding new comment also outside the function, as\nit talks about the following tests inside a twice-indented code comment\ninside a subshell.\n\n> -_git_test_path_comp ()\n> -{\n> -\t__git_complete_index_file --others\n> -}\n\nA new test case was inserted here, in the move-edited section:\n'__git_complete_index_file - simple'.\n\n> -\n> -test_expect_success 'complete files - escaped characters on cmdline' '\n> -\ttest_when_finished \"rm -rf \\\"New|Dir\\\"\" &&\n> -\tmkdir \"New|Dir\" &&\n> -\t>\"New|Dir/New&File.c\" &&\n> -\n> -\ttest_completion \"git test-path-comp N\" \\\n> -\t\t\t\"New|Dir\" &&\t# Bash will turn this into \"New\\|Dir/\"\n> -\ttest_completion \"git test-path-comp New\\\\|D\" \\\n> -\t\t\t\"New|Dir\" &&\n> -\ttest_completion \"git test-path-comp New\\\\|Dir/N\" \\\n> -\t\t\t\"New|Dir/New&File.c\" &&\t# Bash will turn this into\n> -\t\t\t\t\t\t# \"New\\|Dir/New\\&File.c \"\n> -\ttest_completion \"git test-path-comp New\\\\|Dir/New\\\\&F\" \\\n> -\t\t\t\"New|Dir/New&File.c\"\n> -'\n\nThis corresponds to the new '__git_complete_index_file - escaped\ncharacters on cmdline' test case, which looks different by necessity: it\navoids funny file names by using spaces in file names instead.\n\nFrom what I can see, the new version is indeed equivalent to the old\nversion.\n\n> -\n> -test_expect_success 'complete files - quoted characters on cmdline' '\n> -\ttest_when_finished \"rm -r \\\"New(Dir\\\"\" &&\n> -\tmkdir \"New(Dir\" &&\n> -\t>\"New(Dir/New)File.c\" &&\n> -\n> -\t# Testing with an opening but without a corresponding closing\n> -\t# double quote is important.\n> -\ttest_completion \"git test-path-comp \\\"New(D\" \"New(Dir\" &&\n> -\ttest_completion \"git test-path-comp \\\"New(Dir/New)F\" \\\n> -\t\t\t\"New(Dir/New)File.c\"\n> -'\n\nThe move-edited code is essentially testing the same, and then two more\nconditions: in addition to testing with a prefix without a space, it also\ntests with prefixes with a space (when trying to complete \"spaces in dir\",\nit should not matter whether we start writing \"spac or \"spaces\ni.\n\nGood.\n\n> -\n> -test_expect_success 'complete files - UTF-8 in ls-files output' '\n> -\ttest_when_finished \"rm -r árvíztűrő\" &&\n> -\tmkdir árvíztűrő &&\n> -\t>\"árvíztűrő/Сайн яваарай\" &&\n> -\n> -\ttest_completion \"git test-path-comp á\" \"árvíztűrő\" &&\n> -\ttest_completion \"git test-path-comp árvíztűrő/С\" \\\n> -\t\t\t\"árvíztűrő/Сайн яваарай\"\n> -'\n\nThis one is identical to the move-edited code (modulo the\nno-longer-necessary directory/file).\n\n> -\n> -# Testing with a path containing a backslash is important.\n> -if test_have_prereq !MINGW &&\n> - mkdir 'New\\Dir' 2>/dev/null &&\n> - touch 'New\\Dir/New\"File.c' 2>/dev/null\n> -then\n> -\ttest_set_prereq FUNNYNAMES_BS_DQ\n> -else\n> -\tsay \"Your filesystem does not allow \\\\ and \\\" in filenames.\"\n> -\trm -rf 'New\\Dir'\n> -fi\n> -test_expect_success FUNNYNAMES_BS_DQ \\\n> - 'complete files - C-style escapes in ls-files output' '\n> -\ttest_when_finished \"rm -r \\\"New\\\\\\\\Dir\\\"\" &&\n> -\n> -\ttest_completion \"git test-path-comp N\" \"New\\\\Dir\" &&\n> -\ttest_completion \"git test-path-comp New\\\\\\\\D\" \"New\\\\Dir\" &&\n> -\ttest_completion \"git test-path-comp New\\\\\\\\Dir/N\" \\\n> -\t\t\t\"New\\\\Dir/New\\\"File.c\" &&\n> -\ttest_completion \"git test-path-comp New\\\\\\\\Dir/New\\\\\\\"F\" \\\n> -\t\t\t\"New\\\\Dir/New\\\"File.c\"\n> -'\n\nThe move-edited code uses BS\\dir instead of New\\Dir.\n\n> -\n> -if test_have_prereq !MINGW &&\n> - mkdir $'New\\034Special\\035Dir' 2>/dev/null &&\n> - touch $'New\\034Special\\035Dir/New\\036Special\\037File' 2>/dev/null\n> -then\n> -\ttest_set_prereq FUNNYNAMES_SEPARATORS\n> -else\n> -\tsay 'Your filesystem does not allow special separator characters (FS, GS, RS, US) in filenames.'\n> -\trm -rf $'New\\034Special\\035Dir'\n> -fi\n> -test_expect_success FUNNYNAMES_SEPARATORS \\\n> - 'complete files - \\nnn-escaped control characters in ls-files output' '\n> -\ttest_when_finished \"rm -r '$'New\\034Special\\035Dir''\" &&\n> -\n> -\t# Note: these will be literal separator characters on the cmdline.\n> -\ttest_completion \"git test-path-comp N\" \"'$'New\\034Special\\035Dir''\" &&\n> -\ttest_completion \"git test-path-comp '$'New\\034S''\" \\\n> -\t\t\t\"'$'New\\034Special\\035Dir''\" &&\n> -\ttest_completion \"git test-path-comp '$'New\\034Special\\035Dir/''\" \\\n> -\t\t\t\"'$'New\\034Special\\035Dir/New\\036Special\\037File''\" &&\n> -\ttest_completion \"git test-path-comp '$'New\\034Special\\035Dir/New\\036S''\" \\\n> -\t\t\t\"'$'New\\034Special\\035Dir/New\\036Special\\037File''\"\n> -'\n\nThe move-edited code uses the file name\n`$separatorsindir/sepinfile` instead of\n`$NewSpecialDir/NewSpecialFile`, but is otherwise\nidentical.\n\nToo many differences for the --color-moved code to catch, though.\n\n> -\n> -test_expect_success FUNNYNAMES_BS_DQ \\\n> - 'complete files - removing repeated quoted path components' '\n> -\ttest_when_finished rm -rf NewDir &&\n> -\tmkdir NewDir && # A dirname which in itself would not be quoted ...\n> -\t>NewDir/0-file &&\n> -\t>NewDir/1\\\"file && # ... but here the file makes the dirname quoted ...\n> -\t>NewDir/2-file &&\n> -\t>NewDir/3\\\"file && # ... and here, too.\n> -\n> -\t# Still, we should only list it once.\n> -\ttest_completion \"git test-path-comp New\" \"NewDir\"\n> -'\n\nThe move-edited code uses `repeated` instead of `New` and\n`repeated-quoted` instead of `NewDir`. I could not spot any other\ndifferences.\n\nOf course, `--color-moved` had no chance here, either.\n\n> -\n> -\n> test_expect_success \"completion uses completion for alias: !sh -c 'git ...'\" '\n> \ttest_config alias.co \"!sh -c '\"'\"'git checkout ...'\"'\"'\" &&\n> \ttest_completion \"git co m\" <<-\\EOF\n\nThe move-edited code needed to insert a cleanup step at the end, because\nthe new directories and files need to live for more than one test case\n(therefore `test_when_finished` is out of the game).\n\nNote to self: should we ever switch to a modern test framework that allows\nparallelizing tests, then these test cases need to be marked up with\n@Before and @After to create/delete those directories/files.\n\nEven if it was hard to review, I think this patch is essentially good, at\nleast after fixing the typo pointed out by Eric and then shortening the\nlong line.\n\nThank you for keeping on the ball!\nDscho"},{"id":"348195","messageId":"nycvar.QRO.7.76.6.1805211414200.77@tvgsbejvaqbjf.bet","threadId":"48065","inReplyTo":"nycvar.QRO.7.76.6.1805211334220.77@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/2] Test improvements for 'sg/complete-paths'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-21T12:17:49Z","receivedAt":"2018-05-21T12:17:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 21 May 2018, Johannes Schindelin wrote:\n\n> On Fri, 18 May 2018, SZEDER Gábor wrote:\n> \n> > > > So, I think for v2 I will rewrite these tests to call\n> > > > __git_complete_index_file() directly instead of using\n> > > > 'test_completion', and will include a test with spaces in path\n> > > > names.\n> > > \n> > > Quite well thought-out reasoning. Thanks.\n> > \n> > Unfortunately I couldn't get around to it soon enough, and now the topic\n> > 'sg/complete-paths' is already in next, so here are those test\n> > improvements on top.\n> \n> I can verify that the weeks-long breakage of `pu` on Windows has been\n> addressed, probably by this patch series.\n\nPlease note that as the branch that is fixed by these two patches was\nalready merged down to `next`, the Continuous Testing on Windows reports\nthat `next` is broken.\n\n(I saw that breakage for over a week, but was too busy elsewhere to act on\nit.)\n\nIt would be really lovely to see it fixed soon, so that other bugs cannot\nbe hidden by that breakage.\n\nAnd I would also love to see sg/complete-paths to be merged down to\n`master` *only* in conjunction with these two patches, not on its own\n(because that would break the Continuous Testing on Windows of `master`,\nwhich is something I really want us to avoid).\n\nThanks,\nDscho"}]}