{"thread":{"id":"32074","subject":"[PATCH v2 0/6] completion: test consolidations","startedAt":"2012-11-11T14:35:52Z","lastAt":"2012-11-17T20:26:00Z","messageCount":16,"participants":["Felipe Contreras","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"202857","messageId":"1352644558-9410-1-git-send-email-felipe.contreras@gmail.com","threadId":"32074","inReplyTo":null,"subject":"[PATCH v2 0/6] completion: test consolidations","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T14:35:52Z","receivedAt":"2012-11-11T14:35:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"These started from a discussion with SZEDER, but then I realized there were\nmany improvements possible.\n\nChanges since v1:\n\n * A lot more cleanups\n\nFelipe Contreras (6):\n  completion: add comment for test_completion()\n  completion: standardize final space marker in tests\n  completion: simplify tests using test_completion_long()\n  completion: consolidate test_completion*() tests\n  completion: refactor __gitcomp related tests\n  completion: simplify __gitcomp() test helper\n\n t/t9902-completion.sh | 133 +++++++++++++++++++-------------------------------\n 1 file changed, 51 insertions(+), 82 deletions(-)\n\n-- \n1.8.0\n"},{"id":"202859","messageId":"1352644558-9410-2-git-send-email-felipe.contreras@gmail.com","threadId":"32074","inReplyTo":"1352644558-9410-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 1/6] completion: add comment for test_completion()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T14:35:53Z","receivedAt":"2012-11-11T14:35:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So that it's easier to understand what it does.\n\nAlso, make sure we pass only the first argument for completion.\nShouldn't cause any functional changes because run_completion only\nchecks $1.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex cbd0fb6..5c06709 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -54,10 +54,14 @@ run_completion ()\n \t__git_wrap__git_main && print_comp\n }\n \n+# Test high-level completion\n+# Arguments are:\n+# 1: typed text so far (cur)\n+# 2: expected completion\n test_completion ()\n {\n \ttest $# -gt 1 && echo \"$2\" > expected\n-\trun_completion \"$@\" &&\n+\trun_completion \"$1\" &&\n \ttest_cmp expected out\n }\n \n-- \n1.8.0\n"},{"id":"202858","messageId":"1352644558-9410-3-git-send-email-felipe.contreras@gmail.com","threadId":"32074","inReplyTo":"1352644558-9410-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 2/6] completion: standardize final space marker in tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T14:35:54Z","receivedAt":"2012-11-11T14:35:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The rest of the code uses ' Z$'. Lets use that for\ntest_completion_long() as well.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 27 +++++++++++++--------------\n 1 file changed, 13 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 5c06709..aff7e44 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -66,11 +66,10 @@ test_completion ()\n }\n \n # Like test_completion, but reads expectation from stdin,\n-# which is convenient when it is multiline. We also process \"_\" into\n-# spaces to make test vectors more readable.\n+# which is convenient when it is multiline.\n test_completion_long ()\n {\n-\ttr _ \" \" >expected &&\n+\tsed -e 's/Z$//' > expected &&\n \ttest_completion \"$1\"\n }\n \n@@ -252,24 +251,24 @@ test_expect_success 'setup for ref completion' '\n \n test_expect_success 'checkout completes ref names' '\n \ttest_completion_long \"git checkout m\" <<-\\EOF\n-\tmaster_\n-\tmybranch_\n-\tmytag_\n+\tmaster Z\n+\tmybranch Z\n+\tmytag Z\n \tEOF\n '\n \n test_expect_success 'show completes all refs' '\n \ttest_completion_long \"git show m\" <<-\\EOF\n-\tmaster_\n-\tmybranch_\n-\tmytag_\n+\tmaster Z\n+\tmybranch Z\n+\tmytag Z\n \tEOF\n '\n \n test_expect_success '<ref>: completes paths' '\n \ttest_completion_long \"git show mytag:f\" <<-\\EOF\n-\tfile1_\n-\tfile2_\n+\tfile1 Z\n+\tfile2 Z\n \tEOF\n '\n \n@@ -278,7 +277,7 @@ test_expect_success 'complete tree filename with spaces' '\n \tgit add . &&\n \tgit commit -m spaces &&\n \ttest_completion_long \"git show HEAD:nam\" <<-\\EOF\n-\tname with spaces_\n+\tname with spaces Z\n \tEOF\n '\n \n@@ -287,8 +286,8 @@ test_expect_failure 'complete tree filename with metacharacters' '\n \tgit add . &&\n \tgit commit -m meta &&\n \ttest_completion_long \"git show HEAD:nam\" <<-\\EOF\n-\tname with ${meta}_\n-\tname with spaces_\n+\tname with ${meta} Z\n+\tname with spaces Z\n \tEOF\n '\n \n-- \n1.8.0\n"},{"id":"202860","messageId":"1352644558-9410-4-git-send-email-felipe.contreras@gmail.com","threadId":"32074","inReplyTo":"1352644558-9410-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 3/6] completion: simplify tests using test_completion_long()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T14:35:55Z","receivedAt":"2012-11-11T14:35:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No need to duplicate that functionality.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 9 +++------\n 1 file changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex aff7e44..204c92a 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -172,7 +172,7 @@ test_expect_success 'basic' '\n '\n \n test_expect_success 'double dash \"git\" itself' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_completion_long \"git --\" <<-\\EOF\n \t--paginate Z\n \t--no-pager Z\n \t--git-dir=\n@@ -187,11 +187,10 @@ test_expect_success 'double dash \"git\" itself' '\n \t--no-replace-objects Z\n \t--help Z\n \tEOF\n-\ttest_completion \"git --\"\n '\n \n test_expect_success 'double dash \"git checkout\"' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_completion_long \"git checkout --\" <<-\\EOF\n \t--quiet Z\n \t--ours Z\n \t--theirs Z\n@@ -202,17 +201,15 @@ test_expect_success 'double dash \"git checkout\"' '\n \t--orphan Z\n \t--patch Z\n \tEOF\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-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_completion_long \"git --exe\" <<-\\EOF &&\n \t--exec-path Z\n \t--exec-path=\n \tEOF\n-\ttest_completion \"git --exe\" &&\n \ttest_completion \"git --htm\" \"--html-path \" &&\n \ttest_completion \"git --pag\" \"--paginate \" &&\n \ttest_completion \"git --no-p\" \"--no-pager \" &&\n-- \n1.8.0\n"},{"id":"202861","messageId":"1352644558-9410-5-git-send-email-felipe.contreras@gmail.com","threadId":"32074","inReplyTo":"1352644558-9410-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 4/6] completion: consolidate test_completion*() tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T14:35:56Z","receivedAt":"2012-11-11T14:35:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No need to have two versions; if a second argument is specified, use\nthat, otherwise use stdin.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 30 +++++++++++++-----------------\n 1 file changed, 13 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 204c92a..59cdbfd 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -60,19 +60,15 @@ run_completion ()\n # 2: expected completion\n test_completion ()\n {\n-\ttest $# -gt 1 && echo \"$2\" > expected\n+\tif [ $# -gt 1 ]; then\n+\t\techo \"$2\" > expected\n+\telse\n+\t\tsed -e 's/Z$//' > expected\n+\tfi &&\n \trun_completion \"$1\" &&\n \ttest_cmp expected out\n }\n \n-# Like test_completion, but reads expectation from stdin,\n-# which is convenient when it is multiline.\n-test_completion_long ()\n-{\n-\tsed -e 's/Z$//' > expected &&\n-\ttest_completion \"$1\"\n-}\n-\n newline=$'\\n'\n \n test_expect_success '__gitcomp - trailing space - options' '\n@@ -172,7 +168,7 @@ test_expect_success 'basic' '\n '\n \n test_expect_success 'double dash \"git\" itself' '\n-\ttest_completion_long \"git --\" <<-\\EOF\n+\ttest_completion \"git --\" <<-\\EOF\n \t--paginate Z\n \t--no-pager Z\n \t--git-dir=\n@@ -190,7 +186,7 @@ test_expect_success 'double dash \"git\" itself' '\n '\n \n test_expect_success 'double dash \"git checkout\"' '\n-\ttest_completion_long \"git checkout --\" <<-\\EOF\n+\ttest_completion \"git checkout --\" <<-\\EOF\n \t--quiet Z\n \t--ours Z\n \t--theirs Z\n@@ -206,7 +202,7 @@ test_expect_success 'double dash \"git checkout\"' '\n test_expect_success 'general options' '\n \ttest_completion \"git --ver\" \"--version \" &&\n \ttest_completion \"git --hel\" \"--help \" &&\n-\ttest_completion_long \"git --exe\" <<-\\EOF &&\n+\ttest_completion \"git --exe\" <<-\\EOF &&\n \t--exec-path Z\n \t--exec-path=\n \tEOF\n@@ -247,7 +243,7 @@ test_expect_success 'setup for ref completion' '\n '\n \n test_expect_success 'checkout completes ref names' '\n-\ttest_completion_long \"git checkout m\" <<-\\EOF\n+\ttest_completion \"git checkout m\" <<-\\EOF\n \tmaster Z\n \tmybranch Z\n \tmytag Z\n@@ -255,7 +251,7 @@ test_expect_success 'checkout completes ref names' '\n '\n \n test_expect_success 'show completes all refs' '\n-\ttest_completion_long \"git show m\" <<-\\EOF\n+\ttest_completion \"git show m\" <<-\\EOF\n \tmaster Z\n \tmybranch Z\n \tmytag Z\n@@ -263,7 +259,7 @@ test_expect_success 'show completes all refs' '\n '\n \n test_expect_success '<ref>: completes paths' '\n-\ttest_completion_long \"git show mytag:f\" <<-\\EOF\n+\ttest_completion \"git show mytag:f\" <<-\\EOF\n \tfile1 Z\n \tfile2 Z\n \tEOF\n@@ -273,7 +269,7 @@ test_expect_success 'complete tree filename with spaces' '\n \techo content >\"name with spaces\" &&\n \tgit add . &&\n \tgit commit -m spaces &&\n-\ttest_completion_long \"git show HEAD:nam\" <<-\\EOF\n+\ttest_completion \"git show HEAD:nam\" <<-\\EOF\n \tname with spaces Z\n \tEOF\n '\n@@ -282,7 +278,7 @@ test_expect_failure 'complete tree filename with metacharacters' '\n \techo content >\"name with \\${meta}\" &&\n \tgit add . &&\n \tgit commit -m meta &&\n-\ttest_completion_long \"git show HEAD:nam\" <<-\\EOF\n+\ttest_completion \"git show HEAD:nam\" <<-\\EOF\n \tname with ${meta} Z\n \tname with spaces Z\n \tEOF\n-- \n1.8.0\n"},{"id":"202862","messageId":"1352644558-9410-6-git-send-email-felipe.contreras@gmail.com","threadId":"32074","inReplyTo":"1352644558-9410-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 5/6] completion: refactor __gitcomp related tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T14:35:57Z","receivedAt":"2012-11-11T14:35:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Lots of duplicated code!\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 76 ++++++++++++++++++---------------------------------\n 1 file changed, 27 insertions(+), 49 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 59cdbfd..66c7af6 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -71,87 +71,65 @@ test_completion ()\n \n newline=$'\\n'\n \n-test_expect_success '__gitcomp - trailing space - options' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n-\t--reuse-message=Z\n-\t--reedit-message=Z\n-\t--reset-author Z\n-\tEOF\n+# Test __gitcomp.\n+# Arguments are:\n+# 1: typed text so far (cur)\n+# *: arguments to pass to __gitcomp\n+test_gitcomp ()\n+{\n+\tsed -e 's/Z$//' > expected &&\n \t(\n \t\tlocal -a COMPREPLY &&\n-\t\tcur=\"--re\" &&\n-\t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n-\t\t\t\t--reset-author\" &&\n+\t\tcur=\"$1\" &&\n+\t\tshift &&\n+\t\t__gitcomp \"$@\" &&\n \t\tIFS=\"$newline\" &&\n \t\techo \"${COMPREPLY[*]}\" > out\n \t) &&\n \ttest_cmp expected out\n+}\n+\n+test_expect_success '__gitcomp - trailing space - options' '\n+\ttest_gitcomp \"--re\" \"--dry-run --reuse-message= --reedit-message=\n+\t\t--reset-author\" <<-EOF\n+\t--reuse-message=Z\n+\t--reedit-message=Z\n+\t--reset-author Z\n+\tEOF\n '\n \n test_expect_success '__gitcomp - trailing space - config keys' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"br\" \"branch. branch.autosetupmerge\n+\t\tbranch.autosetuprebase browser.\" <<-\\EOF\n \tbranch.Z\n \tbranch.autosetupmerge Z\n \tbranch.autosetuprebase Z\n \tbrowser.Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"br\" &&\n-\t\t__gitcomp \"branch. branch.autosetupmerge\n-\t\t\t\tbranch.autosetuprebase browser.\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success '__gitcomp - option parameter' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"--strategy=re\" \"octopus ours recursive resolve subtree\" \\\n+\t\t\"\" \"re\" <<-\\EOF\n \trecursive Z\n \tresolve Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"--strategy=re\" &&\n-\t\t__gitcomp \"octopus ours recursive resolve subtree\n-\t\t\t\" \"\" \"re\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success '__gitcomp - prefix' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"branch.me\" \"remote merge mergeoptions rebase\" \\\n+\t\t\"branch.maint.\" \"me\" <<-\\EOF\n \tbranch.maint.merge Z\n \tbranch.maint.mergeoptions Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"branch.me\" &&\n-\t\t__gitcomp \"remote merge mergeoptions rebase\n-\t\t\t\" \"branch.maint.\" \"me\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success '__gitcomp - suffix' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"branch.me\" \"master maint next pu\" \"branch.\" \\\n+\t\t\"ma\" \".\" <<-\\EOF\n \tbranch.master.Z\n \tbranch.maint.Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"branch.me\" &&\n-\t\t__gitcomp \"master maint next pu\n-\t\t\t\" \"branch.\" \"ma\" \".\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success 'basic' '\n-- \n1.8.0\n"},{"id":"202863","messageId":"1352644558-9410-7-git-send-email-felipe.contreras@gmail.com","threadId":"32074","inReplyTo":"1352644558-9410-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 6/6] completion: simplify __gitcomp() test helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T14:35:58Z","receivedAt":"2012-11-11T14:35:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"By using print_comp as suggested by SZEDER Gábor.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 15 +++++----------\n 1 file changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 66c7af6..9b38b69 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -69,23 +69,18 @@ test_completion ()\n \ttest_cmp expected out\n }\n \n-newline=$'\\n'\n-\n # Test __gitcomp.\n # Arguments are:\n # 1: typed text so far (cur)\n # *: arguments to pass to __gitcomp\n test_gitcomp ()\n {\n+\tlocal -a COMPREPLY &&\n \tsed -e 's/Z$//' > expected &&\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"$1\" &&\n-\t\tshift &&\n-\t\t__gitcomp \"$@\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n+\tcur=\"$1\" &&\n+\tshift &&\n+\t__gitcomp \"$@\" &&\n+\tprint_comp &&\n \ttest_cmp expected out\n }\n \n-- \n1.8.0\n"},{"id":"203360","messageId":"7v8va1mkcv.fsf@alter.siamese.dyndns.org","threadId":"32074","inReplyTo":"1352644558-9410-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 5/6] completion: refactor __gitcomp related tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-16T19:13:52Z","receivedAt":"2012-11-16T19:13:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Not asking for a re-roll but am asking for clarification so that I\ncan locally update before queuing.\n\nFelipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Lots of duplicated code!\n\n... removed, you mean?\n\n> No functional changes.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 76 ++++++++++++++++++---------------------------------\n>  1 file changed, 27 insertions(+), 49 deletions(-)\n>\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 59cdbfd..66c7af6 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -71,87 +71,65 @@ test_completion ()\n>  \n>  newline=$'\\n'\n>  \n> -test_expect_success '__gitcomp - trailing space - options' '\n> -\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n> -\t--reuse-message=Z\n> -\t--reedit-message=Z\n> -\t--reset-author Z\n> -\tEOF\n> +# Test __gitcomp.\n> +# Arguments are:\n> +# 1: typed text so far (cur)\n> +# *: arguments to pass to __gitcomp\n\ns/\\*/remainder/, perhaps?  I think you shift $1 out and do not pass\nit to __gitcomp.\n\nAnd expected output is from the standard input just like\ntest_completion?\n\n> +test_gitcomp ()\n> +{\n> +\tsed -e 's/Z$//' > expected &&\n>  \t(\n>  \t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"--re\" &&\n> -\t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n> -\t\t\t\t--reset-author\" &&\n> +\t\tcur=\"$1\" &&\n> +\t\tshift &&\n> +\t\t__gitcomp \"$@\" &&\n>  \t\tIFS=\"$newline\" &&\n>  \t\techo \"${COMPREPLY[*]}\" > out\n>  \t) &&\n>  \ttest_cmp expected out\n> +}\n> +\n> +test_expect_success '__gitcomp - trailing space - options' '\n> +\ttest_gitcomp \"--re\" \"--dry-run --reuse-message= --reedit-message=\n> +\t\t--reset-author\" <<-EOF\n> +\t--reuse-message=Z\n> +\t--reedit-message=Z\n> +\t--reset-author Z\n> +\tEOF\n>  '\n\nNice shrinkage.\n"},{"id":"203362","messageId":"CAMP44s2ESxhQQGxy-E4LpiBwPnZLgVwBFDk4Ck90ySTXdC=DYA@mail.gmail.com","threadId":"32074","inReplyTo":"7v8va1mkcv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 5/6] completion: refactor __gitcomp related tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-16T19:53:56Z","receivedAt":"2012-11-16T19:53:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 16, 2012 at 8:13 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Not asking for a re-roll but am asking for clarification so that I\n> can locally update before queuing.\n>\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Lots of duplicated code!\n>\n> ... removed, you mean?\n\nYes.\n\n>> No functional changes.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  t/t9902-completion.sh | 76 ++++++++++++++++++---------------------------------\n>>  1 file changed, 27 insertions(+), 49 deletions(-)\n>>\n>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>> index 59cdbfd..66c7af6 100755\n>> --- a/t/t9902-completion.sh\n>> +++ b/t/t9902-completion.sh\n>> @@ -71,87 +71,65 @@ test_completion ()\n>>\n>>  newline=$'\\n'\n>>\n>> -test_expect_success '__gitcomp - trailing space - options' '\n>> -     sed -e \"s/Z$//\" >expected <<-\\EOF &&\n>> -     --reuse-message=Z\n>> -     --reedit-message=Z\n>> -     --reset-author Z\n>> -     EOF\n>> +# Test __gitcomp.\n>> +# Arguments are:\n>> +# 1: typed text so far (cur)\n>> +# *: arguments to pass to __gitcomp\n>\n> s/\\*/remainder/, perhaps?  I think you shift $1 out and do not pass\n> it to __gitcomp.\n\nRight, by * I meant the rest.\n\n> And expected output is from the standard input just like\n> test_completion?\n\nCorrect.\n\n>> +test_gitcomp ()\n>> +{\n>> +     sed -e 's/Z$//' > expected &&\n>>       (\n>>               local -a COMPREPLY &&\n>> -             cur=\"--re\" &&\n>> -             __gitcomp \"--dry-run --reuse-message= --reedit-message=\n>> -                             --reset-author\" &&\n>> +             cur=\"$1\" &&\n>> +             shift &&\n>> +             __gitcomp \"$@\" &&\n>>               IFS=\"$newline\" &&\n>>               echo \"${COMPREPLY[*]}\" > out\n>>       ) &&\n>>       test_cmp expected out\n>> +}\n>> +\n>> +test_expect_success '__gitcomp - trailing space - options' '\n>> +     test_gitcomp \"--re\" \"--dry-run --reuse-message= --reedit-message=\n>> +             --reset-author\" <<-EOF\n>> +     --reuse-message=Z\n>> +     --reedit-message=Z\n>> +     --reset-author Z\n>> +     EOF\n>>  '\n>\n> Nice shrinkage.\n\nThat's a comment about the whole patch series I hope :)\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203372","messageId":"20121116205457.GY12052@goldbirke","threadId":"32074","inReplyTo":"1352644558-9410-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 1/6] completion: add comment for test_completion()","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-16T20:54:57Z","receivedAt":"2012-11-16T20:54:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Nov 11, 2012 at 03:35:53PM +0100, Felipe Contreras wrote:\n> So that it's easier to understand what it does.\n> \n> Also, make sure we pass only the first argument for completion.\n> Shouldn't cause any functional changes because run_completion only\n> checks $1.\n> \n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 6 +++++-\n>  1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index cbd0fb6..5c06709 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -54,10 +54,14 @@ run_completion ()\n>  \t__git_wrap__git_main && print_comp\n>  }\n>  \n> +# Test high-level completion\n> +# Arguments are:\n> +# 1: typed text so far (cur)\n\nBash manuals calls this the current command line or words in the\ncurrent command line.  I'm not sure what you mean with '(cur)' here.\nThe variable $cur in the completion script (or in bash-completion in\ngeneral) is something completely different.\n\n> +# 2: expected completion\n>  test_completion ()\n>  {\n>  \ttest $# -gt 1 && echo \"$2\" > expected\n> -\trun_completion \"$@\" &&\n> +\trun_completion \"$1\" &&\n>  \ttest_cmp expected out\n>  }\n>  \n> -- \n> 1.8.0\n"},{"id":"203373","messageId":"20121116210242.GZ12052@goldbirke","threadId":"32074","inReplyTo":"1352644558-9410-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 5/6] completion: refactor __gitcomp related tests","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-16T21:02:42Z","receivedAt":"2012-11-16T21:02:42Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Nov 11, 2012 at 03:35:57PM +0100, Felipe Contreras wrote:\n> Lots of duplicated code!\n> \n> No functional changes.\n> \n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 76 ++++++++++++++++++---------------------------------\n>  1 file changed, 27 insertions(+), 49 deletions(-)\n\nDespite the impressive numbers, these tests are more useful without\nthis cleanup.\n\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 59cdbfd..66c7af6 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -71,87 +71,65 @@ test_completion ()\n>  \n>  newline=$'\\n'\n>  \n> -test_expect_success '__gitcomp - trailing space - options' '\n> -\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n> -\t--reuse-message=Z\n> -\t--reedit-message=Z\n> -\t--reset-author Z\n> -\tEOF\n> +# Test __gitcomp.\n> +# Arguments are:\n> +# 1: typed text so far (cur)\n\nThe first argument is not the typed text so far, but the word\ncurrently containing the cursor position.\n\n> +# *: arguments to pass to __gitcomp\n> +test_gitcomp ()\n> +{\n> +\tsed -e 's/Z$//' > expected &&\n>  \t(\n>  \t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"--re\" &&\n> -\t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n> -\t\t\t\t--reset-author\" &&\n> +\t\tcur=\"$1\" &&\n> +\t\tshift &&\n> +\t\t__gitcomp \"$@\" &&\n>  \t\tIFS=\"$newline\" &&\n>  \t\techo \"${COMPREPLY[*]}\" > out\n>  \t) &&\n>  \ttest_cmp expected out\n> +}\n> +\n> +test_expect_success '__gitcomp - trailing space - options' '\n> +\ttest_gitcomp \"--re\" \"--dry-run --reuse-message= --reedit-message=\n> +\t\t--reset-author\" <<-EOF\n> +\t--reuse-message=Z\n> +\t--reedit-message=Z\n> +\t--reset-author Z\n> +\tEOF\n>  '\n>  \n>  test_expect_success '__gitcomp - trailing space - config keys' '\n> -\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n> +\ttest_gitcomp \"br\" \"branch. branch.autosetupmerge\n> +\t\tbranch.autosetuprebase browser.\" <<-\\EOF\n>  \tbranch.Z\n>  \tbranch.autosetupmerge Z\n>  \tbranch.autosetuprebase Z\n>  \tbrowser.Z\n>  \tEOF\n> -\t(\n> -\t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"br\" &&\n> -\t\t__gitcomp \"branch. branch.autosetupmerge\n> -\t\t\t\tbranch.autosetuprebase browser.\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> -\t) &&\n> -\ttest_cmp expected out\n>  '\n>  \n>  test_expect_success '__gitcomp - option parameter' '\n> -\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n> +\ttest_gitcomp \"--strategy=re\" \"octopus ours recursive resolve subtree\" \\\n> +\t\t\"\" \"re\" <<-\\EOF\n>  \trecursive Z\n>  \tresolve Z\n>  \tEOF\n> -\t(\n> -\t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"--strategy=re\" &&\n> -\t\t__gitcomp \"octopus ours recursive resolve subtree\n> -\t\t\t\" \"\" \"re\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> -\t) &&\n> -\ttest_cmp expected out\n>  '\n>  \n>  test_expect_success '__gitcomp - prefix' '\n> -\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n> +\ttest_gitcomp \"branch.me\" \"remote merge mergeoptions rebase\" \\\n> +\t\t\"branch.maint.\" \"me\" <<-\\EOF\n>  \tbranch.maint.merge Z\n>  \tbranch.maint.mergeoptions Z\n>  \tEOF\n> -\t(\n> -\t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"branch.me\" &&\n> -\t\t__gitcomp \"remote merge mergeoptions rebase\n> -\t\t\t\" \"branch.maint.\" \"me\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> -\t) &&\n> -\ttest_cmp expected out\n>  '\n>  \n>  test_expect_success '__gitcomp - suffix' '\n> -\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n> +\ttest_gitcomp \"branch.me\" \"master maint next pu\" \"branch.\" \\\n> +\t\t\"ma\" \".\" <<-\\EOF\n>  \tbranch.master.Z\n>  \tbranch.maint.Z\n>  \tEOF\n> -\t(\n> -\t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"branch.me\" &&\n> -\t\t__gitcomp \"master maint next pu\n> -\t\t\t\" \"branch.\" \"ma\" \".\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> -\t) &&\n> -\ttest_cmp expected out\n>  '\n>  \n>  test_expect_success 'basic' '\n> -- \n> 1.8.0\n> \n"},{"id":"203375","messageId":"CAMP44s333124DkyG5VX+n=p_eHRmPYCu27rsLgLQG8mOVcRBPg@mail.gmail.com","threadId":"32074","inReplyTo":"20121116205457.GY12052@goldbirke","subject":"Re: [PATCH v2 1/6] completion: add comment for test_completion()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-16T21:06:53Z","receivedAt":"2012-11-16T21:06:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 16, 2012 at 9:54 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> On Sun, Nov 11, 2012 at 03:35:53PM +0100, Felipe Contreras wrote:\n>> So that it's easier to understand what it does.\n>>\n>> Also, make sure we pass only the first argument for completion.\n>> Shouldn't cause any functional changes because run_completion only\n>> checks $1.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  t/t9902-completion.sh | 6 +++++-\n>>  1 file changed, 5 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>> index cbd0fb6..5c06709 100755\n>> --- a/t/t9902-completion.sh\n>> +++ b/t/t9902-completion.sh\n>> @@ -54,10 +54,14 @@ run_completion ()\n>>       __git_wrap__git_main && print_comp\n>>  }\n>>\n>> +# Test high-level completion\n>> +# Arguments are:\n>> +# 1: typed text so far (cur)\n>\n> Bash manuals calls this the current command line or words in the\n> current command line.  I'm not sure what you mean with '(cur)' here.\n\nThe current _word_ text typed so far.\n\n> The variable $cur in the completion script (or in bash-completion in\n> general) is something completely different.\n\nI believe bash's completion, this test, and the whole git completion\nstuff uses the same definition of 'cur'.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203376","messageId":"CAMP44s0m2iATv7Bt0Dy4hTcJvBNvzvN+Vcc+=NVLCMrVP_YJDw@mail.gmail.com","threadId":"32074","inReplyTo":"CAMP44s333124DkyG5VX+n=p_eHRmPYCu27rsLgLQG8mOVcRBPg@mail.gmail.com","subject":"Re: [PATCH v2 1/6] completion: add comment for test_completion()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-16T21:09:05Z","receivedAt":"2012-11-16T21:09:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 16, 2012 at 10:06 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Fri, Nov 16, 2012 at 9:54 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n>> On Sun, Nov 11, 2012 at 03:35:53PM +0100, Felipe Contreras wrote:\n>>> So that it's easier to understand what it does.\n>>>\n>>> Also, make sure we pass only the first argument for completion.\n>>> Shouldn't cause any functional changes because run_completion only\n>>> checks $1.\n>>>\n>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>>> ---\n>>>  t/t9902-completion.sh | 6 +++++-\n>>>  1 file changed, 5 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>>> index cbd0fb6..5c06709 100755\n>>> --- a/t/t9902-completion.sh\n>>> +++ b/t/t9902-completion.sh\n>>> @@ -54,10 +54,14 @@ run_completion ()\n>>>       __git_wrap__git_main && print_comp\n>>>  }\n>>>\n>>> +# Test high-level completion\n>>> +# Arguments are:\n>>> +# 1: typed text so far (cur)\n>>\n>> Bash manuals calls this the current command line or words in the\n>> current command line.  I'm not sure what you mean with '(cur)' here.\n>\n> The current _word_ text typed so far.\n>\n>> The variable $cur in the completion script (or in bash-completion in\n>> general) is something completely different.\n>\n> I believe bash's completion, this test, and the whole git completion\n> stuff uses the same definition of 'cur'.\n\nOops, actually in this particular helper this is not actually 'cur'. I\nthink 'command line' might be more appropriate.\n\n-- \nFelipe Contreras\n"},{"id":"203382","messageId":"7vip95ktfy.fsf@alter.siamese.dyndns.org","threadId":"32074","inReplyTo":"20121116210242.GZ12052@goldbirke","subject":"Re: [PATCH v2 5/6] completion: refactor __gitcomp related tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-16T23:40:33Z","receivedAt":"2012-11-16T23:40:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n> On Sun, Nov 11, 2012 at 03:35:57PM +0100, Felipe Contreras wrote:\n>> Lots of duplicated code!\n>> \n>> No functional changes.\n>> \n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  t/t9902-completion.sh | 76 ++++++++++++++++++---------------------------------\n>>  1 file changed, 27 insertions(+), 49 deletions(-)\n>\n> Despite the impressive numbers, these tests are more useful without\n> this cleanup.\n\nIs this because consolidation of the duplicated part of the tests\ninto a single helper makes it harder to instrument one test you are\ninterested in (or developing) for debugging?\n\nIt indeed is a problem, and cutting and pasting the same code to\nmultiple tests is one way to solve the problem (you can easily\ninstrument the copy in the test you are interested in while leaving\nothers intact), but I do not think that is a good solution.  A bugfix\nor enhancement to the shared (or duplicated) part can be done by\ntouching one place only after this change, while with the current\ncode you have to repeat the same fix to all places, no?\n\n>> +# Test __gitcomp.\n>> +# Arguments are:\n>> +# 1: typed text so far (cur)\n>\n> The first argument is not the typed text so far, but the word\n> currently containing the cursor position.\n\nCare to update this with a follow-up patch, so that I do not have to\nkeep track of minute details ;-)?\n"},{"id":"203383","messageId":"7v7gplktdt.fsf@alter.siamese.dyndns.org","threadId":"32074","inReplyTo":"1352644558-9410-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 4/6] completion: consolidate test_completion*() tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-16T23:41:50Z","receivedAt":"2012-11-16T23:41:50Z","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> No need to have two versions; if a second argument is specified, use\n> that, otherwise use stdin.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 30 +++++++++++++-----------------\n>  1 file changed, 13 insertions(+), 17 deletions(-)\n>\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 204c92a..59cdbfd 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -60,19 +60,15 @@ run_completion ()\n>  # 2: expected completion\n>  test_completion ()\n>  {\n> -\ttest $# -gt 1 && echo \"$2\" > expected\n> +\tif [ $# -gt 1 ]; then\n> +\t\techo \"$2\" > expected\n> +\telse\n> +\t\tsed -e 's/Z$//' > expected\n> +\tfi &&\n\nAs \"$2\" could begin with dash, end with \\c, etc. that possibly can\nbe misinterpred by echo, I'd rewrite this as\n\n\t\tprintf '%s\\n' \"$2\" >expected\n\nOtherwise looked fine; thanks.\n"},{"id":"203441","messageId":"CAMP44s2ppLttMkqBDQ6ROnMfMyRUUq371LCH+FWOvKX-uE4=SA@mail.gmail.com","threadId":"32074","inReplyTo":"7v7gplktdt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 4/6] completion: consolidate test_completion*() tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T20:26:00Z","receivedAt":"2012-11-17T20:26:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 17, 2012 at 12:41 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> No need to have two versions; if a second argument is specified, use\n>> that, otherwise use stdin.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  t/t9902-completion.sh | 30 +++++++++++++-----------------\n>>  1 file changed, 13 insertions(+), 17 deletions(-)\n>>\n>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>> index 204c92a..59cdbfd 100755\n>> --- a/t/t9902-completion.sh\n>> +++ b/t/t9902-completion.sh\n>> @@ -60,19 +60,15 @@ run_completion ()\n>>  # 2: expected completion\n>>  test_completion ()\n>>  {\n>> -     test $# -gt 1 && echo \"$2\" > expected\n>> +     if [ $# -gt 1 ]; then\n>> +             echo \"$2\" > expected\n>> +     else\n>> +             sed -e 's/Z$//' > expected\n>> +     fi &&\n>\n> As \"$2\" could begin with dash, end with \\c, etc. that possibly can\n> be misinterpred by echo, I'd rewrite this as\n>\n>                 printf '%s\\n' \"$2\" >expected\n>\n> Otherwise looked fine; thanks.\n\nBut that was the case before. I would do that in a separate patch.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}