{"thread":{"id":"33654","subject":"[PATCH v2 0/9] completion: general cleanups","startedAt":"2013-04-27T20:09:58Z","lastAt":"2013-04-28T01:45:39Z","messageCount":15,"participants":["Felipe Contreras","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":2,"patchTotal":9},"messages":[{"id":"215739","messageId":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":null,"subject":"[PATCH v2 0/9] completion: general cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:09:58Z","receivedAt":"2013-04-27T20:09:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nIn the previous series I didn't notice -o filenames actually did something\nuseful, so in this series it's still used, however, for versions older than 4,\nwe use a hack to enable it. This allows us to get rid of a lot of compat code.\n\nI also got rid of some wrappers, and fixed a regression: spaces were not added\nat the end of the completion.\n\nThe result is that now we have the same completion for bash < 4, bash >=4, and\nzsh. Although we already had that for the last two, now the code is simpler.\n\nThe only difference is that bash < 4 doesn't add a trailing space, like before\nfea16b4 (git-completion.bash: add support for path completion).\n\nAlso, add the missing tests.\n\nCheers.\n\nFelipe Contreras (9):\n  completion: add file completion tests\n  completion: document tilde expansion failure in tests\n  completion; remove unuseful comments\n  completion: use __gitcompadd for __gitcomp_file\n  completion: refactor diff_index wrappers\n  completion: refactor __git_complete_index_file()\n  completion: add hack to enable file mode in bash < 4\n  completion: add space after completed filename\n  completion: remove __git_index_file_list_filter()\n\n contrib/completion/git-completion.bash | 145 +++++++--------------------------\n t/t9902-completion.sh                  |  77 +++++++++++++++++\n 2 files changed, 107 insertions(+), 115 deletions(-)\n\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215741","messageId":"1367093407-8898-2-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 1/9] completion: add file completion tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:09:59Z","receivedAt":"2013-04-27T20:09:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The commit fea16b4 (git-completion.bash: add support for path\ncompletion) introduced quite a few changes, without the usual tests.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 68 insertions(+)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 6d9d141..385e1e4 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -347,4 +347,72 @@ test_expect_success 'send-email' '\n \ttest_completion \"git send-email ma\" \"master \"\n '\n \n+test_expect_success 'complete files' '\n+\tgit init tmp && cd tmp &&\n+\ttest_when_finished \"cd .. && rm -rf tmp\" &&\n+\n+\techo \"expected\" > .gitignore &&\n+\techo \"out\" >> .gitignore &&\n+\n+\tgit add .gitignore &&\n+\ttest_completion \"git commit \" \".gitignore\" &&\n+\n+\tgit commit -m ignore &&\n+\n+\ttouch new &&\n+\ttest_completion \"git add \" \"new\" &&\n+\n+\tgit add new &&\n+\tgit commit -a -m new &&\n+\ttest_completion \"git add \" \"\" &&\n+\n+\tgit mv new modified &&\n+\techo modify > modified &&\n+\ttest_completion \"git add \" \"modified\" &&\n+\n+\ttouch untracked &&\n+\n+\t: TODO .gitignore should not be here &&\n+\ttest_completion \"git rm \" <<-\\EOF &&\n+\t.gitignore\n+\tmodified\n+\tEOF\n+\n+\ttest_completion \"git clean \" \"untracked\" &&\n+\n+\t: TODO .gitignore should not be here &&\n+\ttest_completion \"git mv \" <<-\\EOF &&\n+\t.gitignore\n+\tmodified\n+\tEOF\n+\n+\tmkdir dir &&\n+\ttouch dir/file-in-dir &&\n+\tgit add dir/file-in-dir &&\n+\tgit commit -m dir &&\n+\n+\tmkdir untracked-dir &&\n+\n+\t: TODO .gitignore should not be here &&\n+\ttest_completion \"git mv modified \" <<-\\EOF &&\n+\t.gitignore\n+\tdir\n+\tmodified\n+\tuntracked\n+\tuntracked-dir\n+\tEOF\n+\n+\ttest_completion \"git commit \" \"modified\" &&\n+\n+\t: TODO .gitignore should not be here &&\n+\ttest_completion \"git ls-files \" <<-\\EOF\n+\t.gitignore\n+\tdir\n+\tmodified\n+\tEOF\n+\n+\ttouch momified &&\n+\ttest_completion \"git add mom\" \"momified\"\n+'\n+\n test_done\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215740","messageId":"1367093407-8898-3-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 2/9] completion: document tilde expansion failure in tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:00Z","receivedAt":"2013-04-27T20:10:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 385e1e4..81a1657 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -415,4 +415,13 @@ test_expect_success 'complete files' '\n \ttest_completion \"git add mom\" \"momified\"\n '\n \n+test_expect_failure 'complete with tilde expansion' '\n+\tgit init tmp && cd tmp &&\n+\ttest_when_finished \"cd .. && rm -rf tmp\" &&\n+\n+\ttouch ~/tmp/file &&\n+\n+\ttest_completion \"git add ~/tmp/\" \"~/tmp/file\"\n+'\n+\n test_done\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215746","messageId":"1367093407-8898-4-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 3/9] completion; remove unuseful comments","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:01Z","receivedAt":"2013-04-27T20:10:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The only caller, __git_complete_index_file() doesn't specify any limits\nto the options for 'git ls-files', neither should this function.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex bc3fc9e..f7b0f3c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -323,8 +323,6 @@ __git_diff_index_helper ()\n \n # __git_index_files accepts 1 or 2 arguments:\n # 1: Options to pass to ls-files (required).\n-#    Supported options are --cached, --modified, --deleted, --others,\n-#    and --directory.\n # 2: A directory path (optional).\n #    If provided, only files within the specified directory are listed.\n #    Sub directories are never recursed.  Path must have a trailing\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215744","messageId":"1367093407-8898-5-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 4/9] completion: use __gitcompadd for __gitcomp_file","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:02Z","receivedAt":"2013-04-27T20:10:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Like the rest of the script does; let's not access COMPREPLY directly.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\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 f7b0f3c..7f1ebe4 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -252,7 +252,7 @@ __gitcomp_file ()\n \t# since tilde expansion is not applied.\n \t# This means that COMPREPLY will be empty and Bash default\n \t# completion will be used.\n-\tCOMPREPLY=($(compgen -P \"${2-}\" -W \"$1\" -- \"${3-$cur}\"))\n+\t__gitcompadd \"$1\" \"${2-}\" \"${3-$cur}\" \"\"\n \n \t# Tell Bash that compspec generates filenames.\n \tcompopt -o filenames 2>/dev/null\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215742","messageId":"1367093407-8898-6-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 5/9] completion: refactor diff_index wrappers","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:03Z","receivedAt":"2013-04-27T20:10:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"At the end of the day what we really need is to find out the files that\nhave been staged, or modified, because those files are the ones that\nmake sense to pass as arguments to 'git commit'.\n\nWe need diff-index to find those out, since 'git ls-files' doesn't do\nthat.\n\nBut we don't need wrappers and wrappers basically identical to the ones\nused for 'git ls-files', when we can pretend it receives a --committable\noption that would return what we need.\n\nThat way, we can remove a bunch of code without any functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 71 ++++++++--------------------------\n 1 file changed, 16 insertions(+), 55 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7f1ebe4..2561265 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -297,30 +297,25 @@ __git_index_file_list_filter ()\n \t__git_index_file_list_filter_bash\n }\n \n-# Execute git ls-files, returning paths relative to the directory\n-# specified in the first argument, and using the options specified in\n-# the second argument.\n+# Execute 'git ls-files', unless the --committable option is specified, in\n+# which case it runs 'git diff-index' to find out the files that can be\n+# committed.  It return paths relative to the directory specified in the first\n+# argument, and using the options specified in the second argument.\n __git_ls_files_helper ()\n {\n \t(\n \t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n-\t\t# NOTE: $2 is not quoted in order to support multiple options\n-\t\tcd \"$1\" && git ls-files --exclude-standard $2\n+\t\tcd \"$1\"\n+\t\tif [ \"$2\" == \"--committable\" ]; then\n+\t\t\tgit diff-index --name-only --relative HEAD\n+\t\telse\n+\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n+\t\t\tgit ls-files --exclude-standard $2\n+\t\tfi\n \t) 2>/dev/null\n }\n \n \n-# Execute git diff-index, returning paths relative to the directory\n-# specified in the first argument, and using the tree object id\n-# specified in the second argument.\n-__git_diff_index_helper ()\n-{\n-\t(\n-\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n-\t\tcd \"$1\" && git diff-index --name-only --relative \"$2\"\n-\t) 2>/dev/null\n-}\n-\n # __git_index_files accepts 1 or 2 arguments:\n # 1: Options to pass to ls-files (required).\n # 2: A directory path (optional).\n@@ -337,22 +332,6 @@ __git_index_files ()\n \tfi\n }\n \n-# __git_diff_index_files accepts 1 or 2 arguments:\n-# 1) The id of a tree object.\n-# 2) A directory path (optional).\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-__git_diff_index_files ()\n-{\n-\tlocal dir=\"$(__gitdir)\" root=\"${2-.}\"\n-\n-\tif [ -d \"$dir\" ]; then\n-\t\t__git_diff_index_helper \"$root\" \"$1\" | __git_index_file_list_filter |\n-\t\t\tsort | uniq\n-\tfi\n-}\n-\n __git_heads ()\n {\n \tlocal dir=\"$(__gitdir)\"\n@@ -550,8 +529,10 @@ __git_complete_revlist_file ()\n }\n \n \n-# __git_complete_index_file requires 1 argument: the options to pass to\n-# ls-file\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@@ -570,26 +551,6 @@ __git_complete_index_file ()\n \tesac\n }\n \n-# __git_complete_diff_index_file requires 1 argument: the id of a tree\n-# object\n-__git_complete_diff_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-\n-\t\t__gitcomp_file \"$(__git_diff_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\n-\t\t;;\n-\t*)\n-\t\t__gitcomp_file \"$(__git_diff_index_files \"$1\")\" \"\" \"$cur_\"\n-\t\t;;\n-\tesac\n-}\n-\n __git_complete_file ()\n {\n \t__git_complete_revlist_file\n@@ -1211,7 +1172,7 @@ _git_commit ()\n \tesac\n \n \tif git rev-parse --verify --quiet HEAD >/dev/null; then\n-\t\t__git_complete_diff_index_file \"HEAD\"\n+\t\t__git_complete_index_file \"--committable\"\n \telse\n \t\t# This is the first commit\n \t\t__git_complete_index_file \"--cached\"\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215745","messageId":"1367093407-8898-7-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 6/9] completion: refactor __git_complete_index_file()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:04Z","receivedAt":"2013-04-27T20:10:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The calls to __gitcomp_file() are essentially the same, but with\ndifferent prefix.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 9 +++------\n 1 file changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2561265..9cea170 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -535,20 +535,17 @@ __git_complete_revlist_file ()\n # The exception is --committable, which finds the files appropriate commit.\n __git_complete_index_file ()\n {\n-\tlocal pfx cur_=\"$cur\"\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-\n-\t\t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\n-\t\t;;\n-\t*)\n-\t\t__gitcomp_file \"$(__git_index_files \"$1\")\" \"\" \"$cur_\"\n \t\t;;\n \tesac\n+\n+\t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\n }\n \n __git_complete_file ()\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215743","messageId":"1367093407-8898-8-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 7/9] completion: add hack to enable file mode in bash < 4","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:05Z","receivedAt":"2013-04-27T20:10:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This way we don't need all the compat stuff, different filters, and so\non. Also, now we complete exactly the same in bash 3 and bash 4.\n\nThis is the way bash-completion did it for quite some time, when bash 3\nwas supported. For more information about the hack:\n\nhttp://bugs.debian.org/cgi-bin/bugreport.cgi?bug=272660#64\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 43 ++++++----------------------------\n 1 file changed, 7 insertions(+), 36 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 9cea170..f9e8e7d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -254,33 +254,21 @@ __gitcomp_file ()\n \t# completion will be used.\n \t__gitcompadd \"$1\" \"${2-}\" \"${3-$cur}\" \"\"\n \n-\t# Tell Bash that compspec generates filenames.\n-\tcompopt -o filenames 2>/dev/null\n+\t# use a hack to enable file mode in bash < 4\n+\tcompopt -o filenames 2>/dev/null ||\n+\tcompgen -f /non-existing-dir/ > /dev/null\n }\n \n-__git_index_file_list_filter_compat ()\n-{\n-\tlocal path\n-\n-\twhile read -r path; do\n-\t\tcase \"$path\" in\n-\t\t?*/*) echo \"${path%%/*}/\" ;;\n-\t\t*) echo \"$path\" ;;\n-\t\tesac\n-\tdone\n-}\n-\n-__git_index_file_list_filter_bash ()\n+# Process path list returned by \"ls-files\" and \"diff-index --name-only\"\n+# commands, in order to list only file names relative to a specified\n+# directory, and append a slash to directory names.\n+__git_index_file_list_filter ()\n {\n \tlocal path\n \n \twhile read -r path; do\n \t\tcase \"$path\" in\n \t\t?*/*)\n-\t\t\t# XXX if we append a slash to directory names when using\n-\t\t\t# `compopt -o filenames`, Bash will append another slash.\n-\t\t\t# This is pretty stupid, and this the reason why we have to\n-\t\t\t# define a compatible version for this function.\n \t\t\techo \"${path%%/*}\" ;;\n \t\t*)\n \t\t\techo \"$path\" ;;\n@@ -288,15 +276,6 @@ __git_index_file_list_filter_bash ()\n \tdone\n }\n \n-# Process path list returned by \"ls-files\" and \"diff-index --name-only\"\n-# commands, in order to list only file names relative to a specified\n-# directory, and append a slash to directory names.\n-__git_index_file_list_filter ()\n-{\n-\t# Default to Bash >= 4.x\n-\t__git_index_file_list_filter_bash\n-}\n-\n # Execute 'git ls-files', unless the --committable option is specified, in\n # which case it runs 'git diff-index' to find out the files that can be\n # committed.  It return paths relative to the directory specified in the first\n@@ -2651,14 +2630,6 @@ if [[ -n ${ZSH_VERSION-} ]]; then\n \n \tcompdef _git git gitk\n \treturn\n-elif [[ -n ${BASH_VERSION-} ]]; then\n-\tif ((${BASH_VERSINFO[0]} < 4)); then\n-\t\t# compopt is not supported\n-\t\t__git_index_file_list_filter ()\n-\t\t{\n-\t\t\t__git_index_file_list_filter_compat\n-\t\t}\n-\tfi\n fi\n \n __git_func_wrap ()\n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215748","messageId":"1367093407-8898-9-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 8/9] completion: add space after completed filename","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:06Z","receivedAt":"2013-04-27T20:10:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Just like before fea16b4 (git-completion.bash: add support for path\ncompletion).\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\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 f9e8e7d..20c9718 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -255,7 +255,7 @@ __gitcomp_file ()\n \t__gitcompadd \"$1\" \"${2-}\" \"${3-$cur}\" \"\"\n \n \t# use a hack to enable file mode in bash < 4\n-\tcompopt -o filenames 2>/dev/null ||\n+\tcompopt -o filenames +o nospace 2>/dev/null ||\n \tcompgen -f /non-existing-dir/ > /dev/null\n }\n \n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215747","messageId":"1367093407-8898-10-git-send-email-felipe.contreras@gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 9/9] completion: remove __git_index_file_list_filter()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-27T20:10:07Z","receivedAt":"2013-04-27T20:10:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Refactor the code into the only caller; __git_index_files().\n\nAlso, Somehow messing up with the 'path' variable messes up the 'PATH'\nvariable. So let's not do that.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 28 ++++++++--------------------\n 1 file changed, 8 insertions(+), 20 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 20c9718..edb7428 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -259,23 +259,6 @@ __gitcomp_file ()\n \tcompgen -f /non-existing-dir/ > /dev/null\n }\n \n-# Process path list returned by \"ls-files\" and \"diff-index --name-only\"\n-# commands, in order to list only file names relative to a specified\n-# directory, and append a slash to directory names.\n-__git_index_file_list_filter ()\n-{\n-\tlocal path\n-\n-\twhile read -r path; do\n-\t\tcase \"$path\" in\n-\t\t?*/*)\n-\t\t\techo \"${path%%/*}\" ;;\n-\t\t*)\n-\t\t\techo \"$path\" ;;\n-\t\tesac\n-\tdone\n-}\n-\n # Execute 'git ls-files', unless the --committable option is specified, in\n # which case it runs 'git diff-index' to find out the files that can be\n # committed.  It return paths relative to the directory specified in the first\n@@ -303,11 +286,16 @@ __git_ls_files_helper ()\n #    slash.\n __git_index_files ()\n {\n-\tlocal dir=\"$(__gitdir)\" root=\"${2-.}\"\n+\tlocal dir=\"$(__gitdir)\" root=\"${2-.}\" file\n \n \tif [ -d \"$dir\" ]; then\n-\t\t__git_ls_files_helper \"$root\" \"$1\" | __git_index_file_list_filter |\n-\t\t\tsort | uniq\n+\t\t__git_ls_files_helper \"$root\" \"$1\" |\n+\t\twhile read -r file; do\n+\t\t\tcase \"$file\" in\n+\t\t\t?*/*) echo \"${file%%/*}\" ;;\n+\t\t\t*) echo \"$file\" ;;\n+\t\t\tesac\n+\t\tdone | sort | uniq\n \tfi\n }\n \n-- \n1.8.2.1.1031.g2ee5873\n"},{"id":"215757","messageId":"7vehdvu0og.fsf@alter.siamese.dyndns.org","threadId":"33654","inReplyTo":"1367093407-8898-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 1/9] completion: add file completion tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-27T21:24:31Z","receivedAt":"2013-04-27T21:24:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> The commit fea16b4 (git-completion.bash: add support for path\n> completion) introduced quite a few changes, without the usual tests.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 68 insertions(+)\n>\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 6d9d141..385e1e4 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -347,4 +347,72 @@ test_expect_success 'send-email' '\n>  \ttest_completion \"git send-email ma\" \"master \"\n>  '\n>  \n> +test_expect_success 'complete files' '\n> +\tgit init tmp && cd tmp &&\n> +\ttest_when_finished \"cd .. && rm -rf tmp\" &&\n\nHmm, this construct is not very nice.\n\nLater part of the code can be modified carelessly and chdir around\neven more, and \"cd ..\" may end up at an unexpected place.  Can't\nthis be done in a way to make later reviews of patches that come\nafter this change lands doable without looking at the whole 70\nlines?  For example, if the test ran the whole \"these are done in a\nsubdirectry\" part, starting from the \"cd tmp\", inside a subshell,\nand \"rm -rf tmp\" is done in the original shell that ran \"git init\ntmp\" to create it, it would make it much harder to make mistakes for\nanybody who add new code to the body of the test to affect the\nclean-up part.\n\n> +\n> +\techo \"expected\" > .gitignore &&\n> +\techo \"out\" >> .gitignore &&\n\nStyle: we tend to omit SP between redireciton operator and targets.\n\n> +\tgit add .gitignore &&\n> +\ttest_completion \"git commit \" \".gitignore\" &&\n> +\n> +\tgit commit -m ignore &&\n> +\n> +\ttouch new &&\n> +\ttest_completion \"git add \" \"new\" &&\n> +\n> +\tgit add new &&\n> +\tgit commit -a -m new &&\n> +\ttest_completion \"git add \" \"\" &&\n> +\n> +\tgit mv new modified &&\n> +\techo modify > modified &&\n> +\ttest_completion \"git add \" \"modified\" &&\n> +\n> +\ttouch untracked &&\n> +\n> +\t: TODO .gitignore should not be here &&\n> +\ttest_completion \"git rm \" <<-\\EOF &&\n> +\t.gitignore\n> +\tmodified\n> +\tEOF\n> +\n> +\ttest_completion \"git clean \" \"untracked\" &&\n> +\n> +\t: TODO .gitignore should not be here &&\n> +\ttest_completion \"git mv \" <<-\\EOF &&\n> +\t.gitignore\n> +\tmodified\n> +\tEOF\n> +\n> +\tmkdir dir &&\n> +\ttouch dir/file-in-dir &&\n> +\tgit add dir/file-in-dir &&\n> +\tgit commit -m dir &&\n> +\n> +\tmkdir untracked-dir &&\n> +\n> +\t: TODO .gitignore should not be here &&\n> +\ttest_completion \"git mv modified \" <<-\\EOF &&\n> +\t.gitignore\n> +\tdir\n> +\tmodified\n> +\tuntracked\n> +\tuntracked-dir\n> +\tEOF\n> +\n> +\ttest_completion \"git commit \" \"modified\" &&\n> +\n> +\t: TODO .gitignore should not be here &&\n> +\ttest_completion \"git ls-files \" <<-\\EOF\n> +\t.gitignore\n> +\tdir\n> +\tmodified\n> +\tEOF\n> +\n> +\ttouch momified &&\n> +\ttest_completion \"git add mom\" \"momified\"\n> +'\n> +\n>  test_done\n"},{"id":"215758","messageId":"7v7gjnu0gd.fsf@alter.siamese.dyndns.org","threadId":"33654","inReplyTo":"1367093407-8898-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 5/9] completion: refactor diff_index wrappers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-27T21:29:22Z","receivedAt":"2013-04-27T21:29:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> At the end of the day what we really need is to find out the files that\n> have been staged, or modified, because those files are the ones that\n> make sense to pass as arguments to 'git commit'.\n>\n> We need diff-index to find those out, since 'git ls-files' doesn't do\n> that.\n>\n> But we don't need wrappers and wrappers basically identical to the ones\n> used for 'git ls-files', when we can pretend it receives a --committable\n> option that would return what we need.\n>\n> That way, we can remove a bunch of code without any functional changes.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  contrib/completion/git-completion.bash | 71 ++++++++--------------------------\n>  1 file changed, 16 insertions(+), 55 deletions(-)\n\nNice line reduction.\n\n\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 7f1ebe4..2561265 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -297,30 +297,25 @@ __git_index_file_list_filter ()\n>  \t__git_index_file_list_filter_bash\n>  }\n>  \n> -# Execute git ls-files, returning paths relative to the directory\n> -# specified in the first argument, and using the options specified in\n> -# the second argument.\n> +# Execute 'git ls-files', unless the --committable option is specified, in\n> +# which case it runs 'git diff-index' to find out the files that can be\n> +# committed.  It return paths relative to the directory specified in the first\n> +# argument, and using the options specified in the second argument.\n>  __git_ls_files_helper ()\n>  {\n>  \t(\n>  \t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n> -\t\t# NOTE: $2 is not quoted in order to support multiple options\n> -\t\tcd \"$1\" && git ls-files --exclude-standard $2\n> +\t\tcd \"$1\"\n> +\t\tif [ \"$2\" == \"--committable\" ]; then\n> +\t\t\tgit diff-index --name-only --relative HEAD\n> +\t\telse\n> +\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n> +\t\t\tgit ls-files --exclude-standard $2\n> +\t\tfi\n>  \t) 2>/dev/null\n>  }\n>  \n>  \n> -# Execute git diff-index, returning paths relative to the directory\n> -# specified in the first argument, and using the tree object id\n> -# specified in the second argument.\n> -__git_diff_index_helper ()\n> -{\n> -\t(\n> -\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n> -\t\tcd \"$1\" && git diff-index --name-only --relative \"$2\"\n> -\t) 2>/dev/null\n> -}\n> -\n>  # __git_index_files accepts 1 or 2 arguments:\n>  # 1: Options to pass to ls-files (required).\n>  # 2: A directory path (optional).\n> @@ -337,22 +332,6 @@ __git_index_files ()\n>  \tfi\n>  }\n>  \n> -# __git_diff_index_files accepts 1 or 2 arguments:\n> -# 1) The id of a tree object.\n> -# 2) A directory path (optional).\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> -__git_diff_index_files ()\n> -{\n> -\tlocal dir=\"$(__gitdir)\" root=\"${2-.}\"\n> -\n> -\tif [ -d \"$dir\" ]; then\n> -\t\t__git_diff_index_helper \"$root\" \"$1\" | __git_index_file_list_filter |\n> -\t\t\tsort | uniq\n> -\tfi\n> -}\n> -\n>  __git_heads ()\n>  {\n>  \tlocal dir=\"$(__gitdir)\"\n> @@ -550,8 +529,10 @@ __git_complete_revlist_file ()\n>  }\n>  \n>  \n> -# __git_complete_index_file requires 1 argument: the options to pass to\n> -# ls-file\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> @@ -570,26 +551,6 @@ __git_complete_index_file ()\n>  \tesac\n>  }\n>  \n> -# __git_complete_diff_index_file requires 1 argument: the id of a tree\n> -# object\n> -__git_complete_diff_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> -\n> -\t\t__gitcomp_file \"$(__git_diff_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\n> -\t\t;;\n> -\t*)\n> -\t\t__gitcomp_file \"$(__git_diff_index_files \"$1\")\" \"\" \"$cur_\"\n> -\t\t;;\n> -\tesac\n> -}\n> -\n>  __git_complete_file ()\n>  {\n>  \t__git_complete_revlist_file\n> @@ -1211,7 +1172,7 @@ _git_commit ()\n>  \tesac\n>  \n>  \tif git rev-parse --verify --quiet HEAD >/dev/null; then\n> -\t\t__git_complete_diff_index_file \"HEAD\"\n> +\t\t__git_complete_index_file \"--committable\"\n>  \telse\n>  \t\t# This is the first commit\n>  \t\t__git_complete_index_file \"--cached\"\n"},{"id":"215759","messageId":"7v38ubu087.fsf@alter.siamese.dyndns.org","threadId":"33654","inReplyTo":"1367093407-8898-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 0/9] completion: general cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-27T21:34:16Z","receivedAt":"2013-04-27T21:34:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Overall this looks like a nice clean-up.  I didn't look at the 5/9\ncarefully to verify its claim that the change is a no-op, though.\n\nWill queue.\n"},{"id":"215777","messageId":"CAPig+cSp2wsvZaJ1vKn3UOGL4FyGFj4j4OFKQAeN-pVNMYZKaw@mail.gmail.com","threadId":"33654","inReplyTo":"1367093407-8898-10-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 9/9] completion: remove __git_index_file_list_filter()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-04-28T01:41:54Z","receivedAt":"2013-04-28T01:41:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 27, 2013 at 4:10 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> Refactor the code into the only caller; __git_index_files().\n>\n> Also, Somehow messing up with the 'path' variable messes up the 'PATH'\n\ns/Somehow/somehow/\ns/messing up/messing/\n\n> variable. So let's not do that.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n"},{"id":"215778","messageId":"CAMP44s0kqMVFF-PGJSiRJ9TYpA8UQrdbEUumMRuL=FDGtg0B3A@mail.gmail.com","threadId":"33654","inReplyTo":"CAPig+cSp2wsvZaJ1vKn3UOGL4FyGFj4j4OFKQAeN-pVNMYZKaw@mail.gmail.com","subject":"Re: [PATCH v2 9/9] completion: remove __git_index_file_list_filter()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-28T01:45:39Z","receivedAt":"2013-04-28T01:45:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Apr 27, 2013 at 8:41 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Sat, Apr 27, 2013 at 4:10 PM, Felipe Contreras\n> <felipe.contreras@gmail.com> wrote:\n>> Refactor the code into the only caller; __git_index_files().\n>>\n>> Also, Somehow messing up with the 'path' variable messes up the 'PATH'\n>\n> s/Somehow/somehow/\n> s/messing up/messing/\n\nMore important than that is the fact that the problem is only on zsh.\n\n-- \nFelipe Contreras\n"}]}