{"thread":{"id":"30182","subject":"[PATCH 03/12] completion: simplify __gitcomp_1","startedAt":"2012-04-08T03:07:47Z","lastAt":"2012-04-12T23:08:25Z","messageCount":48,"participants":["Felipe Contreras","Junio C Hamano","Jeff King","Andreas Schwab","John Keeping","Jonathan Nieder","SZEDER Gábor","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":12},"messages":[{"id":"188741","messageId":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":null,"subject":"[PATCH 00/12] Bash completion rework","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:47Z","receivedAt":"2012-04-08T03:07:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nHere's a bunch of patches for bash completion. First of all some tests are\nadded to verify that there are no regressions, then simple cleanups, then some\nfixes, and finally support for external aliases.\n\nSo now you can do something like 'git_complete gf git_fetch' and it would just work :)\n\nCheers.\n\nFelipe Contreras (12):\n  tests: add initial bash completion tests\n  completion: simplify __gitcomp\n  completion: simplify __gitcomp_1\n  completion: trivial simplification\n  completion: add missing global options\n  tests: add more bash completion tests\n  completion: simplify command stuff\n  completion: simplify _git_bundle\n  completion: calculate argument position properly\n  completion: add new git_complete helper\n  test: add tests for aliases in bash completion\n  completion: rename _git and _gitk\n\n contrib/completion/git-completion.bash |  153 +++++++++-------\n t/t9902-completion.sh                  |  314 ++++++++++++++++++++++++++++++++\n 2 files changed, 398 insertions(+), 69 deletions(-)\n create mode 100755 t/t9902-completion.sh\n\n-- \n1.7.10.3.g5a738d\n"},{"id":"188742","messageId":"1333854479-23260-2-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 01/12] tests: add initial bash completion tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:48Z","receivedAt":"2012-04-08T03:07:48Z","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 |  201 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 201 insertions(+)\n create mode 100755 t/t9902-completion.sh\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nnew file mode 100755\nindex 0000000..7eb80dd\n--- /dev/null\n+++ b/t/t9902-completion.sh\n@@ -0,0 +1,201 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2012 Felipe Contreras\n+#\n+\n+if type bash >/dev/null 2>&1\n+then\n+\t# execute inside bash\n+\ttest -z \"$BASH\" && exec bash \"$0\" \"$@\"\n+else\n+\techo '1..0 #SKIP skipping bash completion tests; bash not available'\n+\texit 0\n+fi\n+\n+test_description='test bash completion'\n+\n+. ./test-lib.sh\n+\n+complete ()\n+{\n+\t# do nothing\n+\treturn 0\n+}\n+\n+. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n+\n+print_comp ()\n+{\n+\tlocal IFS=$'\\n'\n+\techo \"${COMPREPLY[*]}\" > out\n+}\n+\n+_get_comp_words_by_ref ()\n+{\n+\twhile [ $# -gt 0 ]; do\n+\t\tcase \"$1\" in\n+\t\tcur)\n+\t\t\tcur=${_words[_cword]}\n+\t\t\t;;\n+\t\tprev)\n+\t\t\tprev=${_words[_cword-1]}\n+\t\t\t;;\n+\t\twords)\n+\t\t\twords=(\"${_words[@]}\")\n+\t\t\t;;\n+\t\tcword)\n+\t\t\tcword=$_cword\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+}\n+\n+test_completion ()\n+{\n+\tlocal -a COMPREPLY _words\n+\tlocal _cword\n+\t_words=( $1 )\n+\ttest $# -gt 1 && echo \"$2\" > expected\n+\t(( _cword = ${#_words[@]} - 1 ))\n+\t_git && print_comp &&\n+\ttest_cmp expected out\n+}\n+\n+test_expect_success 'basic' '\n+\tcat >expected <<-\\EOF &&\n+\thelp \n+\tadd \n+\tgc \n+\treflog \n+\tget-tar-commit-id \n+\trelink \n+\tgrep \n+\trelink.perl \n+\tam \n+\tremote \n+\tam.sh \n+\thelp \n+\tannotate \n+\tapply \n+\tarchimport.perl \n+\timap-send \n+\tarchive \n+\tbisect \n+\tinit \n+\trepack \n+\tbisect.sh \n+\tinstaweb \n+\trepack.sh \n+\tblame \n+\tinstaweb.sh \n+\treplace \n+\tbranch \n+\tlog \n+\tbundle \n+\trequest-pull \n+\tlost-found.sh \n+\trequest-pull.sh \n+\treset \n+\tcheckout \n+\tcherry \n+\trevert \n+\tcherry-pick \n+\tmerge \n+\trm \n+\tclean \n+\tsend-email \n+\tclone \n+\tsend-email.perl \n+\tcommit \n+\tconfig \n+\tshortlog \n+\tcredential-cache \n+\tshow \n+\tshow-branch \n+\tcredential-store \n+\tcvsexportcommit.perl \n+\tstage \n+\tstash \n+\tcvsimport.perl \n+\tstash.sh \n+\tmergetool \n+\tstatus \n+\tcvsserver.perl \n+\tmergetool.sh \n+\tsubmodule \n+\tdescribe \n+\tsubmodule.sh \n+\tdiff \n+\tmv \n+\tsvn \n+\tname-rev \n+\tsvn.perl \n+\tnotes \n+\ttag \n+\tdifftool \n+\tdifftool.perl \n+\tfetch \n+\tpull \n+\tpull.sh \n+\tfilter-branch \n+\tpush \n+\tfilter-branch.sh \n+\tquiltimport.sh \n+\tformat-patch \n+\trebase \n+\tfsck \n+\trebase.sh \n+\twhatchanged \n+\tcccmd \n+\tproxy \n+\tsend-pull \n+\tEOF\n+\ttest_completion \"git\" &&\n+\n+\tcat >expected <<-\\EOF &&\n+\thelp \n+\thelp \n+\tEOF\n+\ttest_completion \"git he\"\n+\n+\tcat >expected <<-\\EOF &&\n+\tfetch \n+\tfilter-branch \n+\tfilter-branch.sh \n+\tformat-patch \n+\tfsck \n+\tEOF\n+\ttest_completion \"git f\"\n+'\n+\n+test_expect_success 'double dash' '\n+\tcat >expected <<-\\EOF &&\n+\t--paginate \n+\t--no-pager \n+\t--git-dir=\n+\t--bare \n+\t--version \n+\t--exec-path \n+\t--html-path \n+\t--work-tree=\n+\t--namespace=\n+\t--help \n+\tEOF\n+\ttest_completion \"git --\"\n+\n+\tcat >expected <<-\\EOF &&\n+\t--quiet \n+\t--ours \n+\t--theirs \n+\t--track \n+\t--no-track \n+\t--merge \n+\t--conflict=\n+\t--orphan \n+\t--patch \n+\tEOF\n+\ttest_completion \"git checkout --\"\n+'\n+\n+test_done\n-- \n1.7.10.3.g5a738d\n"},{"id":"188748","messageId":"1333854479-23260-3-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 02/12] completion: simplify __gitcomp","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:49Z","receivedAt":"2012-04-08T03:07:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The suffix is never used.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |   12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 31f714d..39f5435 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -304,15 +304,14 @@ __git_ps1 ()\n \tfi\n }\n \n-# __gitcomp_1 requires 2 arguments\n __gitcomp_1 ()\n {\n \tlocal c IFS=' '$'\\t'$'\\n'\n \tfor c in $1; do\n-\t\tcase \"$c$2\" in\n-\t\t--*=*) printf %s$'\\n' \"$c$2\" ;;\n-\t\t*.)    printf %s$'\\n' \"$c$2\" ;;\n-\t\t*)     printf %s$'\\n' \"$c$2 \" ;;\n+\t\tcase \"$c\" in\n+\t\t--*=*) printf %s$'\\n' \"$c\" ;;\n+\t\t*.)    printf %s$'\\n' \"$c\" ;;\n+\t\t*)     printf %s$'\\n' \"$c \" ;;\n \t\tesac\n \tdone\n }\n@@ -479,7 +478,6 @@ fi\n # 1: List of possible completion words.\n # 2: A prefix to be added to each possible completion word (optional).\n # 3: Generate possible completion matches for this word (optional).\n-# 4: A suffix to be appended to each possible completion word (optional).\n __gitcomp ()\n {\n \tlocal cur_=\"${3-$cur}\"\n@@ -491,7 +489,7 @@ __gitcomp ()\n \t*)\n \t\tlocal IFS=$'\\n'\n \t\tCOMPREPLY=($(compgen -P \"${2-}\" \\\n-\t\t\t-W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n+\t\t\t-W \"$(__gitcomp_1 \"${1-}\")\" \\\n \t\t\t-- \"$cur_\"))\n \t\t;;\n \tesac\n-- \n1.7.10.3.g5a738d\n"},{"id":"188740","messageId":"1333854479-23260-4-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 03/12] completion: simplify __gitcomp_1","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:50Z","receivedAt":"2012-04-08T03:07:50Z","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 contrib/completion/git-completion.bash |    8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 39f5435..5c66674 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -306,13 +306,13 @@ __git_ps1 ()\n \n __gitcomp_1 ()\n {\n-\tlocal c IFS=' '$'\\t'$'\\n'\n+\tlocal c s IFS=' '$'\\t'$'\\n'\n \tfor c in $1; do\n \t\tcase \"$c\" in\n-\t\t--*=*) printf %s$'\\n' \"$c\" ;;\n-\t\t*.)    printf %s$'\\n' \"$c\" ;;\n-\t\t*)     printf %s$'\\n' \"$c \" ;;\n+\t\t--*=* | *.) s=\"\" ;;\n+\t\t*)          s=\" \" ;;\n \t\tesac\n+\t\techo \"$c$s\"\n \tdone\n }\n \n-- \n1.7.10.3.g5a738d\n"},{"id":"188750","messageId":"1333854479-23260-5-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 04/12] completion: trivial simplification","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:51Z","receivedAt":"2012-04-08T03:07:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"cword-1 is the previous word ($prev).\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |    4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 5c66674..3bc8d85 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1656,7 +1656,7 @@ _git_notes ()\n \t\t__gitcomp '--ref'\n \t\t;;\n \t,*)\n-\t\tcase \"${words[cword-1]}\" in\n+\t\tcase \"$prev\" in\n \t\t--ref)\n \t\t\t__gitcomp_nl \"$(__git_refs)\"\n \t\t\t;;\n@@ -1682,7 +1682,7 @@ _git_notes ()\n \tprune,*)\n \t\t;;\n \t*)\n-\t\tcase \"${words[cword-1]}\" in\n+\t\tcase \"$prev\" in\n \t\t-m|-F)\n \t\t\t;;\n \t\t*)\n-- \n1.7.10.3.g5a738d\n"},{"id":"188749","messageId":"1333854479-23260-6-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 05/12] completion: add missing global options","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:52Z","receivedAt":"2012-04-08T03:07:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Otherwise 'git --foo bar<tab>' fails, also, other options are completely\nmissing.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |    9 +++++++--\n t/t9902-completion.sh                  |   31 +++++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 3bc8d85..c9672b2 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2621,7 +2621,10 @@ _git ()\n \t\tcase \"$i\" in\n \t\t--git-dir=*) __git_dir=\"${i#--git-dir=}\" ;;\n \t\t--bare)      __git_dir=\".\" ;;\n-\t\t--version|-p|--paginate) ;;\n+\t\t--version) ;;\n+\t\t-p|--paginate|--no-pager) ;;\n+\t\t--exec-path=*|--html-path|--info-path) ;;\n+\t\t--work-tree=*|--namespace=*|--no-replace-objects) ;;\n \t\t--help) command=\"help\"; break ;;\n \t\t*) command=\"$i\"; break ;;\n \t\tesac\n@@ -2636,10 +2639,12 @@ _git ()\n \t\t\t--git-dir=\n \t\t\t--bare\n \t\t\t--version\n-\t\t\t--exec-path\n+\t\t\t--exec-path=\n \t\t\t--html-path\n+\t\t\t--info-path\n \t\t\t--work-tree=\n \t\t\t--namespace=\n+\t\t\t--no-replace-objects\n \t\t\t--help\n \t\t\t\"\n \t\t\t;;\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 7eb80dd..ee5654d 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -198,4 +198,35 @@ test_expect_success 'double dash' '\n \ttest_completion \"git checkout --\"\n '\n \n+test_expect_success 'general options' '\n+\ttest_completion \"git --ver\" \"--version \" &&\n+\ttest_completion \"git --hel\" \"--help \" &&\n+\ttest_completion \"git --exe\" \"--exec-path=\" &&\n+\ttest_completion \"git --htm\" \"--html-path \" &&\n+\ttest_completion \"git --pag\" \"--paginate \" &&\n+\ttest_completion \"git --no-p\" \"--no-pager \" &&\n+\ttest_completion \"git --git\" \"--git-dir=\" &&\n+\ttest_completion \"git --wor\" \"--work-tree=\" &&\n+\ttest_completion \"git --nam\" \"--namespace=\" &&\n+\ttest_completion \"git --bar\" \"--bare \" &&\n+\ttest_completion \"git --inf\" \"--info-path \" &&\n+\ttest_completion \"git --no-r\" \"--no-replace-objects \"\n+'\n+\n+test_expect_success 'general options plus command' '\n+\ttest_completion \"git --version check\" \"checkout \" &&\n+\ttest_completion \"git --paginate check\" \"checkout \" &&\n+\ttest_completion \"git --git-dir=foo check\" \"checkout \" &&\n+\ttest_completion \"git --bare check\" \"checkout \" &&\n+\ttest_completion \"git --help des\" \"describe \" &&\n+\ttest_completion \"git --exec-path=foo check\" \"checkout \" &&\n+\ttest_completion \"git --html-path check\" \"checkout \" &&\n+\ttest_completion \"git --no-pager check\" \"checkout \" &&\n+\ttest_completion \"git --work-tree=foo check\" \"checkout \" &&\n+\ttest_completion \"git --namespace=foo check\" \"checkout \" &&\n+\ttest_completion \"git --paginate check\" \"checkout \" &&\n+\ttest_completion \"git --info-path check\" \"checkout \" &&\n+\ttest_completion \"git --no-replace-objects check\" \"checkout \"\n+'\n+\n test_done\n-- \n1.7.10.3.g5a738d\n"},{"id":"188751","messageId":"1333854479-23260-7-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 06/12] tests: add more bash completion tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:53Z","receivedAt":"2012-04-08T03:07:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"These tests try to excercise code that deals with 'words' and 'cword'.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh |   55 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 55 insertions(+)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex ee5654d..f1b660f 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -62,6 +62,22 @@ test_completion ()\n \ttest_cmp expected out\n }\n \n+setup_repository ()\n+{\n+\tmkdir \"$1\" && (\n+\tcd \"$1\" &&\n+\tgit init &&\n+\ttest_tick &&\n+\tgit commit --allow-empty -m \"Initial\"\n+\t)\n+}\n+\n+test_expect_success 'prepare' '\n+\tsetup_repository one &&\n+\tgit clone one test &&\n+\tcd test\n+'\n+\n test_expect_success 'basic' '\n \tcat >expected <<-\\EOF &&\n \thelp \n@@ -229,4 +245,43 @@ test_expect_success 'general options plus command' '\n \ttest_completion \"git --no-replace-objects check\" \"checkout \"\n '\n \n+test_expect_success 'remote or refspec' '\n+\ttest_completion \"git fetch o\" \"origin \" &&\n+\ttest_completion \"git fetch origin m\" \"master:master \" &&\n+\ttest_completion \"git pull o\" \"origin \" &&\n+\ttest_completion \"git pull origin m\" \"master \" &&\n+\ttest_completion \"git push o\" \"origin \" &&\n+\ttest_completion \"git push origin m\" \"master \"\n+'\n+\n+test_expect_success 'subcomands' '\n+\ttest_completion \"git bisect st\" \"start \"\n+'\n+\n+test_expect_success 'has double dash' '\n+\ttest_completion \"git add -- foo\" \"\"\n+'\n+\n+test_expect_success 'config' '\n+\tgit config --file=foo color.ui auto &&\n+\ttest_completion \"git config --file=foo --get c\" \"color.ui \"\n+'\n+\n+test_expect_success 'other' '\n+\tcat >expected <<-\\EOF &&\n+\torigin/HEAD \n+\torigin/master \n+\tEOF\n+\ttest_completion \"git branch -r o\" &&\n+\ttest_completion \"git bundle cr\" \"create \" &&\n+\n+\techo foobar > tags &&\n+\ttest_completion \"git grep f\" \"foobar \" &&\n+\n+\ttest_completion \"git notes --ref m\" \"master \" &&\n+\n+\tgit tag v0.1 &&\n+\ttest_completion \"git tag -d v\" \"v0.1 \"\n+'\n+\n test_done\n-- \n1.7.10.3.g5a738d\n"},{"id":"188752","messageId":"1333854479-23260-8-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 07/12] completion: simplify command stuff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:54Z","receivedAt":"2012-04-08T03:07:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No need to recalculate it.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |   14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c9672b2..1fe11f4 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -721,7 +721,7 @@ __git_complete_revlist ()\n \n __git_complete_remote_or_refspec ()\n {\n-\tlocal cur_=\"$cur\" cmd=\"${words[1]}\"\n+\tlocal cur_=\"$cur\"\n \tlocal i c=2 remote=\"\" pfx=\"\" lhs=1 no_complete_refspec=0\n \tif [ \"$cmd\" = \"remote\" ]; then\n \t\t((c++))\n@@ -2599,7 +2599,7 @@ _git_whatchanged ()\n \n _git ()\n {\n-\tlocal i c=1 command __git_dir\n+\tlocal i c=1 cmd __git_dir\n \n \tif [[ -n ${ZSH_VERSION-} ]]; then\n \t\temulate -L bash\n@@ -2625,13 +2625,13 @@ _git ()\n \t\t-p|--paginate|--no-pager) ;;\n \t\t--exec-path=*|--html-path|--info-path) ;;\n \t\t--work-tree=*|--namespace=*|--no-replace-objects) ;;\n-\t\t--help) command=\"help\"; break ;;\n-\t\t*) command=\"$i\"; break ;;\n+\t\t--help) cmd=\"help\"; break ;;\n+\t\t*) cmd=\"$i\"; break ;;\n \t\tesac\n \t\t((c++))\n \tdone\n \n-\tif [ -z \"$command\" ]; then\n+\tif [ -z \"$cmd\" ]; then\n \t\tcase \"$cur\" in\n \t\t--*)   __gitcomp \"\n \t\t\t--paginate\n@@ -2654,10 +2654,10 @@ _git ()\n \t\treturn\n \tfi\n \n-\tlocal completion_func=\"_git_${command//-/_}\"\n+\tlocal completion_func=\"_git_${cmd//-/_}\"\n \tdeclare -f $completion_func >/dev/null && $completion_func && return\n \n-\tlocal expansion=$(__git_aliased_command \"$command\")\n+\tlocal expansion=$(__git_aliased_command \"$cmd\")\n \tif [ -n \"$expansion\" ]; then\n \t\tcompletion_func=\"_git_${expansion//-/_}\"\n \t\tdeclare -f $completion_func >/dev/null && $completion_func\n-- \n1.7.10.3.g5a738d\n"},{"id":"188744","messageId":"1333854479-23260-9-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 08/12] completion: simplify _git_bundle","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:55Z","receivedAt":"2012-04-08T03:07:55Z","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 contrib/completion/git-completion.bash |    8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 1fe11f4..c26f96c 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1140,16 +1140,18 @@ _git_branch ()\n \n _git_bundle ()\n {\n-\tlocal cmd=\"${words[2]}\"\n+\tlocal subcommands='create list-heads verify unbundle'\n+\tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n+\n \tcase \"$cword\" in\n \t2)\n-\t\t__gitcomp \"create list-heads verify unbundle\"\n+\t\t__gitcomp \"$subcommands\"\n \t\t;;\n \t3)\n \t\t# looking for a file\n \t\t;;\n \t*)\n-\t\tcase \"$cmd\" in\n+\t\tcase \"$subcommand\" in\n \t\t\tcreate)\n \t\t\t\t__git_complete_revlist\n \t\t\t;;\n-- \n1.7.10.3.g5a738d\n"},{"id":"188746","messageId":"1333854479-23260-10-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 09/12] completion: calculate argument position properly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:56Z","receivedAt":"2012-04-08T03:07:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Positions are currently hard-coded, which means completions in the form\nof 'git --foo bar' fail, because positions have been shifted.\n\nThis fixes the issue that was spotted in the mailing list regarding\ncertain aliases[1].\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/185184\n\nCc: SZEDER Gábor <szeder@ira.uka.de>\nCc: Scott Bronson <bronson@rinspin.com>\nCc: Nathan Broadbent <nathan.f77@gmail.com>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |   35 +++++++++++++++++++++++---------\n t/t9902-completion.sh                  |   21 +++++++++++++++++++\n 2 files changed, 46 insertions(+), 10 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c26f96c..60ea224 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -719,10 +719,18 @@ __git_complete_revlist ()\n \t__git_complete_revlist_file\n }\n \n+__git_get_pos ()\n+{\n+\techo $(( t = cmd_pos - 2 + $1 ))\n+}\n+\n __git_complete_remote_or_refspec ()\n {\n \tlocal cur_=\"$cur\"\n-\tlocal i c=2 remote=\"\" pfx=\"\" lhs=1 no_complete_refspec=0\n+\tlocal i c remote=\"\" pfx=\"\" lhs=1 no_complete_refspec=0\n+\n+\tc=$(__git_get_pos 2)\n+\n \tif [ \"$cmd\" = \"remote\" ]; then\n \t\t((c++))\n \tfi\n@@ -974,7 +982,8 @@ __git_aliased_command ()\n # __git_find_on_cmdline requires 1 argument\n __git_find_on_cmdline ()\n {\n-\tlocal word subcommand c=1\n+\tlocal word subcommand c\n+\tc=$(__git_get_pos 1)\n \twhile [ $c -lt $cword ]; do\n \t\tword=\"${words[c]}\"\n \t\tfor subcommand in $1; do\n@@ -989,7 +998,8 @@ __git_find_on_cmdline ()\n \n __git_has_doubledash ()\n {\n-\tlocal c=1\n+\tlocal c\n+\tc=$(__git_get_pos 1)\n \twhile [ $c -lt $cword ]; do\n \t\tif [ \"--\" = \"${words[c]}\" ]; then\n \t\t\treturn 0\n@@ -1109,8 +1119,8 @@ _git_bisect ()\n \n _git_branch ()\n {\n-\tlocal i c=1 only_local_ref=\"n\" has_r=\"n\"\n-\n+\tlocal i c only_local_ref=\"n\" has_r=\"n\"\n+\tc=$(__git_get_pos 1)\n \twhile [ $c -lt $cword ]; do\n \t\ti=\"${words[c]}\"\n \t\tcase \"$i\" in\n@@ -1142,8 +1152,9 @@ _git_bundle ()\n {\n \tlocal subcommands='create list-heads verify unbundle'\n \tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n-\n-\tcase \"$cword\" in\n+\tlocal r\n+\t(( r = cword - cmd_pos + 2 ))\n+\tcase \"$r\" in\n \t2)\n \t\t__gitcomp \"$subcommands\"\n \t\t;;\n@@ -1427,6 +1438,7 @@ __git_match_ctag() {\n _git_grep ()\n {\n \t__git_has_doubledash && return\n+\tlocal p=$(__git_get_pos 2)\n \n \tcase \"$cur\" in\n \t--*)\n@@ -1447,7 +1459,7 @@ _git_grep ()\n \tesac\n \n \tcase \"$cword,$prev\" in\n-\t2,*|*,-*)\n+\t$p,*|*,-*)\n \t\tif test -r tags; then\n \t\t\t__gitcomp_nl \"$(__git_match_ctag \"$cur\" tags)\"\n \t\t\treturn\n@@ -2562,7 +2574,8 @@ _git_svn ()\n \n _git_tag ()\n {\n-\tlocal i c=1 f=0\n+\tlocal i c f=0\n+\tc=$(__git_get_pos 1)\n \twhile [ $c -lt $cword ]; do\n \t\ti=\"${words[c]}\"\n \t\tcase \"$i\" in\n@@ -2601,7 +2614,7 @@ _git_whatchanged ()\n \n _git ()\n {\n-\tlocal i c=1 cmd __git_dir\n+\tlocal i c=1 cmd cmd_pos __git_dir\n \n \tif [[ -n ${ZSH_VERSION-} ]]; then\n \t\temulate -L bash\n@@ -2656,6 +2669,8 @@ _git ()\n \t\treturn\n \tfi\n \n+\t(( cmd_pos = c + 1 ))\n+\n \tlocal completion_func=\"_git_${cmd//-/_}\"\n \tdeclare -f $completion_func >/dev/null && $completion_func && return\n \ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex f1b660f..b99fb88 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -284,4 +284,25 @@ test_expect_success 'other' '\n \ttest_completion \"git tag -d v\" \"v0.1 \"\n '\n \n+test_expect_success 'global options extra checks' '\n+\ttest_completion \"git --no-pager fetch o\" \"origin \" &&\n+\ttest_completion \"git --no-pager fetch origin m\" \"master:master \" &&\n+\ttest_completion \"git --no-pager pull o\" \"origin \" &&\n+\ttest_completion \"git --no-pager pull origin m\" \"master \" &&\n+\ttest_completion \"git --no-pager push o\" \"origin \" &&\n+\ttest_completion \"git --no-pager push origin m\" \"master \" &&\n+\ttest_completion \"git --no-pager bisect st\" \"start \" &&\n+\ttest_completion \"git --no-pager add -- foo\" \"\" &&\n+\ttest_completion \"git --no-pager config --file=foo --get c\" \"color.ui \" &&\n+\tcat >expected <<-\\EOF &&\n+\torigin/HEAD \n+\torigin/master \n+\tEOF\n+\ttest_completion \"git --no-pager branch -r o\" &&\n+\ttest_completion \"git --no-pager bundle cr\" \"create \" &&\n+\ttest_completion \"git --no-pager grep f\" \"foobar \" &&\n+\ttest_completion \"git --no-pager notes --ref m\" \"master \" &&\n+\ttest_completion \"git --no-pager tag -d v\" \"v0.1 \"\n+'\n+\n test_done\n-- \n1.7.10.3.g5a738d\n"},{"id":"188743","messageId":"1333854479-23260-11-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 10/12] completion: add new git_complete helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:57Z","receivedAt":"2012-04-08T03:07:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This simplifies the completions, and makes it easier to define aliases:\n\n git_complete gf git_fetch\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |   69 +++++++++++++++-----------------\n t/t9902-completion.sh                  |    2 +-\n 2 files changed, 33 insertions(+), 38 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 60ea224..6cf1d98 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2616,21 +2616,6 @@ _git ()\n {\n \tlocal i c=1 cmd cmd_pos __git_dir\n \n-\tif [[ -n ${ZSH_VERSION-} ]]; then\n-\t\temulate -L bash\n-\t\tsetopt KSH_TYPESET\n-\n-\t\t# workaround zsh's bug that leaves 'words' as a special\n-\t\t# variable in versions < 4.3.12\n-\t\ttypeset -h words\n-\n-\t\t# workaround zsh's bug that quotes spaces in the COMPREPLY\n-\t\t# array if IFS doesn't contain spaces.\n-\t\ttypeset -h IFS\n-\tfi\n-\n-\tlocal cur words cword prev\n-\t_get_comp_words_by_ref -n =: cur words cword prev\n \twhile [ $c -lt $cword ]; do\n \t\ti=\"${words[c]}\"\n \t\tcase \"$i\" in\n@@ -2683,22 +2668,6 @@ _git ()\n \n _gitk ()\n {\n-\tif [[ -n ${ZSH_VERSION-} ]]; then\n-\t\temulate -L bash\n-\t\tsetopt KSH_TYPESET\n-\n-\t\t# workaround zsh's bug that leaves 'words' as a special\n-\t\t# variable in versions < 4.3.12\n-\t\ttypeset -h words\n-\n-\t\t# workaround zsh's bug that quotes spaces in the COMPREPLY\n-\t\t# array if IFS doesn't contain spaces.\n-\t\ttypeset -h IFS\n-\tfi\n-\n-\tlocal cur words cword prev\n-\t_get_comp_words_by_ref -n =: cur words cword prev\n-\n \t__git_has_doubledash && return\n \n \tlocal g=\"$(__gitdir)\"\n@@ -2719,16 +2688,42 @@ _gitk ()\n \t__git_complete_revlist\n }\n \n-complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \\\n-\t|| complete -o default -o nospace -F _git git\n-complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \\\n-\t|| complete -o default -o nospace -F _gitk gitk\n+foo_wrap ()\n+{\n+\tlocal cmd=foo_cmd cmd_pos=1\n+\tif [[ -n ${ZSH_VERSION-} ]]; then\n+\t\temulate -L bash\n+\t\tsetopt KSH_TYPESET\n+\n+\t\t# workaround zsh's bug that leaves 'words' as a special\n+\t\t# variable in versions < 4.3.12\n+\t\ttypeset -h words\n+\n+\t\t# workaround zsh's bug that quotes spaces in the COMPREPLY\n+\t\t# array if IFS doesn't contain spaces.\n+\t\ttypeset -h IFS\n+\tfi\n+\tlocal cur words cword prev\n+\t_get_comp_words_by_ref -n =: cur words cword prev\n+\tfoo \"$@\"\n+}\n+\n+git_complete ()\n+{\n+\tlocal name=\"${2-$1}\"\n+\tlocal cmd=\"${name#git_}\"\n+\teval \"$(typeset -f foo_wrap | sed -e \"s/foo_cmd/$cmd/\" -e \"s/foo/_$name/\")\"\n+\tcomplete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \\\n+\t\t|| complete -o default -o nospace -F _${name}_wrap $1\n+}\n+\n+git_complete git\n+git_complete gitk\n \n # The following are necessary only for Cygwin, and only are needed\n # when the user has tab-completed the executable name and consequently\n # included the '.exe' suffix.\n #\n if [ Cygwin = \"$(uname -o 2>/dev/null)\" ]; then\n-complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \\\n-\t|| complete -o default -o nospace -F _git git.exe\n+git_complete git.exe git\n fi\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex b99fb88..7b87f4c 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -58,7 +58,7 @@ test_completion ()\n \t_words=( $1 )\n \ttest $# -gt 1 && echo \"$2\" > expected\n \t(( _cword = ${#_words[@]} - 1 ))\n-\t_git && print_comp &&\n+\t_git_wrap && print_comp &&\n \ttest_cmp expected out\n }\n \n-- \n1.7.10.3.g5a738d\n"},{"id":"188747","messageId":"1333854479-23260-12-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 11/12] test: add tests for aliases in bash completion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:58Z","receivedAt":"2012-04-08T03:07:58Z","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 |   10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 7b87f4c..6c61e7a 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -54,11 +54,12 @@ _get_comp_words_by_ref ()\n test_completion ()\n {\n \tlocal -a COMPREPLY _words\n-\tlocal _cword\n+\tlocal _cword wrap\n \t_words=( $1 )\n \ttest $# -gt 1 && echo \"$2\" > expected\n+\twrap=\"${3-_git_wrap}\"\n \t(( _cword = ${#_words[@]} - 1 ))\n-\t_git_wrap && print_comp &&\n+\t$wrap && print_comp &&\n \ttest_cmp expected out\n }\n \n@@ -305,4 +306,9 @@ test_expect_success 'global options extra checks' '\n \ttest_completion \"git --no-pager tag -d v\" \"v0.1 \"\n '\n \n+test_expect_success 'aliases' '\n+\tgit_complete gf git_fetch &&\n+\ttest_completion \"gf o\" \"origin \" \"_git_fetch_wrap\"\n+'\n+\n test_done\n-- \n1.7.10.3.g5a738d\n"},{"id":"188745","messageId":"1333854479-23260-13-git-send-email-felipe.contreras@gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 12/12] completion: rename _git and _gitk","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:07:59Z","receivedAt":"2012-04-08T03:07:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Would be helpful to reuse these for zsh completion; it uses _git, and it\ncannot be changed.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash |    8 ++++----\n t/t9902-completion.sh                  |    2 +-\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 6cf1d98..7d663f7 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2612,7 +2612,7 @@ _git_whatchanged ()\n \t_git_log\n }\n \n-_git ()\n+_main_git ()\n {\n \tlocal i c=1 cmd cmd_pos __git_dir\n \n@@ -2666,7 +2666,7 @@ _git ()\n \tfi\n }\n \n-_gitk ()\n+_main_gitk ()\n {\n \t__git_has_doubledash && return\n \n@@ -2710,7 +2710,7 @@ foo_wrap ()\n \n git_complete ()\n {\n-\tlocal name=\"${2-$1}\"\n+\tlocal name=\"${2-main_$1}\"\n \tlocal cmd=\"${name#git_}\"\n \teval \"$(typeset -f foo_wrap | sed -e \"s/foo_cmd/$cmd/\" -e \"s/foo/_$name/\")\"\n \tcomplete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \\\n@@ -2725,5 +2725,5 @@ git_complete gitk\n # included the '.exe' suffix.\n #\n if [ Cygwin = \"$(uname -o 2>/dev/null)\" ]; then\n-git_complete git.exe git\n+git_complete git.exe main_git\n fi\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 6c61e7a..ef1323f 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -57,7 +57,7 @@ test_completion ()\n \tlocal _cword wrap\n \t_words=( $1 )\n \ttest $# -gt 1 && echo \"$2\" > expected\n-\twrap=\"${3-_git_wrap}\"\n+\twrap=\"${3-_main_git_wrap}\"\n \t(( _cword = ${#_words[@]} - 1 ))\n \t$wrap && print_comp &&\n \ttest_cmp expected out\n-- \n1.7.10.3.g5a738d\n"},{"id":"188753","messageId":"CAMP44s2h==jd4fNFSSJXC2xTkHDozRSrH_2CHDzSB5=Jfu9uWA@mail.gmail.com","threadId":"30182","inReplyTo":"1333854479-23260-12-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 11/12] test: add tests for aliases in bash completion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T03:20:38Z","receivedAt":"2012-04-08T03:20:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 8, 2012 at 6:07 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh |   10 ++++++++--\n>  1 file changed, 8 insertions(+), 2 deletions(-)\n\nMaybe something like this would be better:\n\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -58,7 +58,7 @@ test_completion ()\n        _words=( $1 )\n        test $# -gt 1 && echo \"$2\" > expected\n        (( _cword = ${#_words[@]} - 1 ))\n-       _git_wrap && print_comp &&\n+       ${comp_wrap-_git_wrap} && print_comp &&\n        test_cmp expected out\n }\n\n@@ -305,4 +305,11 @@ test_expect_success 'global options extra checks' '\n        test_completion \"git --no-pager tag -d v\" \"v0.1 \"\n '\n\n+test_expect_success 'aliases' '\n+       local comp_wrap=_git_fetch_wrap &&\n+       git_complete gf git_fetch &&\n+       test_completion \"gf o\" \"origin \" &&\n+       test_completion \"gf origin m\" \"master:master \"\n+'\n+\n test_done\n\n\n-- \nFelipe Contreras\n"},{"id":"188756","messageId":"7vpqbiyhmu.fsf@alter.siamese.dyndns.org","threadId":"30182","inReplyTo":"1333854479-23260-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-08T04:25:29Z","receivedAt":"2012-04-08T04:25:29Z","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> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh |  201 +++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 201 insertions(+)\n>  create mode 100755 t/t9902-completion.sh\n>\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> new file mode 100755\n> index 0000000..7eb80dd\n> --- /dev/null\n> +++ b/t/t9902-completion.sh\n> @@ -0,0 +1,201 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2012 Felipe Contreras\n> +#\n> +\n> +if type bash >/dev/null 2>&1\n> +then\n> +\t# execute inside bash\n> +\ttest -z \"$BASH\" && exec bash \"$0\" \"$@\"\n> +else\n> +\techo '1..0 #SKIP skipping bash completion tests; bash not available'\n> +\texit 0\n> +fi\n\nWhat shell do you use on your system as /bin/sh (or if you use SHELL_PATH\nin the Makefile to override it, what do you use)?\n\nBash has a special case code to behave in a \"posixly correct\" mode, where\na lot of extensions are disabled, when invoked as \"sh\".  Unfortunately, it\nstill defines BASH_* variables, including BASH, when operating in this\nmode.\n\nThis unfortunately breaks the above code on a system where /bin/sh is\nsymlinked to \"bash\".  \"make\" runs test scripts using with SHELL_PATH,\ndefaulting to /bin/sh, and you say \"Ah, bash is available, oh, we have\nBASH defined so let's keep going\" without doing an \"exec\", so later,\n\n> +. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n\nmay trigger errors like this:\n\n  sh: .../git-completion.bash: line 128: syntax error near unexpected token `<'\n  sh: .../git-completion.bash: line 128: `    done < <(git config -z --get...\n\ndepending on what bash-ism is used in the then-current version of that\nfile.\n\nSo we may want to be a bit more careful like this:\n\nif test \"z${BASH%/bash}\" != \"z${BASH}\"\nthen\n\t: ends with bash so we are already running bash as bash\nelif type bash >/dev/null 2>&1\nthen\n\texec bash \"$0\" \"$@\"\nelse\n\techo '1..0 #SKIP skipping bash completion tests; bash not available'\n\texit 0\nfi\n\nThis incidentally avoids running type twice when we are already running as\nbash.\n\n> +\n> +test_description='test bash completion'\n> +\n> +. ./test-lib.sh\n> +\n> +complete ()\n> +{\n> +\t# do nothing\n> +\treturn 0\n> +}\n> +\n> +. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n> +\n> +print_comp ()\n> +{\n> +\tlocal IFS=$'\\n'\n> +\techo \"${COMPREPLY[*]}\" > out\n> +}\n> +\n> +_get_comp_words_by_ref ()\n> +{\n> +\twhile [ $# -gt 0 ]; do\n> +\t\tcase \"$1\" in\n> +\t\tcur)\n> +\t\t\tcur=${_words[_cword]}\n> +\t\t\t;;\n> +\t\tprev)\n> +\t\t\tprev=${_words[_cword-1]}\n> +\t\t\t;;\n> +\t\twords)\n> +\t\t\twords=(\"${_words[@]}\")\n> +\t\t\t;;\n> +\t\tcword)\n> +\t\t\tcword=$_cword\n> +\t\t\t;;\n> +\t\tesac\n> +\t\tshift\n> +\tdone\n> +}\n\nCould you add an explanation before this function to describe what is\ngoing on?  The completion script being tested relies on the shell function\nof the same name defined there to behave one way, and this overrides it\nwith a different implementation.  Often such an override is an effective\nway to intercept and expose the internal state to test framework, but the\nabove does not seem to be such an override, and it is unclear what is\ngoing on here.\n"},{"id":"188759","messageId":"20120408044825.GA4296@sigill.intra.peff.net","threadId":"30182","inReplyTo":"7vpqbiyhmu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-08T04:48:25Z","receivedAt":"2012-04-08T04:48:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 07, 2012 at 09:25:29PM -0700, Junio C Hamano wrote:\n\n> Bash has a special case code to behave in a \"posixly correct\" mode, where\n> a lot of extensions are disabled, when invoked as \"sh\".  Unfortunately, it\n> still defines BASH_* variables, including BASH, when operating in this\n> mode.\n> [...]\n> So we may want to be a bit more careful like this:\n> \n> if test \"z${BASH%/bash}\" != \"z${BASH}\"\n> then\n> \t: ends with bash so we are already running bash as bash\n\nIf bash is in posix mode (including \"bash --posix\" and being invoked as\n\"bin/sh\"), it will set POSIXLY_CORRECT (but not export it). Similarly,\nif POSIXLY_CORRECT is set in the outer environment, it will act more\nlike sh. So maybe that would be a better test.\n\n-Peff\n"},{"id":"188760","messageId":"20120408050140.GA5862@sigill.intra.peff.net","threadId":"30182","inReplyTo":"1333854479-23260-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-08T05:01:40Z","receivedAt":"2012-04-08T05:01:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 08, 2012 at 06:07:48AM +0300, Felipe Contreras wrote:\n\n> +\tcat >expected <<-\\EOF &&\n> +\tfetch \n> +\tfilter-branch \n> +\tfilter-branch.sh \n> +\tformat-patch \n> +\tfsck \n> +\tEOF\n> +\ttest_completion \"git f\"\n\nThis test fails for me. The problem is that I have other git-f* programs\nin my PATH, and the completion finds and displays them. In other words,\nthe environment outside the test suite can pollute the result.\n\nI'm not sure of the right solution. We can't just sanitize the PATH in\ntest-lib.sh, since those git programs might be in /usr/bin or some other\ndirectory containing other commands necessary to run the test suite. We\ncould sanitize it temporarily just for the _git completion invocation,\nwhich consists only of builtins (and we know we're running under bash,\nso we can trust that things like \"test\" are builtins). But it still\nfeels horribly hacky.\n\nThat patch might look like this:\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 7eb80dd..713f4b1 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -58,8 +58,11 @@ test_completion ()\n \t_words=( $1 )\n \ttest $# -gt 1 && echo \"$2\" > expected\n \t(( _cword = ${#_words[@]} - 1 ))\n+\tsaved_path=$PATH\n+\tPATH=$MINIMAL_PATH\n \t_git && print_comp &&\n \ttest_cmp expected out\n+\tPATH=$saved_path\n }\n \n test_expect_success 'basic' '\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex b7d7100..348b68d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -466,12 +466,14 @@ then\n \tIFS=$OLDIFS\n \tPATH=$GIT_VALGRIND/bin:$PATH\n \tGIT_EXEC_PATH=$GIT_VALGRIND/bin\n+\tMINIMAL_PATH=$GIT_VALGRIND/bin\n \texport GIT_VALGRIND\n elif test -n \"$GIT_TEST_INSTALLED\" ; then\n \tGIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||\n \terror \"Cannot run git from $GIT_TEST_INSTALLED.\"\n \tPATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH\n \tGIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}\n+\tMINIMAL_PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR\n else # normal case, use ../bin-wrappers only unless $with_dashes:\n \tgit_bin_dir=\"$GIT_BUILD_DIR/bin-wrappers\"\n \tif ! test -x \"$git_bin_dir/git\" ; then\n@@ -482,8 +484,10 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:\n \tfi\n \tPATH=\"$git_bin_dir:$PATH\"\n \tGIT_EXEC_PATH=$GIT_BUILD_DIR\n+\tMINIMAL_PATH=$git_bin_dir\n \tif test -n \"$with_dashes\" ; then\n \t\tPATH=\"$GIT_BUILD_DIR:$PATH\"\n+\t\tMINIMAL_PATH=$MINIMAL_PATH:$GIT_BUILD_DIR\n \tfi\n fi\n GIT_TEMPLATE_DIR=\"$GIT_BUILD_DIR\"/templates/blt\n"},{"id":"188762","messageId":"7v62daye43.fsf@alter.siamese.dyndns.org","threadId":"30182","inReplyTo":"20120408044825.GA4296@sigill.intra.peff.net","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-08T05:41:32Z","receivedAt":"2012-04-08T05:41:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Apr 07, 2012 at 09:25:29PM -0700, Junio C Hamano wrote:\n>\n>> Bash has a special case code to behave in a \"posixly correct\" mode, where\n>> a lot of extensions are disabled, when invoked as \"sh\".  Unfortunately, it\n>> still defines BASH_* variables, including BASH, when operating in this\n>> mode.\n>> [...]\n>> So we may want to be a bit more careful like this:\n>> \n>> if test \"z${BASH%/bash}\" != \"z${BASH}\"\n>> then\n>> \t: ends with bash so we are already running bash as bash\n>\n> If bash is in posix mode (including \"bash --posix\" and being invoked as\n> \"bin/sh\"), it will set POSIXLY_CORRECT (but not export it). Similarly,\n> if POSIXLY_CORRECT is set in the outer environment, it will act more\n> like sh. So maybe that would be a better test.\n\nYes, but the check needs to be careful to make sure the shell that is\nrunning the check is indeed bash, so that it will explicitly exec bash for\nsomebody who is running dash but exports POSIXLY_CORRECT to make GNU\nprograms (other than bash) behave more standard compliant way.\n\nIn other words,\n\n\tif test -n \"$POSIXLY_CORRECT\" && test -n \"$BASH\"\n\tthen\n        \t: we are running bash under posix mode\n\telif ...\n\nor somesuch.\n"},{"id":"188763","messageId":"20120408054251.GA8100@sigill.intra.peff.net","threadId":"30182","inReplyTo":"7v62daye43.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-08T05:42:51Z","receivedAt":"2012-04-08T05:42:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 07, 2012 at 10:41:32PM -0700, Junio C Hamano wrote:\n\n> > If bash is in posix mode (including \"bash --posix\" and being invoked as\n> > \"bin/sh\"), it will set POSIXLY_CORRECT (but not export it). Similarly,\n> > if POSIXLY_CORRECT is set in the outer environment, it will act more\n> > like sh. So maybe that would be a better test.\n> \n> Yes, but the check needs to be careful to make sure the shell that is\n> running the check is indeed bash, so that it will explicitly exec bash for\n> somebody who is running dash but exports POSIXLY_CORRECT to make GNU\n> programs (other than bash) behave more standard compliant way.\n\nSorry, I thought that was obvious. Yes, this:\n\n> In other words,\n> \n> \tif test -n \"$POSIXLY_CORRECT\" && test -n \"$BASH\"\n> \tthen\n>         \t: we are running bash under posix mode\n> \telif ...\n> \n> or somesuch.\n\nis what I meant. Replace the \"does it end in /bash\" bit with\n\"POSIXLY_CORRECT\" but, keep the $BASH check.\n\n-Peff\n"},{"id":"188767","messageId":"20120408081245.GA18407@sigill.intra.peff.net","threadId":"30182","inReplyTo":"20120408054251.GA8100@sigill.intra.peff.net","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-08T08:12:45Z","receivedAt":"2012-04-08T08:12:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 08, 2012 at 01:42:51AM -0400, Jeff King wrote:\n\n> > Yes, but the check needs to be careful to make sure the shell that is\n> > running the check is indeed bash, so that it will explicitly exec bash for\n> > somebody who is running dash but exports POSIXLY_CORRECT to make GNU\n> > programs (other than bash) behave more standard compliant way.\n> \n> Sorry, I thought that was obvious. Yes, this:\n> \n> > In other words,\n> > \n> > \tif test -n \"$POSIXLY_CORRECT\" && test -n \"$BASH\"\n> > \tthen\n> >         \t: we are running bash under posix mode\n> > \telif ...\n> > \n> > or somesuch.\n> \n> is what I meant. Replace the \"does it end in /bash\" bit with\n> \"POSIXLY_CORRECT\" but, keep the $BASH check.\n\nBTW, if you intend to exec bash when we see a posix bash, I think you\nwould want to unset POSIXLY_CORRECT in case it is exported. Otherwise,\ndoing this:\n\n POSIXLY_CORRECT=1 bash ./t9902-*\n\nwould loop forever, trying to restart bash, but ending up in the POSIX\nversion each time. So all together:\n\n  if test -n \"$BASH\" && test -z \"$POSIXLY_CORRECT\"; then\n          : we are in full-on bash mode. Great.\n  elif type bash >/dev/null 2>&1; then\n          # We are not bash, or are in posix mode. Run a new bash,\n          # making sure not to let any posix environment variable\n          # propagate.\n          unset POSIXLY_CORRECT\n          exec bash \"$0\" \"$@\"\n  else\n          echo '1..0 # SKIP skipping bash completion tests; bash not available'\n          exit 0\n  fi\n\n-Peff\n\nPS I wondered if we can continue past the \"exec\" in the second branch if\n   it fails, and if we should be exiting explicitly. POSIX specifies\n   that \"exec\" with a command never returns, and that is what dash does.\n   Bash will continue after a failed exec. I expected it not to do so\n   in posix mode, but it seems to. Which is perhaps a bug in bash, but\n   one we might want to deal with. Granted, the likelihood of \"type\"\n   succeeding but the exec failing is low, so it may not be worth caring\n   about.\n"},{"id":"188768","messageId":"m27gxqk2vs.fsf@igel.home","threadId":"30182","inReplyTo":"20120408081245.GA18407@sigill.intra.peff.net","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-08T09:07:51Z","receivedAt":"2012-04-08T09:07:51Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>    Bash will continue after a failed exec.\n\nOnly when interactive.  A non-interactive shell respects the execfail\nshopt (off by default).\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"188769","messageId":"20120408102230.GA22648@river.lan","threadId":"30182","inReplyTo":"1333854479-23260-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 05/12] completion: add missing global options","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2012-04-08T10:22:41Z","receivedAt":"2012-04-08T10:22:41Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Apr 08, 2012 at 06:07:52AM +0300, Felipe Contreras wrote:\n> @@ -2621,7 +2621,10 @@ _git ()\n>  \t\tcase \"$i\" in\n>  \t\t--git-dir=*) __git_dir=\"${i#--git-dir=}\" ;;\n>  \t\t--bare)      __git_dir=\".\" ;;\n> -\t\t--version|-p|--paginate) ;;\n> +\t\t--version) ;;\n> +\t\t-p|--paginate|--no-pager) ;;\n> +\t\t--exec-path=*|--html-path|--info-path) ;;\n\nSince exec-path's argument is optional, shouldn't this be:\n\n  +\t\t--exec-path*|--html-path|--info-path) ;;\n\n> +\t\t--work-tree=*|--namespace=*|--no-replace-objects) ;;\n\nIt would be nice to accept options with argument with and without the\nequals sign, as the git command does, so both:\n    git --git-dir=/foo/bar\nand\n    git --git-dir /foo/bar\n\nI guess this looks like (adding in -c as well):\n\n\t\t--git-dir)  ((c++)) ; __git_dir=\"${words[c]}\" ;;\n                --work-tree|--namespacei|-c) ((c++)) ;;\n\n\n>  \t\t--help) command=\"help\"; break ;;\n>  \t\t*) command=\"$i\"; break ;;\n>  \t\tesac\n> @@ -2636,10 +2639,12 @@ _git ()\n>  \t\t\t--git-dir=\n>  \t\t\t--bare\n>  \t\t\t--version\n> -\t\t\t--exec-path\n> +\t\t\t--exec-path=\n\nI think this change is incorrect since giving --exec-path without an\nargument is perfectly fine and has a defined meaning.\n\n>  \t\t\t--html-path\n> +\t\t\t--info-path\n>  \t\t\t--work-tree=\n>  \t\t\t--namespace=\n> +\t\t\t--no-replace-objects\n>  \t\t\t--help\n\nShould \"-c\" be in this list as well?\n\n>  \t\t\t\"\n>  \t\t\t;;\n\n-- \nJohn\n"},{"id":"188770","messageId":"20120408102856.GB22648@river.lan","threadId":"30182","inReplyTo":"1333854479-23260-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2012-04-08T10:28:56Z","receivedAt":"2012-04-08T10:28:56Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Apr 08, 2012 at 06:07:48AM +0300, Felipe Contreras wrote:\n> +test_completion ()\n> +{\n> +\tlocal -a COMPREPLY _words\n> +\tlocal _cword\n> +\t_words=( $1 )\n> +\ttest $# -gt 1 && echo \"$2\" > expected\n> +\t(( _cword = ${#_words[@]} - 1 ))\n> +\t_git && print_comp &&\n> +\ttest_cmp expected out\n> +}\n\nCould you check that _git doesn't output anything to stderr here?  I\ncan't see any reason why a completion command should ever do so\nlegitimately and it's generally annoying if one does.\n\n-- \nJohn\n"},{"id":"188771","messageId":"20120408103002.GA3177@burratino","threadId":"30182","inReplyTo":"20120408050140.GA5862@sigill.intra.peff.net","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-08T10:30:02Z","receivedAt":"2012-04-08T10:30:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(dropping Shawn from cc list since he hasn't been working on the\n completion script for a couple of years now)\nJeff King wrote:\n> On Sun, Apr 08, 2012 at 06:07:48AM +0300, Felipe Contreras wrote:\n\n>> +\tcat >expected <<-\\EOF &&\n>> +\tfetch \n>> +\tfilter-branch \n>> +\tfilter-branch.sh \n>> +\tformat-patch \n>> +\tfsck \n>> +\tEOF\n>> +\ttest_completion \"git f\"\n>\n> This test fails for me. The problem is that I have other git-f* programs\n> in my PATH\n[...]\n>                                     We can't just sanitize the PATH in\n> test-lib.sh, since those git programs might be in /usr/bin or some other\n> directory containing other commands necessary to run the test suite. We\n> could sanitize it temporarily just for the _git completion invocation,\n> which consists only of builtins (and we know we're running under bash,\n> so we can trust that things like \"test\" are builtins). But it still\n> feels horribly hacky.\n\nNot to mention that it would break the TEST_INSTALLED_GIT facility.\n\nIs there some reason we care about the exact list of completions for\n\"git f\"?  The above also looks a little bogus because if someone were\nto remove the +x bit on the git-filter-branch.sh source file or ensure\nit is not on the PATH when tests are run (I have no particular opinion\nabout whether that's a good idea; it's just an example) then this\nwould change the actual output.  That doesn't seem particularly\nimportant to check for when talking about how the tab completion\nbehaves on the installed system.\n\nMaybe it would make sense to check a few representative items.\n\n\t# builtin\n\tgrep \"^fetch\\$\" out &&\n\n\t# builtin whose implementation is not in builtin/cmd.c\n\tgrep \"^format-patch\\$\" out &&\n\n\t# script\n\tgrep \"^filter-branch\\$\" out &&\n\n\t# plumbing\n\t! grep \"^fsck-objects\\$\" out &&\n\n\tsort out >out.sorted &&\n\ttest_cmp out.sorted out\n"},{"id":"188772","messageId":"20120408110451.GA21217@sigill.intra.peff.net","threadId":"30182","inReplyTo":"m27gxqk2vs.fsf@igel.home","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-08T11:04:51Z","receivedAt":"2012-04-08T11:04:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 08, 2012 at 11:07:51AM +0200, Andreas Schwab wrote:\n\n> >    Bash will continue after a failed exec.\n> \n> Only when interactive.  A non-interactive shell respects the execfail\n> shopt (off by default).\n\nThanks, I didn't know that. Let's not worry about error-handling after\nthe exec then.\n\n-Peff\n"},{"id":"188773","messageId":"20120408110642.GB21217@sigill.intra.peff.net","threadId":"30182","inReplyTo":"20120408103002.GA3177@burratino","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-08T11:06:42Z","receivedAt":"2012-04-08T11:06:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 08, 2012 at 05:30:02AM -0500, Jonathan Nieder wrote:\n\n> > This test fails for me. The problem is that I have other git-f* programs\n> > in my PATH\n> [...]\n> Is there some reason we care about the exact list of completions for\n> \"git f\"?\n> [...]\n> Maybe it would make sense to check a few representative items.\n> \n> \t# builtin\n> \tgrep \"^fetch\\$\" out &&\n\nI think your approach (to stop worrying about stopping pollution from\nthe environment, and start being more specific in what the tests care\nabout) is much saner than the snippet I posted.\n\n-Peff\n"},{"id":"188774","messageId":"20120408123646.GM2289@goldbirke","threadId":"30182","inReplyTo":"1333854479-23260-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 05/12] completion: add missing global options","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-04-08T12:36:46Z","receivedAt":"2012-04-08T12:36:46Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Sun, Apr 08, 2012 at 06:07:52AM +0300, Felipe Contreras wrote:\n> Otherwise 'git --foo bar<tab>' fails, also, other options are completely\n> missing.\n> \n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  contrib/completion/git-completion.bash |    9 +++++++--\n>  t/t9902-completion.sh                  |   31 +++++++++++++++++++++++++++++++\n>  2 files changed, 38 insertions(+), 2 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 3bc8d85..c9672b2 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2621,7 +2621,10 @@ _git ()\n>  \t\tcase \"$i\" in\n>  \t\t--git-dir=*) __git_dir=\"${i#--git-dir=}\" ;;\n>  \t\t--bare)      __git_dir=\".\" ;;\n> -\t\t--version|-p|--paginate) ;;\n> +\t\t--version) ;;\n> +\t\t-p|--paginate|--no-pager) ;;\n> +\t\t--exec-path=*|--html-path|--info-path) ;;\n> +\t\t--work-tree=*|--namespace=*|--no-replace-objects) ;;\n>  \t\t--help) command=\"help\"; break ;;\n>  \t\t*) command=\"$i\"; break ;;\n>  \t\tesac\n\nThis is not future proof, it will break again when we add yet another\nglobal option.  This is also incomplete, because the '-c\n<configvar=value>' option is missing.\n\nI send a patch to fix this issue a couple of months ago but it got\ndropped on the floor.\n\n  http://thread.gmane.org/gmane.comp.version-control.git/180650\n\nMy patch handles the -c global option and it won't break when we add\na new global option, so I think it should replace the above hunk.\n\nPlease feel free to copy the commit message, too, for the benefit of\nother developers who might read the log message a year later.\n\n\n> +test_expect_success 'general options plus command' '\n> +\ttest_completion \"git --version check\" \"checkout \" &&\n> +\ttest_completion \"git --paginate check\" \"checkout \" &&\n> +\ttest_completion \"git --git-dir=foo check\" \"checkout \" &&\n> +\ttest_completion \"git --bare check\" \"checkout \" &&\n> +\ttest_completion \"git --help des\" \"describe \" &&\n> +\ttest_completion \"git --exec-path=foo check\" \"checkout \" &&\n> +\ttest_completion \"git --html-path check\" \"checkout \" &&\n> +\ttest_completion \"git --no-pager check\" \"checkout \" &&\n> +\ttest_completion \"git --work-tree=foo check\" \"checkout \" &&\n> +\ttest_completion \"git --namespace=foo check\" \"checkout \" &&\n> +\ttest_completion \"git --paginate check\" \"checkout \" &&\n> +\ttest_completion \"git --info-path check\" \"checkout \" &&\n> +\ttest_completion \"git --no-replace-objects check\" \"checkout \"\n> +'\n\nThen you could add a \n\n\ttest_completion \"git -c 'name=value' check\" \"checkout \"\n\ntest, too.\n\n\nBest,\nGábor\n"},{"id":"188775","messageId":"20120408124652.GN2289@goldbirke","threadId":"30182","inReplyTo":"1333854479-23260-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-04-08T12:46:52Z","receivedAt":"2012-04-08T12:46:52Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Sun, Apr 08, 2012 at 06:07:49AM +0300, Felipe Contreras wrote:\n> The suffix is never used.\n\nThe suffix was used to offer completion after 'git config\nbranch.<TAB>' ending with a period.  It is not used by git's\ncompletion script anymore, because nowadays this case is handled by\nthe __gitcomp_nl helper() function.\n\nHowever, users can provide custom completion functions to their\naliases or additional git commands, and there might be a custom\ncompletion function out there that passes a suffix to __gitcomp().\n\nWe don't want to break those custom completion functions, so I think\nthe suffix should stay.\n\n\nBest,\nGábor\n\n\n\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  contrib/completion/git-completion.bash |   12 +++++-------\n>  1 file changed, 5 insertions(+), 7 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 31f714d..39f5435 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -304,15 +304,14 @@ __git_ps1 ()\n>  \tfi\n>  }\n>  \n> -# __gitcomp_1 requires 2 arguments\n>  __gitcomp_1 ()\n>  {\n>  \tlocal c IFS=' '$'\\t'$'\\n'\n>  \tfor c in $1; do\n> -\t\tcase \"$c$2\" in\n> -\t\t--*=*) printf %s$'\\n' \"$c$2\" ;;\n> -\t\t*.)    printf %s$'\\n' \"$c$2\" ;;\n> -\t\t*)     printf %s$'\\n' \"$c$2 \" ;;\n> +\t\tcase \"$c\" in\n> +\t\t--*=*) printf %s$'\\n' \"$c\" ;;\n> +\t\t*.)    printf %s$'\\n' \"$c\" ;;\n> +\t\t*)     printf %s$'\\n' \"$c \" ;;\n>  \t\tesac\n>  \tdone\n>  }\n> @@ -479,7 +478,6 @@ fi\n>  # 1: List of possible completion words.\n>  # 2: A prefix to be added to each possible completion word (optional).\n>  # 3: Generate possible completion matches for this word (optional).\n> -# 4: A suffix to be appended to each possible completion word (optional).\n>  __gitcomp ()\n>  {\n>  \tlocal cur_=\"${3-$cur}\"\n> @@ -491,7 +489,7 @@ __gitcomp ()\n>  \t*)\n>  \t\tlocal IFS=$'\\n'\n>  \t\tCOMPREPLY=($(compgen -P \"${2-}\" \\\n> -\t\t\t-W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n> +\t\t\t-W \"$(__gitcomp_1 \"${1-}\")\" \\\n>  \t\t\t-- \"$cur_\"))\n>  \t\t;;\n>  \tesac\n> -- \n> 1.7.10.3.g5a738d\n> \n"},{"id":"188777","messageId":"CAMP44s0nSH7ZjC4T=7pXS2MyAkZ133hOXa_tn3q4er+FbcOc_w@mail.gmail.com","threadId":"30182","inReplyTo":"20120408124652.GN2289@goldbirke","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T13:07:47Z","receivedAt":"2012-04-08T13:07:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"2012/4/8 SZEDER Gábor <szeder@ira.uka.de>:\n> On Sun, Apr 08, 2012 at 06:07:49AM +0300, Felipe Contreras wrote:\n>> The suffix is never used.\n>\n> The suffix was used to offer completion after 'git config\n> branch.<TAB>' ending with a period.  It is not used by git's\n> completion script anymore, because nowadays this case is handled by\n> the __gitcomp_nl helper() function.\n>\n> However, users can provide custom completion functions to their\n> aliases or additional git commands, and there might be a custom\n> completion function out there that passes a suffix to __gitcomp().\n\nYes, there _might_ be, and they would have to be fixed; APIs change.\nHowever, this is highly theoretical, what suffix could anybody\npossibly want? I believe __gitcomp_1 already does the job of adding\nsensible suffixes, and there's nothing else anybody else could\npossibly want.\n\nIn any case, shouldn't compgen -S \"${4-}\" do the trick?\n\n> We don't want to break those custom completion functions, so I think\n> the suffix should stay.\n\nSo we should never make any cleanups because some custom completion\nfunctions might theoretically break? I think at some point we need to\naccept that nobody is using this argument anyway (and they should).\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"188779","messageId":"20120408132704.GB4103@burratino","threadId":"30182","inReplyTo":"CAMP44s0nSH7ZjC4T=7pXS2MyAkZ133hOXa_tn3q4er+FbcOc_w@mail.gmail.com","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-08T13:27:04Z","receivedAt":"2012-04-08T13:27:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Felipe Contreras wrote:\n\n> So we should never make any cleanups because some custom completion\n> functions might theoretically break?\n\nNo, tasteful cleanups that don't break custom completion functions are\nvery welcome.\n\nAlso if you have evidence that this is definitely (not just possibly)\nonly a theoretical problem, then it would not be a regression.  But I\nactually find it likely that custom completion code would have\nimitated the old completion code in git that used the suffix argument,\nso...\n\nCan you remind me what the benefit that the user is getting from this\npatch in exchange for us breaking their tab completion?  The title\n(\"simplify __gitcomp\") tells me exactly nothing about its impact\nexcept that it is perhaps supposed to result in no behavior change.\n\nSorry for the fuss and hope that helps,\nJonathan\n"},{"id":"188780","messageId":"CAMP44s3c2SNSh=TGhYv=JKGu4QJfPB_xoBip5wwLoDh3dLHYNw@mail.gmail.com","threadId":"30182","inReplyTo":"20120408132704.GB4103@burratino","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T13:47:04Z","receivedAt":"2012-04-08T13:47:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 8, 2012 at 4:27 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Felipe Contreras wrote:\n>\n>> So we should never make any cleanups because some custom completion\n>> functions might theoretically break?\n>\n> No, tasteful cleanups that don't break custom completion functions are\n> very welcome.\n>\n> Also if you have evidence that this is definitely (not just possibly)\n> only a theoretical problem, then it would not be a regression.  But I\n> actually find it likely that custom completion code would have\n> imitated the old completion code in git that used the suffix argument,\n> so...\n\nShow me an example. If you do something like:\n\n __gitcomp \"foo bar opt=\" \"\" \"\" \"suffix\"\n\nThe result would be 'foo suffix' 'bar suffix' 'opt=suffix'. How could\nthat *possibly* be useful?\n\n> Can you remind me what the benefit that the user is getting from this\n> patch\n\nAbsolutely none.\n\n> in exchange for us breaking their tab completion?\n\nWhoa! Breaking their tab completion? Where? Can you show me some evidence?\n\n>  The title\n> (\"simplify __gitcomp\") tells me exactly nothing about its impact\n> except that it is perhaps supposed to result in no behavior change.\n\nThat is correct. No functional changes. I would need to update the\ntests if something changed, right?\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"293734","messageId":"CAMP44s2vdEzn8_N=sauq-6OvqA+4tDtM9+yYYWuiQHgkZ2DyYA@mail.gmail.com","threadId":"30182","inReplyTo":"20120408132704.GB4103@burratino","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T14:11:31Z","receivedAt":"2012-04-08T14:11:31Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nI've looked at the history of __gitcomp and __gitcomp_nl, now I think\nit makes sense to keep their arguments similar, at least for the time\nbeing.\n\nSo I suggest this patch to be dropped.\n\nOn Sun, Apr 8, 2012 at 4:27 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Felipe Contreras wrote:\n>\n>> So we should never make any cleanups because some custom completion\n>> functions might theoretically break?\n>\n> No, tasteful cleanups that don't break custom completion functions are\n> very welcome.\n>\n> Also if you have evidence that this is definitely (not just possibly)\n> only a theoretical problem, then it would not be a regression.  But I\n> actually find it likely that custom completion code would have\n> imitated the old completion code in git that used the suffix argument,\n> so...\n>\n> Can you remind me what the benefit that the user is getting from this\n> patch in exchange for us breaking their tab completion?  The title\n> (\"simplify __gitcomp\") tells me exactly nothing about its impact\n> except that it is perhaps supposed to result in no behavior change.\n>\n> Sorry for the fuss and hope that helps,\n> Jonathan\n\n\n\n-- \n"},{"id":"188781","messageId":"20120408143621.GA2631@burratino","threadId":"30182","inReplyTo":"CAMP44s3c2SNSh=TGhYv=JKGu4QJfPB_xoBip5wwLoDh3dLHYNw@mail.gmail.com","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-08T14:36:21Z","receivedAt":"2012-04-08T14:36:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(dropping Shawn from cc since I'm not under the impression he works\n on the completion script these days)\nFelipe Contreras wrote:\n\n> Whoa! Breaking their tab completion? Where? Can you show me some evidence?\n\nIf you weren't listening before, I'm not sure what I can add now[*].\n\nLuckily, I already said what I needed to say.  Yes, cleanups can be\ngood when they don't break things, and no, cleanups that break things\nare not good.  Sometimes it is not obvious which category each case\nfalls into.  As you well know, \"tests pass\" is not enough (e.g.,\nsometimes there are no tests!).  If you want someone to argue with,\nyou can find someone else.\n\nSorry,\nJonathan\n\n[*] To be extra, extra clear: I never said your patch breaks people's\ncustom tab completion scripts.  I said it might do so and that that is\nnot very comforting to apply the patch when no one seems to have\nthought carefully about how to investigate that and mitigate the\ndamage if there is any.\n"},{"id":"188782","messageId":"20120408143912.GB2631@burratino","threadId":"30182","inReplyTo":"CAMP44s2vdEzn8_N=sauq-6OvqA+4tDtM9+yYYWuiQHgkZ2DyYA@mail.gmail.com","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-08T14:39:12Z","receivedAt":"2012-04-08T14:39:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(-cc: Shawn)\nFelipe Contreras wrote:\n\n> I've looked at the history of __gitcomp and __gitcomp_nl, now I think\n> it makes sense to keep their arguments similar, at least for the time\n> being.\n>\n> So I suggest this patch to be dropped.\n\nThanks for looking into it, and sorry I responded to a heated response\ninstead of waiting for a calmer one.\n\nI do encourage you to examine your communication style.\n\nCheers,\nJonathan\n"},{"id":"188783","messageId":"CAMP44s1hmG_xMjeB_Wv8pU=aPt=QK27R1OsePeeH5TK9w2GCqg@mail.gmail.com","threadId":"30182","inReplyTo":"20120408143621.GA2631@burratino","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-08T14:58:08Z","receivedAt":"2012-04-08T14:58:08Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 8, 2012 at 5:36 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> (dropping Shawn from cc since I'm not under the impression he works\n>  on the completion script these days)\n> Felipe Contreras wrote:\n>\n>> Whoa! Breaking their tab completion? Where? Can you show me some evidence?\n>\n> If you weren't listening before, I'm not sure what I can add now[*].\n\nThis is what you said; \"in exchange for us breaking their tab\ncompletion\". There's a big difference between \"breaking\", and\n\"*potentially* breaking\". I have never seen any evidence of tab\ncompletion actually being broken.\n\n> Luckily, I already said what I needed to say.  Yes, cleanups can be\n> good when they don't break things, and no, cleanups that break things\n> are not good.  Sometimes it is not obvious which category each case\n> falls into.  As you well know, \"tests pass\" is not enough (e.g.,\n> sometimes there are no tests!).  If you want someone to argue with,\n> you can find someone else.\n\nFortunately there's no evidence of anything being broken; all the\nissues mentioned are *theoretical*, and there's not even an example of\na custom completion that would be broken by this.\n\n-- \nFelipe Contreras\n"},{"id":"188800","messageId":"7vsjgcwyrt.fsf@alter.siamese.dyndns.org","threadId":"30182","inReplyTo":"20120408143912.GB2631@burratino","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-09T18:22:46Z","receivedAt":"2012-04-09T18:22:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> (-cc: Shawn)\n> Felipe Contreras wrote:\n>\n>> I've looked at the history of __gitcomp and __gitcomp_nl, now I think\n>> it makes sense to keep their arguments similar, at least for the time\n>> being.\n>>\n>> So I suggest this patch to be dropped.\n>\n> Thanks for looking into it, and sorry I responded to a heated response\n> instead of waiting for a calmer one.\n\nGood to see a civilized discussion of patches on technical merits.\n\nThanks.\n"},{"id":"188808","messageId":"4F83311B.50901@kdbg.org","threadId":"30182","inReplyTo":"CAMP44s1hmG_xMjeB_Wv8pU=aPt=QK27R1OsePeeH5TK9w2GCqg@mail.gmail.com","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-04-09T18:57:31Z","receivedAt":"2012-04-09T18:57:31Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.04.2012 16:58, schrieb Felipe Contreras:\n> On Sun, Apr 8, 2012 at 5:36 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>> (dropping Shawn from cc since I'm not under the impression he works\n>>  on the completion script these days)\n>> Felipe Contreras wrote:\n>>\n>>> Whoa! Breaking their tab completion? Where? Can you show me some evidence?\n>>\n>> If you weren't listening before, I'm not sure what I can add now[*].\n> \n> This is what you said; \"in exchange for us breaking their tab\n> completion\". There's a big difference between \"breaking\", and\n> \"*potentially* breaking\". I have never seen any evidence of tab\n> completion actually being broken.\n\nI have:\n\nhttp://thread.gmane.org/gmane.comp.version-control.msysgit/13310/focus=13335\nhttp://thread.gmane.org/gmane.comp.version-control.git/185184/focus=185189\n\n-- Hannes\n"},{"id":"188811","messageId":"CAMP44s1P2sSWDa9nT-LO+EmFvsMQs3yogO+Q0o460FH_r3b=nA@mail.gmail.com","threadId":"30182","inReplyTo":"4F83311B.50901@kdbg.org","subject":"Re: [PATCH 02/12] completion: simplify __gitcomp","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-09T19:12:54Z","receivedAt":"2012-04-09T19:12:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Apr 9, 2012 at 9:57 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 08.04.2012 16:58, schrieb Felipe Contreras:\n>> On Sun, Apr 8, 2012 at 5:36 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>>> (dropping Shawn from cc since I'm not under the impression he works\n>>>  on the completion script these days)\n>>> Felipe Contreras wrote:\n>>>\n>>>> Whoa! Breaking their tab completion? Where? Can you show me some evidence?\n>>>\n>>> If you weren't listening before, I'm not sure what I can add now[*].\n>>\n>> This is what you said; \"in exchange for us breaking their tab\n>> completion\". There's a big difference between \"breaking\", and\n>> \"*potentially* breaking\". I have never seen any evidence of tab\n>> completion actually being broken.\n>\n> I have:\n>\n> http://thread.gmane.org/gmane.comp.version-control.msysgit/13310/focus=13335\n> http://thread.gmane.org/gmane.comp.version-control.git/185184/focus=185189\n\nI meant being broken by this patch.\n\nPlus this patch series actually fixes those issues.\n\n-- \nFelipe Contreras\n"},{"id":"188813","messageId":"7vpqbgvfsa.fsf@alter.siamese.dyndns.org","threadId":"30182","inReplyTo":"20120408110642.GB21217@sigill.intra.peff.net","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-09T19:58:13Z","receivedAt":"2012-04-09T19:58:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Apr 08, 2012 at 05:30:02AM -0500, Jonathan Nieder wrote:\n>\n>> > This test fails for me. The problem is that I have other git-f* programs\n>> > in my PATH\n>> [...]\n>> Is there some reason we care about the exact list of completions for\n>> \"git f\"?\n>> [...]\n>> Maybe it would make sense to check a few representative items.\n>> \n>> \t# builtin\n>> \tgrep \"^fetch\\$\" out &&\n>\n> I think your approach (to stop worrying about stopping pollution from\n> the environment, and start being more specific in what the tests care\n> about) is much saner than the snippet I posted.\n\nI have to agree with that; it might give unexpeceted results if the user\nhas a $HOME/bin/git-update-index or something, though.\n"},{"id":"189050","messageId":"CAMP44s2EunUabZdU23e+jYnBrWtBmhZeqt9yR+wV3X9HZhd7hA@mail.gmail.com","threadId":"30182","inReplyTo":"7vpqbiyhmu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-11T21:59:38Z","receivedAt":"2012-04-11T21:59:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 8, 2012 at 7:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  t/t9902-completion.sh |  201 +++++++++++++++++++++++++++++++++++++++++++++++++\n>>  1 file changed, 201 insertions(+)\n>>  create mode 100755 t/t9902-completion.sh\n>>\n>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>> new file mode 100755\n>> index 0000000..7eb80dd\n>> --- /dev/null\n>> +++ b/t/t9902-completion.sh\n>> @@ -0,0 +1,201 @@\n>> +#!/bin/sh\n>> +#\n>> +# Copyright (c) 2012 Felipe Contreras\n>> +#\n>> +\n>> +if type bash >/dev/null 2>&1\n>> +then\n>> +     # execute inside bash\n>> +     test -z \"$BASH\" && exec bash \"$0\" \"$@\"\n>> +else\n>> +     echo '1..0 #SKIP skipping bash completion tests; bash not available'\n>> +     exit 0\n>> +fi\n>\n> What shell do you use on your system as /bin/sh (or if you use SHELL_PATH\n> in the Makefile to override it, what do you use)?\n\n/bin/sh is a symlink to bash on my system, and although bash defines\nPOSIXLY_CORRECT, the tests pass just fine. I even tried to run /bin/sh\n--posix.\n\n-- \nFelipe Contreras\n"},{"id":"189054","messageId":"20120411221430.GT2289@goldbirke","threadId":"30182","inReplyTo":"1333854479-23260-8-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 07/12] completion: simplify command stuff","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-04-11T22:14:30Z","receivedAt":"2012-04-11T22:14:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Sun, Apr 08, 2012 at 06:07:54AM +0300, Felipe Contreras wrote:\n> No need to recalculate it.\n\nBased on this short description I would expect that this \"command\nstuff\" is calculated somewhere, and a helper function is changed to\nuse the already calculated value ...\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index c9672b2..1fe11f4 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -721,7 +721,7 @@ __git_complete_revlist ()\n>  \n>  __git_complete_remote_or_refspec ()\n>  {\n> -\tlocal cur_=\"$cur\" cmd=\"${words[1]}\"\n> +\tlocal cur_=\"$cur\"\n>  \tlocal i c=2 remote=\"\" pfx=\"\" lhs=1 no_complete_refspec=0\n>  \tif [ \"$cmd\" = \"remote\" ]; then\n>  \t\t((c++))\n> @@ -2599,7 +2599,7 @@ _git_whatchanged ()\n>  \n>  _git ()\n>  {\n> -\tlocal i c=1 command __git_dir\n> +\tlocal i c=1 cmd __git_dir\n>  \n>  \tif [[ -n ${ZSH_VERSION-} ]]; then\n>  \t\temulate -L bash\n> @@ -2625,13 +2625,13 @@ _git ()\n>  \t\t-p|--paginate|--no-pager) ;;\n>  \t\t--exec-path=*|--html-path|--info-path) ;;\n>  \t\t--work-tree=*|--namespace=*|--no-replace-objects) ;;\n> -\t\t--help) command=\"help\"; break ;;\n> -\t\t*) command=\"$i\"; break ;;\n> +\t\t--help) cmd=\"help\"; break ;;\n> +\t\t*) cmd=\"$i\"; break ;;\n>  \t\tesac\n>  \t\t((c++))\n>  \tdone\n>  \n> -\tif [ -z \"$command\" ]; then\n> +\tif [ -z \"$cmd\" ]; then\n>  \t\tcase \"$cur\" in\n>  \t\t--*)   __gitcomp \"\n>  \t\t\t--paginate\n> @@ -2654,10 +2654,10 @@ _git ()\n>  \t\treturn\n>  \tfi\n>  \n> -\tlocal completion_func=\"_git_${command//-/_}\"\n> +\tlocal completion_func=\"_git_${cmd//-/_}\"\n>  \tdeclare -f $completion_func >/dev/null && $completion_func && return\n>  \n> -\tlocal expansion=$(__git_aliased_command \"$command\")\n> +\tlocal expansion=$(__git_aliased_command \"$cmd\")\n>  \tif [ -n \"$expansion\" ]; then\n>  \t\tcompletion_func=\"_git_${expansion//-/_}\"\n>  \t\tdeclare -f $completion_func >/dev/null && $completion_func\n> -- \n> 1.7.10.3.g5a738d\n> \n\n... but the bulk of this change is in the top-level _git() function,\nas it renames a variable there to match the variable name in the\nhelper function.  Perhaps it should be the other way around.\n\nHowever, this change \"promotes\" the command (or cmd) variable from\nbeing just a variable in _git() to a variable that is used in other\ncompletion functions, too.  Why not go one more step further, and\ndenote this by adding a __git prefix (i.e. renaming the variable to\n__git_command)?\n\n\nBest,\nGábor\n"},{"id":"189055","messageId":"CAMP44s3hYHxs2HZ59meEO=SU7gXuxU=m489LdxV38=7PDeDFdw@mail.gmail.com","threadId":"30182","inReplyTo":"20120411221430.GT2289@goldbirke","subject":"Re: [PATCH 07/12] completion: simplify command stuff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-11T22:21:17Z","receivedAt":"2012-04-11T22:21:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"2012/4/12 SZEDER Gábor <szeder@ira.uka.de>:\n> Hi,\n>\n>\n> On Sun, Apr 08, 2012 at 06:07:54AM +0300, Felipe Contreras wrote:\n>> No need to recalculate it.\n>\n> Based on this short description I would expect that this \"command\n> stuff\" is calculated somewhere, and a helper function is changed to\n> use the already calculated value ...\n\n[...]\n\n> ... but the bulk of this change is in the top-level _git() function,\n> as it renames a variable there to match the variable name in the\n> helper function.  Perhaps it should be the other way around.\n>\n> However, this change \"promotes\" the command (or cmd) variable from\n> being just a variable in _git() to a variable that is used in other\n> completion functions, too.  Why not go one more step further, and\n> denote this by adding a __git prefix (i.e. renaming the variable to\n> __git_command)?\n\nAfter thinking more about it, and analyzing the next patches, I don't\nthink it makes sense to have such a variable; it's only used by\n__git_complete_remote_or_refspec, and it would be easier to make it an\nargument to that function; that would simplify this patch series a\nlot.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"189057","messageId":"20120411225046.GU2289@goldbirke","threadId":"30182","inReplyTo":"1333854479-23260-11-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 10/12] completion: add new git_complete helper","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-04-11T22:50:46Z","receivedAt":"2012-04-11T22:50:46Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Sun, Apr 08, 2012 at 06:07:57AM +0300, Felipe Contreras wrote:\n> This simplifies the completions, and makes it easier to define aliases:\n> \n>  git_complete gf git_fetch\n> \n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  contrib/completion/git-completion.bash |   69 +++++++++++++++-----------------\n>  t/t9902-completion.sh                  |    2 +-\n>  2 files changed, 33 insertions(+), 38 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 60ea224..6cf1d98 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2616,21 +2616,6 @@ _git ()\n>  {\n>  \tlocal i c=1 cmd cmd_pos __git_dir\n>  \n> -\tif [[ -n ${ZSH_VERSION-} ]]; then\n> -\t\temulate -L bash\n> -\t\tsetopt KSH_TYPESET\n> -\n> -\t\t# workaround zsh's bug that leaves 'words' as a special\n> -\t\t# variable in versions < 4.3.12\n> -\t\ttypeset -h words\n> -\n> -\t\t# workaround zsh's bug that quotes spaces in the COMPREPLY\n> -\t\t# array if IFS doesn't contain spaces.\n> -\t\ttypeset -h IFS\n> -\tfi\n> -\n> -\tlocal cur words cword prev\n> -\t_get_comp_words_by_ref -n =: cur words cword prev\n>  \twhile [ $c -lt $cword ]; do\n>  \t\ti=\"${words[c]}\"\n>  \t\tcase \"$i\" in\n> @@ -2683,22 +2668,6 @@ _git ()\n>  \n>  _gitk ()\n>  {\n> -\tif [[ -n ${ZSH_VERSION-} ]]; then\n> -\t\temulate -L bash\n> -\t\tsetopt KSH_TYPESET\n> -\n> -\t\t# workaround zsh's bug that leaves 'words' as a special\n> -\t\t# variable in versions < 4.3.12\n> -\t\ttypeset -h words\n> -\n> -\t\t# workaround zsh's bug that quotes spaces in the COMPREPLY\n> -\t\t# array if IFS doesn't contain spaces.\n> -\t\ttypeset -h IFS\n> -\tfi\n> -\n> -\tlocal cur words cword prev\n> -\t_get_comp_words_by_ref -n =: cur words cword prev\n> -\n\nEliminating code duplication, great!\n\n>  \t__git_has_doubledash && return\n>  \n>  \tlocal g=\"$(__gitdir)\"\n> @@ -2719,16 +2688,42 @@ _gitk ()\n>  \t__git_complete_revlist\n>  }\n>  \n> -complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \\\n> -\t|| complete -o default -o nospace -F _git git\n> -complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \\\n> -\t|| complete -o default -o nospace -F _gitk gitk\n\nMore greatness.\n\n> +foo_wrap ()\n\nThis git-completion-specific internal helper function will pollute the\nuser's environment.  It should get a __git_ prefix.\n\n> +{\n> +\tlocal cmd=foo_cmd cmd_pos=1\n> +\tif [[ -n ${ZSH_VERSION-} ]]; then\n> +\t\temulate -L bash\n> +\t\tsetopt KSH_TYPESET\n> +\n> +\t\t# workaround zsh's bug that leaves 'words' as a special\n> +\t\t# variable in versions < 4.3.12\n> +\t\ttypeset -h words\n> +\n> +\t\t# workaround zsh's bug that quotes spaces in the COMPREPLY\n> +\t\t# array if IFS doesn't contain spaces.\n> +\t\ttypeset -h IFS\n> +\tfi\n> +\tlocal cur words cword prev\n> +\t_get_comp_words_by_ref -n =: cur words cword prev\n> +\tfoo \"$@\"\n> +}\n> +\n> +git_complete ()\n\nMaybe this should also get a prefix, but I'm not sure, because this\nis, strictly speaking, not a bash completion function.\n\n> +{\n> +\tlocal name=\"${2-$1}\"\n> +\tlocal cmd=\"${name#git_}\"\n> +\teval \"$(typeset -f foo_wrap | sed -e \"s/foo_cmd/$cmd/\" -e \"s/foo/_$name/\")\"\n> +\tcomplete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \\\n> +\t\t|| complete -o default -o nospace -F _${name}_wrap $1\n> +}\n> +\n> +git_complete git\n> +git_complete gitk\n\nClever ;)\n\nBut it needs a subshell and a sed invocation, so Windows people might\nnot be all too happy about this.  Perhaps it's not a big deal in the\ndefault case, because it will be done only twice at startup time.\nHowever, if someone wants several aliases on Windows, it can cause\nnoticeable delay.  It's a startup speed vs. convenience trade-off.\nAnyway, I don't have any ideas how those fork()s and exec() could be\navoided.\n\n\nBest,\nGábor\n"},{"id":"189058","messageId":"20120411230157.GV2289@goldbirke","threadId":"30182","inReplyTo":"CAMP44s3hYHxs2HZ59meEO=SU7gXuxU=m489LdxV38=7PDeDFdw@mail.gmail.com","subject":"Re: [PATCH 07/12] completion: simplify command stuff","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-04-11T23:01:57Z","receivedAt":"2012-04-11T23:01:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Apr 12, 2012 at 01:21:17AM +0300, Felipe Contreras wrote:\n> 2012/4/12 SZEDER Gábor <szeder@ira.uka.de>:\n> > However, this change \"promotes\" the command (or cmd) variable from\n> > being just a variable in _git() to a variable that is used in other\n> > completion functions, too.  Why not go one more step further, and\n> > denote this by adding a __git prefix (i.e. renaming the variable to\n> > __git_command)?\n> \n> After thinking more about it, and analyzing the next patches, I don't\n> think it makes sense to have such a variable; it's only used by\n> __git_complete_remote_or_refspec, and it would be easier to make it an\n> argument to that function; that would simplify this patch series a\n> lot.\n\nYou mean that _git_fetch() would call\n\n  __git_complete_remote_or_refspec \"fetch\"\n\nwhile _git_pull() and _git_push() will pass \"pull\" and \"push\",\nrespectively?\n\n\nBest,\nGábor\n"},{"id":"189060","messageId":"CAMP44s3x0ruqP_N9FH=GKnzXMYtX_kUaLO77YKRGr2GmsA3LHg@mail.gmail.com","threadId":"30182","inReplyTo":"20120411225046.GU2289@goldbirke","subject":"Re: [PATCH 10/12] completion: add new git_complete helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-11T23:44:19Z","receivedAt":"2012-04-11T23:44:19Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"2012/4/12 SZEDER Gábor <szeder@ira.uka.de>:\n\n>> +{\n>> +     local name=\"${2-$1}\"\n>> +     local cmd=\"${name#git_}\"\n>> +     eval \"$(typeset -f foo_wrap | sed -e \"s/foo_cmd/$cmd/\" -e \"s/foo/_$name/\")\"\n>> +     complete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \\\n>> +             || complete -o default -o nospace -F _${name}_wrap $1\n>> +}\n>> +\n>> +git_complete git\n>> +git_complete gitk\n>\n> Clever ;)\n>\n> But it needs a subshell and a sed invocation, so Windows people might\n> not be all too happy about this.  Perhaps it's not a big deal in the\n> default case, because it will be done only twice at startup time.\n> However, if someone wants several aliases on Windows, it can cause\n> noticeable delay.  It's a startup speed vs. convenience trade-off.\n> Anyway, I don't have any ideas how those fork()s and exec() could be\n> avoided.\n\nWell, if you look at the foo_wrap function you would notice there is\nnot much 'foo' in it. We could have a __git_wrap () function instead,\nbut we would need to find an alternative way to set the 'cmd'\nvariable. The easiest would be to just don't set it at all. As I\nsuggested, only __git_complete_remote_or_refspec needs it, and can be\neasily worked around.\n\n-- \nFelipe Contreras\n"},{"id":"189061","messageId":"CAMP44s0avJZDZjUUK3TaMDtxRJ2xXo-bELAqDB_gW1cE+LMP9g@mail.gmail.com","threadId":"30182","inReplyTo":"20120411230157.GV2289@goldbirke","subject":"Re: [PATCH 07/12] completion: simplify command stuff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-11T23:45:44Z","receivedAt":"2012-04-11T23:45:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"2012/4/12 SZEDER Gábor <szeder@ira.uka.de>:\n> On Thu, Apr 12, 2012 at 01:21:17AM +0300, Felipe Contreras wrote:\n>> 2012/4/12 SZEDER Gábor <szeder@ira.uka.de>:\n>> > However, this change \"promotes\" the command (or cmd) variable from\n>> > being just a variable in _git() to a variable that is used in other\n>> > completion functions, too.  Why not go one more step further, and\n>> > denote this by adding a __git prefix (i.e. renaming the variable to\n>> > __git_command)?\n>>\n>> After thinking more about it, and analyzing the next patches, I don't\n>> think it makes sense to have such a variable; it's only used by\n>> __git_complete_remote_or_refspec, and it would be easier to make it an\n>> argument to that function; that would simplify this patch series a\n>> lot.\n>\n> You mean that _git_fetch() would call\n>\n>  __git_complete_remote_or_refspec \"fetch\"\n>\n> while _git_pull() and _git_push() will pass \"pull\" and \"push\",\n> respectively?\n\nYeap. Then there would not be any need for the foo_wrap () stuff.\n\n-- \nFelipe Contreras\n"},{"id":"189063","messageId":"7v1untj0cl.fsf@alter.siamese.dyndns.org","threadId":"30182","inReplyTo":"CAMP44s2EunUabZdU23e+jYnBrWtBmhZeqt9yR+wV3X9HZhd7hA@mail.gmail.com","subject":"Re: [PATCH 01/12] tests: add initial bash completion tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-11T23:49:14Z","receivedAt":"2012-04-11T23:49:14Z","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> On Sun, Apr 8, 2012 at 7:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> ...\n>> What shell do you use on your system as /bin/sh (or if you use SHELL_PATH\n>> in the Makefile to override it, what do you use)?\n>\n> /bin/sh is a symlink to bash on my system, and although bash defines\n> POSIXLY_CORRECT, the tests pass just fine. I even tried to run /bin/sh\n> --posix.\n\nIt really depends on what's in contrib/git-completion.bash and what you\ntest.  \n\nFor example, if a (possibly future) version of completion script had lines\nlike these (taken from 6486ca6d7):\n\n\t__some_function ()\n        {\n\t\t...\n                while read key value; do\n                        ...\n                done < <(git config -z --get-regexp ...\n\t\t...\n\t}\n\ninvoking \"bash --posix\" would have failed like this:\n\n        $ bash --posix\n        bash-4.1$ _a ()\n        > {\n        >   while read x; do\n        >     echo \"* $x\"\n        >   done < <(echo a; echo b)\n        bash: syntax error near unexpected token `<'\n\nPerhaps the \"initial tests\" you posted, together with the version of the\ncompletion script we happen to have near 'master', didn't have any\nconstruct that gives a visible failure like the above, but that is just by\ndumb luck and is not a reason for us to be complacent and not to be\ncareful.\n\nThe early part of your re-roll (i.e. v2) seems to be done carefully to\nmake sure we run the script in full-fledged bash, from a quick glance.\n\nThanks.\n"},{"id":"189157","messageId":"CAMP44s3hAaVr2duNgPTg5VD0MmS9EJGVA2kNGrm_5ZshSBu+ew@mail.gmail.com","threadId":"30182","inReplyTo":"CAMP44s0avJZDZjUUK3TaMDtxRJ2xXo-bELAqDB_gW1cE+LMP9g@mail.gmail.com","subject":"Re: [PATCH 07/12] completion: simplify command stuff","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-04-12T23:08:25Z","receivedAt":"2012-04-12T23:08:25Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"2012/4/12 Felipe Contreras <felipe.contreras@gmail.com>:\n> 2012/4/12 SZEDER Gábor <szeder@ira.uka.de>:\n>> On Thu, Apr 12, 2012 at 01:21:17AM +0300, Felipe Contreras wrote:\n>>> 2012/4/12 SZEDER Gábor <szeder@ira.uka.de>:\n>>> > However, this change \"promotes\" the command (or cmd) variable from\n>>> > being just a variable in _git() to a variable that is used in other\n>>> > completion functions, too.  Why not go one more step further, and\n>>> > denote this by adding a __git prefix (i.e. renaming the variable to\n>>> > __git_command)?\n>>>\n>>> After thinking more about it, and analyzing the next patches, I don't\n>>> think it makes sense to have such a variable; it's only used by\n>>> __git_complete_remote_or_refspec, and it would be easier to make it an\n>>> argument to that function; that would simplify this patch series a\n>>> lot.\n>>\n>> You mean that _git_fetch() would call\n>>\n>>  __git_complete_remote_or_refspec \"fetch\"\n>>\n>> while _git_pull() and _git_push() will pass \"pull\" and \"push\",\n>> respectively?\n>\n> Yeap. Then there would not be any need for the foo_wrap () stuff.\n\nActually, there would be, we need to call _git_fetch(), but if we have\ngeneric wrap function we don't know that; all we have is the alias\n(e.g. gf from 'git fetch'). I guess we could do something like 'eval\n$(alias gf)' and find out the right function to call, but that already\nhas an 'eval' there, I'm not sure it's worth it.\n\nAny ideas?\n\n-- \nFelipe Contreras\n"}]}