{"thread":{"id":"36275","subject":"[PATCH 02/10] t4209: factor out helper function test_log()","startedAt":"2014-03-22T17:15:50Z","lastAt":"2014-03-24T22:09:50Z","messageCount":21,"participants":["René Scharfe","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"237402","messageId":"1395508560-19893-1-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":null,"subject":"[PATCH 00/10] pickaxe: honor -i when used with -S and --pickaxe-regex; cleanups","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:50Z","receivedAt":"2014-03-22T17:15:50Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This series allows the options -i/--regexp-ignore-case, --pickaxe-regex,\nand -S to be used together and work as expected to perform a pickaxe\nsearch using case-insensitive regular expression matching.  Its first\nhalf refactors the test script and extends test coverage a bit while\nwe're at it.  The actual change is in the sixth patch.  It enables the\ntwo following cleanups.  The last two patches are independent simple\ncleanups.\n\n  t4209: set up expectations up front\n  t4209: factor out helper function test_log()\n  t4209: factor out helper function test_log_icase()\n  t4209: use helper functions to test --grep\n  t4209: use helper functions to test --author\n  pickaxe: honor -i when used with -S and --pickaxe-regex\n  pickaxe: merge diffcore_pickaxe_grep() and diffcore_pickaxe_count()\n    into diffcore_pickaxe()\n  pickaxe: move pickaxe() after pickaxe_match()\n  pickaxe: call strlen only when necessary in diffcore_pickaxe_count()\n  pickaxe: simplify kwset loop in contains()\n\n diffcore-pickaxe.c     | 142 +++++++++++++++++--------------------------\n t/t4209-log-pickaxe.sh | 159 +++++++++++++++++--------------------------------\n 2 files changed, 110 insertions(+), 191 deletions(-)\n\n-- \n1.9.1\n"},{"id":"237403","messageId":"1395508560-19893-2-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 01/10] t4209: set up expectations up front","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:51Z","receivedAt":"2014-03-22T17:15:51Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Instead of creating an expect file for each test, build three files with\nthe possible valid values during setup and use them in the tests.  This\nshortens the test code and saves nine calls to git rev-parse.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t4209-log-pickaxe.sh | 64 ++++++++++++++++++++------------------------------\n 1 file changed, 25 insertions(+), 39 deletions(-)\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex 38fb80f..ff668b5 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -4,80 +4,74 @@ test_description='log --grep/--author/--regexp-ignore-case/-S/-G'\n . ./test-lib.sh\n \n test_expect_success setup '\n+\t>expect_nomatch &&\n+\n \t>file &&\n \tgit add file &&\n \ttest_tick &&\n \tgit commit -m initial &&\n+\tgit rev-parse --verify HEAD >expect_initial &&\n \n \techo Picked >file &&\n+\tgit add file &&\n \ttest_tick &&\n-\tgit commit -a --author=\"Another Person <another@example.com>\" -m second\n+\tgit commit --author=\"Another Person <another@example.com>\" -m second &&\n+\tgit rev-parse --verify HEAD >expect_second\n '\n \n test_expect_success 'log --grep' '\n \tgit log --grep=initial --format=%H >actual &&\n-\tgit rev-parse --verify HEAD^ >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_initial actual\n '\n \n test_expect_success 'log --grep --regexp-ignore-case' '\n \tgit log --regexp-ignore-case --grep=InItial --format=%H >actual &&\n-\tgit rev-parse --verify HEAD^ >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_initial actual\n '\n \n test_expect_success 'log --grep -i' '\n \tgit log -i --grep=InItial --format=%H >actual &&\n-\tgit rev-parse --verify HEAD^ >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_initial actual\n '\n \n test_expect_success 'log --author --regexp-ignore-case' '\n \tgit log --regexp-ignore-case --author=person --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log --author -i' '\n \tgit log -i --author=person --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log -G (nomatch)' '\n \tgit log -Gpicked --format=%H >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_nomatch actual\n '\n \n test_expect_success 'log -G (match)' '\n \tgit log -GPicked --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log -G --regexp-ignore-case (nomatch)' '\n \tgit log --regexp-ignore-case -Gpickle --format=%H >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_nomatch actual\n '\n \n test_expect_success 'log -G -i (nomatch)' '\n \tgit log -i -Gpickle --format=%H >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_nomatch actual\n '\n \n test_expect_success 'log -G --regexp-ignore-case (match)' '\n \tgit log --regexp-ignore-case -Gpicked --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log -G -i (match)' '\n \tgit log -i -Gpicked --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log -G --textconv (missing textconv tool)' '\n@@ -89,45 +83,38 @@ test_expect_success 'log -G --textconv (missing textconv tool)' '\n test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n \tgit -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual &&\n+\ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n test_expect_success 'log -S (nomatch)' '\n \tgit log -Spicked --format=%H >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_nomatch actual\n '\n \n test_expect_success 'log -S (match)' '\n \tgit log -SPicked --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log -S --regexp-ignore-case (match)' '\n \tgit log --regexp-ignore-case -Spicked --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log -S -i (match)' '\n \tgit log -i -Spicked --format=%H >actual &&\n-\tgit rev-parse --verify HEAD >expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_second actual\n '\n \n test_expect_success 'log -S --regexp-ignore-case (nomatch)' '\n \tgit log --regexp-ignore-case -Spickle --format=%H >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_nomatch actual\n '\n \n test_expect_success 'log -S -i (nomatch)' '\n \tgit log -i -Spickle --format=%H >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect_nomatch actual\n '\n \n test_expect_success 'log -S --textconv (missing textconv tool)' '\n@@ -139,8 +126,7 @@ test_expect_success 'log -S --textconv (missing textconv tool)' '\n test_expect_success 'log -S --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n \tgit -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&\n-\t>expect &&\n-\ttest_cmp expect actual &&\n+\ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n-- \n1.9.1\n"},{"id":"237395","messageId":"1395508560-19893-3-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 02/10] t4209: factor out helper function test_log()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:52Z","receivedAt":"2014-03-22T17:15:52Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Twelve tests in t4209 follow the same simple pattern for description,\ngit log call and checking.  Extract that shared logic into a helper\nfunction named test_log.  Test specifications become a lot more\ncompact, new tests can be added more easily.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t4209-log-pickaxe.sh | 92 +++++++++++++++++++-------------------------------\n 1 file changed, 34 insertions(+), 58 deletions(-)\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex ff668b5..9f3bb40 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -3,6 +3,28 @@\n test_description='log --grep/--author/--regexp-ignore-case/-S/-G'\n . ./test-lib.sh\n \n+test_log() {\n+\texpect=$1\n+\tkind=$2\n+\tneedle=$3\n+\tshift 3\n+\trest=$@\n+\n+\tcase $expect in\n+\texpect_nomatch)\n+\t\tmatch=nomatch\n+\t\t;;\n+\t*)\n+\t\tmatch=match\n+\t\t;;\n+\tesac\n+\n+\ttest_expect_success \"log $kind${rest:+ $rest} ($match)\" \"\n+\t\tgit log $rest $kind $needle --format=%H >actual &&\n+\t\ttest_cmp $expect actual\n+\t\"\n+}\n+\n test_expect_success setup '\n \t>expect_nomatch &&\n \n@@ -44,35 +66,12 @@ test_expect_success 'log --author -i' '\n \ttest_cmp expect_second actual\n '\n \n-test_expect_success 'log -G (nomatch)' '\n-\tgit log -Gpicked --format=%H >actual &&\n-\ttest_cmp expect_nomatch actual\n-'\n-\n-test_expect_success 'log -G (match)' '\n-\tgit log -GPicked --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n-\n-test_expect_success 'log -G --regexp-ignore-case (nomatch)' '\n-\tgit log --regexp-ignore-case -Gpickle --format=%H >actual &&\n-\ttest_cmp expect_nomatch actual\n-'\n-\n-test_expect_success 'log -G -i (nomatch)' '\n-\tgit log -i -Gpickle --format=%H >actual &&\n-\ttest_cmp expect_nomatch actual\n-'\n-\n-test_expect_success 'log -G --regexp-ignore-case (match)' '\n-\tgit log --regexp-ignore-case -Gpicked --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n-\n-test_expect_success 'log -G -i (match)' '\n-\tgit log -i -Gpicked --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n+test_log expect_nomatch -G picked\n+test_log expect_second  -G Picked\n+test_log expect_nomatch -G pickle --regexp-ignore-case\n+test_log expect_nomatch -G pickle -i\n+test_log expect_second  -G picked --regexp-ignore-case\n+test_log expect_second  -G picked -i\n \n test_expect_success 'log -G --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n@@ -87,35 +86,12 @@ test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n \trm .gitattributes\n '\n \n-test_expect_success 'log -S (nomatch)' '\n-\tgit log -Spicked --format=%H >actual &&\n-\ttest_cmp expect_nomatch actual\n-'\n-\n-test_expect_success 'log -S (match)' '\n-\tgit log -SPicked --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n-\n-test_expect_success 'log -S --regexp-ignore-case (match)' '\n-\tgit log --regexp-ignore-case -Spicked --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n-\n-test_expect_success 'log -S -i (match)' '\n-\tgit log -i -Spicked --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n-\n-test_expect_success 'log -S --regexp-ignore-case (nomatch)' '\n-\tgit log --regexp-ignore-case -Spickle --format=%H >actual &&\n-\ttest_cmp expect_nomatch actual\n-'\n-\n-test_expect_success 'log -S -i (nomatch)' '\n-\tgit log -i -Spickle --format=%H >actual &&\n-\ttest_cmp expect_nomatch actual\n-'\n+test_log expect_nomatch -S picked\n+test_log expect_second  -S Picked\n+test_log expect_second  -S picked --regexp-ignore-case\n+test_log expect_second  -S picked -i\n+test_log expect_nomatch -S pickle --regexp-ignore-case\n+test_log expect_nomatch -S pickle -i\n \n test_expect_success 'log -S --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-- \n1.9.1\n"},{"id":"237401","messageId":"1395508560-19893-4-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 03/10] t4209: factor out helper function test_log_icase()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:53Z","receivedAt":"2014-03-22T17:15:53Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Reduce code duplication by introducing test_log_icase() that runs the\nsame test with both --regexp-ignore-case and -i.  The specification of\nthe four basic test scenarios (matching/nomatching combined with case\nsensitive/insensitive) becomes easier to read and write.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t4209-log-pickaxe.sh | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex 9f3bb40..dd911c2 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -25,6 +25,11 @@ test_log() {\n \t\"\n }\n \n+test_log_icase() {\n+\ttest_log $@ --regexp-ignore-case\n+\ttest_log $@ -i\n+}\n+\n test_expect_success setup '\n \t>expect_nomatch &&\n \n@@ -66,12 +71,10 @@ test_expect_success 'log --author -i' '\n \ttest_cmp expect_second actual\n '\n \n-test_log expect_nomatch -G picked\n-test_log expect_second  -G Picked\n-test_log expect_nomatch -G pickle --regexp-ignore-case\n-test_log expect_nomatch -G pickle -i\n-test_log expect_second  -G picked --regexp-ignore-case\n-test_log expect_second  -G picked -i\n+test_log\texpect_nomatch\t-G picked\n+test_log\texpect_second\t-G Picked\n+test_log_icase\texpect_nomatch\t-G pickle\n+test_log_icase\texpect_second\t-G picked\n \n test_expect_success 'log -G --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n@@ -86,12 +89,10 @@ test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n \trm .gitattributes\n '\n \n-test_log expect_nomatch -S picked\n-test_log expect_second  -S Picked\n-test_log expect_second  -S picked --regexp-ignore-case\n-test_log expect_second  -S picked -i\n-test_log expect_nomatch -S pickle --regexp-ignore-case\n-test_log expect_nomatch -S pickle -i\n+test_log\texpect_nomatch\t-S picked\n+test_log\texpect_second\t-S Picked\n+test_log_icase\texpect_second\t-S picked\n+test_log_icase\texpect_nomatch\t-S pickle\n \n test_expect_success 'log -S --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-- \n1.9.1\n"},{"id":"237396","messageId":"1395508560-19893-5-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 04/10] t4209: use helper functions to test --grep","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:54Z","receivedAt":"2014-03-22T17:15:54Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Also add tests for non-matching cases.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t4209-log-pickaxe.sh | 18 ++++--------------\n 1 file changed, 4 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex dd911c2..873a10e 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -46,20 +46,10 @@ test_expect_success setup '\n \tgit rev-parse --verify HEAD >expect_second\n '\n \n-test_expect_success 'log --grep' '\n-\tgit log --grep=initial --format=%H >actual &&\n-\ttest_cmp expect_initial actual\n-'\n-\n-test_expect_success 'log --grep --regexp-ignore-case' '\n-\tgit log --regexp-ignore-case --grep=InItial --format=%H >actual &&\n-\ttest_cmp expect_initial actual\n-'\n-\n-test_expect_success 'log --grep -i' '\n-\tgit log -i --grep=InItial --format=%H >actual &&\n-\ttest_cmp expect_initial actual\n-'\n+test_log\texpect_initial\t--grep initial\n+test_log\texpect_nomatch\t--grep InItial\n+test_log_icase\texpect_initial\t--grep InItial\n+test_log_icase\texpect_nomatch\t--grep initail\n \n test_expect_success 'log --author --regexp-ignore-case' '\n \tgit log --regexp-ignore-case --author=person --format=%H >actual &&\n-- \n1.9.1\n"},{"id":"237398","messageId":"1395508560-19893-6-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 05/10] t4209: use helper functions to test --author","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:55Z","receivedAt":"2014-03-22T17:15:55Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Also add tests for case sensitive and non-matching cases.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t4209-log-pickaxe.sh | 13 ++++---------\n 1 file changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex 873a10e..80aabe2 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -51,15 +51,10 @@ test_log\texpect_nomatch\t--grep InItial\n test_log_icase\texpect_initial\t--grep InItial\n test_log_icase\texpect_nomatch\t--grep initail\n \n-test_expect_success 'log --author --regexp-ignore-case' '\n-\tgit log --regexp-ignore-case --author=person --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n-\n-test_expect_success 'log --author -i' '\n-\tgit log -i --author=person --format=%H >actual &&\n-\ttest_cmp expect_second actual\n-'\n+test_log\texpect_second\t--author Person\n+test_log\texpect_nomatch\t--author person\n+test_log_icase\texpect_second\t--author person\n+test_log_icase\texpect_nomatch\t--author spreon\n \n test_log\texpect_nomatch\t-G picked\n test_log\texpect_second\t-G Picked\n-- \n1.9.1\n"},{"id":"237397","messageId":"1395508560-19893-7-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 06/10] pickaxe: honor -i when used with -S and --pickaxe-regex","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:56Z","receivedAt":"2014-03-22T17:15:56Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"accccde4 (pickaxe: allow -i to search in patch case-insensitively)\nallowed case-insenitive matching for -G and -S, but for the latter\nonly if fixed string matching is used.  Allow it for -S and regular\nexpression matching as well to make the support complete.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n diffcore-pickaxe.c     | 5 ++++-\n t/t4209-log-pickaxe.sh | 5 +++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 401eb72..cb75851 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -237,7 +237,10 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \n \tif (opts & DIFF_PICKAXE_REGEX) {\n \t\tint err;\n-\t\terr = regcomp(&regex, needle, REG_EXTENDED | REG_NEWLINE);\n+\t\tint cflags = REG_EXTENDED | REG_NEWLINE;\n+\t\tif (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))\n+\t\t\tcflags |= REG_ICASE;\n+\t\terr = regcomp(&regex, needle, cflags);\n \t\tif (err) {\n \t\t\t/* The POSIX.2 people are surely sick */\n \t\t\tchar errbuf[1024];\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex 80aabe2..a31388f 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -79,6 +79,11 @@ test_log\texpect_second\t-S Picked\n test_log_icase\texpect_second\t-S picked\n test_log_icase\texpect_nomatch\t-S pickle\n \n+test_log\texpect_nomatch\t-S p.cked --pickaxe-regex\n+test_log\texpect_second\t-S P.cked --pickaxe-regex\n+test_log_icase\texpect_second\t-S p.cked --pickaxe-regex\n+test_log_icase\texpect_nomatch\t-S p.ckle --pickaxe-regex\n+\n test_expect_success 'log -S --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n \ttest_must_fail git -c diff.test.textconv=missing log -Sfoo &&\n-- \n1.9.1\n"},{"id":"237400","messageId":"1395508560-19893-8-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 07/10] pickaxe: merge diffcore_pickaxe_grep() and diffcore_pickaxe_count() into diffcore_pickaxe()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:57Z","receivedAt":"2014-03-22T17:15:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"diffcore_pickaxe_count() initializes the regular expression or kwset for\nthe search term, calls pickaxe() with the callback has_changes() and\ncleans up afterwards.  diffcore_pickaxe_grep() does the same, only it\ndoesn't support kwset and uses the callback diff_grep() instead.  Merge\nthe two functions to form the new diffcore_pickaxe() and thus get rid of\nthe duplicate regex setup and cleanup code.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n diffcore-pickaxe.c | 48 +++++++++---------------------------------------\n 1 file changed, 9 insertions(+), 39 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex cb75851..cfc4262 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -108,29 +108,6 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,\n \treturn ecbdata.hit;\n }\n \n-static void diffcore_pickaxe_grep(struct diff_options *o)\n-{\n-\tint err;\n-\tregex_t regex;\n-\tint cflags = REG_EXTENDED | REG_NEWLINE;\n-\n-\tif (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))\n-\t\tcflags |= REG_ICASE;\n-\n-\terr = regcomp(&regex, o->pickaxe, cflags);\n-\tif (err) {\n-\t\tchar errbuf[1024];\n-\t\tregerror(err, &regex, errbuf, 1024);\n-\t\tregfree(&regex);\n-\t\tdie(\"invalid regex: %s\", errbuf);\n-\t}\n-\n-\tpickaxe(&diff_queued_diff, o, &regex, NULL, diff_grep);\n-\n-\tregfree(&regex);\n-\treturn;\n-}\n-\n static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)\n {\n \tunsigned int cnt;\n@@ -227,7 +204,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,\n \treturn ret;\n }\n \n-static void diffcore_pickaxe_count(struct diff_options *o)\n+void diffcore_pickaxe(struct diff_options *o)\n {\n \tconst char *needle = o->pickaxe;\n \tint opts = o->pickaxe_opts;\n@@ -235,7 +212,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \tregex_t regex, *regexp = NULL;\n \tkwset_t kws = NULL;\n \n-\tif (opts & DIFF_PICKAXE_REGEX) {\n+\tif (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {\n \t\tint err;\n \t\tint cflags = REG_EXTENDED | REG_NEWLINE;\n \t\tif (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))\n@@ -256,20 +233,13 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\tkwsprep(kws);\n \t}\n \n-\tpickaxe(&diff_queued_diff, o, regexp, kws, has_changes);\n-\n-\tif (opts & DIFF_PICKAXE_REGEX)\n-\t\tregfree(&regex);\n-\telse\n-\t\tkwsfree(kws);\n-\treturn;\n-}\n-\n-void diffcore_pickaxe(struct diff_options *o)\n-{\n \t/* Might want to warn when both S and G are on; I don't care... */\n-\tif (o->pickaxe_opts & DIFF_PICKAXE_KIND_G)\n-\t\tdiffcore_pickaxe_grep(o);\n+\tpickaxe(&diff_queued_diff, o, regexp, kws,\n+\t\t(opts & DIFF_PICKAXE_KIND_G) ? diff_grep : has_changes);\n+\n+\tif (regexp)\n+\t\tregfree(regexp);\n \telse\n-\t\tdiffcore_pickaxe_count(o);\n+\t\tkwsfree(kws);\n+\treturn;\n }\n-- \n1.9.1\n"},{"id":"237399","messageId":"1395508560-19893-9-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 08/10] pickaxe: move pickaxe() after pickaxe_match()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:58Z","receivedAt":"2014-03-22T17:15:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"pickaxe() calls pickaxe_match(); moving the definition of the former\nthose after the latter allows us to do without an explicit function\ndeclaration.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n diffcore-pickaxe.c | 79 ++++++++++++++++++++++++++----------------------------\n 1 file changed, 38 insertions(+), 41 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex cfc4262..827a7d7 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -12,47 +12,6 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,\n \t\t\t  struct diff_options *o,\n \t\t\t  regex_t *regexp, kwset_t kws);\n \n-static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,\n-\t\t\t regex_t *regexp, kwset_t kws, pickaxe_fn fn);\n-\n-static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,\n-\t\t    regex_t *regexp, kwset_t kws, pickaxe_fn fn)\n-{\n-\tint i;\n-\tstruct diff_queue_struct outq;\n-\n-\tDIFF_QUEUE_CLEAR(&outq);\n-\n-\tif (o->pickaxe_opts & DIFF_PICKAXE_ALL) {\n-\t\t/* Showing the whole changeset if needle exists */\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (pickaxe_match(p, o, regexp, kws, fn))\n-\t\t\t\treturn; /* do not munge the queue */\n-\t\t}\n-\n-\t\t/*\n-\t\t * Otherwise we will clear the whole queue by copying\n-\t\t * the empty outq at the end of this function, but\n-\t\t * first clear the current entries in the queue.\n-\t\t */\n-\t\tfor (i = 0; i < q->nr; i++)\n-\t\t\tdiff_free_filepair(q->queue[i]);\n-\t} else {\n-\t\t/* Showing only the filepairs that has the needle */\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (pickaxe_match(p, o, regexp, kws, fn))\n-\t\t\t\tdiff_q(&outq, p);\n-\t\t\telse\n-\t\t\t\tdiff_free_filepair(p);\n-\t\t}\n-\t}\n-\n-\tfree(q->queue);\n-\t*q = outq;\n-}\n-\n struct diffgrep_cb {\n \tregex_t *regexp;\n \tint hit;\n@@ -204,6 +163,44 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,\n \treturn ret;\n }\n \n+static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,\n+\t\t    regex_t *regexp, kwset_t kws, pickaxe_fn fn)\n+{\n+\tint i;\n+\tstruct diff_queue_struct outq;\n+\n+\tDIFF_QUEUE_CLEAR(&outq);\n+\n+\tif (o->pickaxe_opts & DIFF_PICKAXE_ALL) {\n+\t\t/* Showing the whole changeset if needle exists */\n+\t\tfor (i = 0; i < q->nr; i++) {\n+\t\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t\tif (pickaxe_match(p, o, regexp, kws, fn))\n+\t\t\t\treturn; /* do not munge the queue */\n+\t\t}\n+\n+\t\t/*\n+\t\t * Otherwise we will clear the whole queue by copying\n+\t\t * the empty outq at the end of this function, but\n+\t\t * first clear the current entries in the queue.\n+\t\t */\n+\t\tfor (i = 0; i < q->nr; i++)\n+\t\t\tdiff_free_filepair(q->queue[i]);\n+\t} else {\n+\t\t/* Showing only the filepairs that has the needle */\n+\t\tfor (i = 0; i < q->nr; i++) {\n+\t\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t\tif (pickaxe_match(p, o, regexp, kws, fn))\n+\t\t\t\tdiff_q(&outq, p);\n+\t\t\telse\n+\t\t\t\tdiff_free_filepair(p);\n+\t\t}\n+\t}\n+\n+\tfree(q->queue);\n+\t*q = outq;\n+}\n+\n void diffcore_pickaxe(struct diff_options *o)\n {\n \tconst char *needle = o->pickaxe;\n-- \n1.9.1\n"},{"id":"237404","messageId":"1395508560-19893-10-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 09/10] pickaxe: call strlen only when necessary in diffcore_pickaxe_count()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:15:59Z","receivedAt":"2014-03-22T17:15:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"We need to determine the search term's length only when fixed-string\nmatching is used; regular expression compilation takes a NUL-terminated\nstring directly.  Only call strlen() in the former case.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n diffcore-pickaxe.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 827a7d7..70753d0 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -205,7 +205,6 @@ void diffcore_pickaxe(struct diff_options *o)\n {\n \tconst char *needle = o->pickaxe;\n \tint opts = o->pickaxe_opts;\n-\tunsigned long len = strlen(needle);\n \tregex_t regex, *regexp = NULL;\n \tkwset_t kws = NULL;\n \n@@ -226,7 +225,7 @@ void diffcore_pickaxe(struct diff_options *o)\n \t} else {\n \t\tkws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)\n \t\t\t       ? tolower_trans_tbl : NULL);\n-\t\tkwsincr(kws, needle, len);\n+\t\tkwsincr(kws, needle, strlen(needle));\n \t\tkwsprep(kws);\n \t}\n \n-- \n1.9.1\n"},{"id":"237405","messageId":"1395508560-19893-11-git-send-email-l.s.r@web.de","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"[PATCH 10/10] pickaxe: simplify kwset loop in contains()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-22T17:16:00Z","receivedAt":"2014-03-22T17:16:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Inlining the variable \"found\" actually makes the code shorter and\neasier to read.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n diffcore-pickaxe.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 70753d0..185f86b 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -94,13 +94,10 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)\n \t\twhile (sz) {\n \t\t\tstruct kwsmatch kwsm;\n \t\t\tsize_t offset = kwsexec(kws, data, sz, &kwsm);\n-\t\t\tconst char *found;\n \t\t\tif (offset == -1)\n \t\t\t\tbreak;\n-\t\t\telse\n-\t\t\t\tfound = data + offset;\n-\t\t\tsz -= found - data + kwsm.size[0];\n-\t\t\tdata = found + kwsm.size[0];\n+\t\t\tsz -= offset + kwsm.size[0];\n+\t\t\tdata += offset + kwsm.size[0];\n \t\t\tcnt++;\n \t\t}\n \t}\n-- \n1.9.1\n"},{"id":"237480","messageId":"xmqqlhvzqx1r.fsf@gitster.dls.corp.google.com","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"Re: [PATCH 00/10] pickaxe: honor -i when used with -S and --pickaxe-regex; cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-24T18:19:28Z","receivedAt":"2014-03-24T18:19:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> This series allows the options -i/--regexp-ignore-case, --pickaxe-regex,\n> and -S to be used together and work as expected to perform a pickaxe\n> search using case-insensitive regular expression matching.  Its first\n> half refactors the test script and extends test coverage a bit while\n> we're at it.  The actual change is in the sixth patch.  It enables the\n> two following cleanups.  The last two patches are independent simple\n> cleanups.\n\nVery nice.  Thanks.\n"},{"id":"237481","messageId":"xmqqa9cfqwwp.fsf@gitster.dls.corp.google.com","threadId":"36275","inReplyTo":"1395508560-19893-4-git-send-email-l.s.r@web.de","subject":"Re: [PATCH 03/10] t4209: factor out helper function test_log_icase()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-24T18:22:30Z","receivedAt":"2014-03-24T18:22:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Reduce code duplication by introducing test_log_icase() that runs the\n> same test with both --regexp-ignore-case and -i.  The specification of\n> the four basic test scenarios (matching/nomatching combined with case\n> sensitive/insensitive) becomes easier to read and write.\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  t/t4209-log-pickaxe.sh | 25 +++++++++++++------------\n>  1 file changed, 13 insertions(+), 12 deletions(-)\n>\n> diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\n> index 9f3bb40..dd911c2 100755\n> --- a/t/t4209-log-pickaxe.sh\n> +++ b/t/t4209-log-pickaxe.sh\n> @@ -25,6 +25,11 @@ test_log() {\n>  \t\"\n>  }\n>  \n> +test_log_icase() {\n> +\ttest_log $@ --regexp-ignore-case\n> +\ttest_log $@ -i\n\n&&-cascade broken?  Will squash in an obvious fix.\n\n> +}\n"},{"id":"237482","messageId":"xmqq38i7qwvx.fsf@gitster.dls.corp.google.com","threadId":"36275","inReplyTo":"1395508560-19893-5-git-send-email-l.s.r@web.de","subject":"Re: [PATCH 04/10] t4209: use helper functions to test --grep","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-24T18:22:58Z","receivedAt":"2014-03-24T18:22:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> -test_expect_success 'log --grep -i' '\n> -\tgit log -i --grep=InItial --format=%H >actual &&\n> -\ttest_cmp expect_initial actual\n> -'\n> +test_log\texpect_initial\t--grep initial\n> +test_log\texpect_nomatch\t--grep InItial\n\nThis, and the next --author one, assumes that we will never break\n\"--grep=foo\" without breaking \"--grep foo\".  That should be OK, but\nwe might want to add separate tests e.g.\n\n\ttest_log expect_initial --grep=initial\n\nperhaps?  I dunno.\n\nQueued without any tweaks for now.  Thanks.\n"},{"id":"237489","messageId":"20140324211006.GA13728@sigill.intra.peff.net","threadId":"36275","inReplyTo":"xmqqa9cfqwwp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/10] t4209: factor out helper function test_log_icase()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-24T21:10:06Z","receivedAt":"2014-03-24T21:10:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 24, 2014 at 11:22:30AM -0700, Junio C Hamano wrote:\n\n> > +test_log_icase() {\n> > +\ttest_log $@ --regexp-ignore-case\n> > +\ttest_log $@ -i\n> \n> &&-cascade broken?  Will squash in an obvious fix.\n\nI don't think so. This is happening outside of test_expect_success,\nwhich is run by test_log. So adding a && means that if the first test\nfails, we do not bother to run the second one at all, which is not what\nwe want.\n\n-Peff\n"},{"id":"237490","messageId":"20140324211412.GB13728@sigill.intra.peff.net","threadId":"36275","inReplyTo":"xmqq38i7qwvx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 04/10] t4209: use helper functions to test --grep","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-24T21:14:12Z","receivedAt":"2014-03-24T21:14:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 24, 2014 at 11:22:58AM -0700, Junio C Hamano wrote:\n\n> René Scharfe <l.s.r@web.de> writes:\n> \n> > -test_expect_success 'log --grep -i' '\n> > -\tgit log -i --grep=InItial --format=%H >actual &&\n> > -\ttest_cmp expect_initial actual\n> > -'\n> > +test_log\texpect_initial\t--grep initial\n> > +test_log\texpect_nomatch\t--grep InItial\n> \n> This, and the next --author one, assumes that we will never break\n> \"--grep=foo\" without breaking \"--grep foo\".  That should be OK, but\n> we might want to add separate tests e.g.\n> \n> \ttest_log expect_initial --grep=initial\n> \n> perhaps?  I dunno.\n\nYeah, I I'd prefer \"--grep=\" here (and in all scripts).  In general, I\nthink our attitude should be that \"--foo=bar\" is guaranteed to work\nforever, but \"--foo bar\" is not. The latter only works if the argument\nis non-optional, so that leaves us room to \"break\" compatibility to make\nan argument optional in a future version.\n\nNow, whether the rest of the world and its script-writers are aware of\nthis fact, I don't know. So it may be too late already (but it does look\nlike we mention it in gitcli(7)).\n\n-Peff\n"},{"id":"237493","messageId":"20140324212101.GC13728@sigill.intra.peff.net","threadId":"36275","inReplyTo":"1395508560-19893-9-git-send-email-l.s.r@web.de","subject":"Re: [PATCH 08/10] pickaxe: move pickaxe() after pickaxe_match()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-24T21:21:02Z","receivedAt":"2014-03-24T21:21:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 22, 2014 at 06:15:58PM +0100, René Scharfe wrote:\n\n> pickaxe() calls pickaxe_match(); moving the definition of the former\n> those after the latter allows us to do without an explicit function\n> declaration.\n\ns/those //\n\n-Peff\n"},{"id":"237495","messageId":"20140324212253.GD13728@sigill.intra.peff.net","threadId":"36275","inReplyTo":"1395508560-19893-1-git-send-email-l.s.r@web.de","subject":"Re: [PATCH 00/10] pickaxe: honor -i when used with -S and --pickaxe-regex; cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-24T21:22:53Z","receivedAt":"2014-03-24T21:22:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 22, 2014 at 06:15:50PM +0100, René Scharfe wrote:\n\n> This series allows the options -i/--regexp-ignore-case, --pickaxe-regex,\n> and -S to be used together and work as expected to perform a pickaxe\n> search using case-insensitive regular expression matching.  Its first\n> half refactors the test script and extends test coverage a bit while\n> we're at it.  The actual change is in the sixth patch.  It enables the\n> two following cleanups.  The last two patches are independent simple\n> cleanups.\n\nThis all looks very nice.\n\nUpon reading your cover letter, I wondered if somebody would ever want\ncase-sensitive \"--grep\", but case-insensitive pickaxe (or vice versa).\nHowever, you are not adding case-insensitivity to -G/-S, but rather\napplying it more consistently. So I think that ship has already sailed\nanyway (and even if it did not, I think applying \"-i\" everywhere makes\nthe interface simpler for the common cases, so the loss of flexibility\nmay be worth it).\n\n-Peff\n"},{"id":"237503","messageId":"5330A6E2.2040607@web.de","threadId":"36275","inReplyTo":"20140324211412.GB13728@sigill.intra.peff.net","subject":"Re: [PATCH 04/10] t4209: use helper functions to test --grep","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-24T21:42:58Z","receivedAt":"2014-03-24T21:42:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.03.2014 22:14, schrieb Jeff King:\n> On Mon, Mar 24, 2014 at 11:22:58AM -0700, Junio C Hamano wrote:\n> \n>> René Scharfe <l.s.r@web.de> writes:\n>>\n>>> -test_expect_success 'log --grep -i' '\n>>> -\tgit log -i --grep=InItial --format=%H >actual &&\n>>> -\ttest_cmp expect_initial actual\n>>> -'\n>>> +test_log\texpect_initial\t--grep initial\n>>> +test_log\texpect_nomatch\t--grep InItial\n>>\n>> This, and the next --author one, assumes that we will never break\n>> \"--grep=foo\" without breaking \"--grep foo\".  That should be OK, but\n>> we might want to add separate tests e.g.\n>>\n>> \ttest_log expect_initial --grep=initial\n>>\n>> perhaps?  I dunno.\n> \n> Yeah, I I'd prefer \"--grep=\" here (and in all scripts).  In general, I\n> think our attitude should be that \"--foo=bar\" is guaranteed to work\n> forever, but \"--foo bar\" is not. The latter only works if the argument\n> is non-optional, so that leaves us room to \"break\" compatibility to make\n> an argument optional in a future version.\n> \n> Now, whether the rest of the world and its script-writers are aware of\n> this fact, I don't know. So it may be too late already (but it does look\n> like we mention it in gitcli(7)).\n\nOK, then the following should be squashed into patch 2 (t4209: factor out\nhelper function test_log()):\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex 9f3bb40..f47231a 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -10,6 +10,14 @@ test_log() {\n \tshift 3\n \trest=$@\n \n+\tcase $kind in\n+\t--*)\n+\t\topt=$kind=$needle\n+\t\t;;\n+\t*)\n+\t\topt=$kind$needle\n+\t\t;;\n+\tesac\n \tcase $expect in\n \texpect_nomatch)\n \t\tmatch=nomatch\n@@ -20,7 +28,7 @@ test_log() {\n \tesac\n \n \ttest_expect_success \"log $kind${rest:+ $rest} ($match)\" \"\n-\t\tgit log $rest $kind $needle --format=%H >actual &&\n+\t\tgit log $rest $opt --format=%H >actual &&\n \t\ttest_cmp $expect actual\n \t\"\n }\n"},{"id":"237504","messageId":"5330A76E.7060907@web.de","threadId":"36275","inReplyTo":"20140324211006.GA13728@sigill.intra.peff.net","subject":"Re: [PATCH 03/10] t4209: factor out helper function test_log_icase()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-03-24T21:45:18Z","receivedAt":"2014-03-24T21:45:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.03.2014 22:10, schrieb Jeff King:\n> On Mon, Mar 24, 2014 at 11:22:30AM -0700, Junio C Hamano wrote:\n>\n>>> +test_log_icase() {\n>>> +\ttest_log $@ --regexp-ignore-case\n>>> +\ttest_log $@ -i\n>>\n>> &&-cascade broken?  Will squash in an obvious fix.\n>\n> I don't think so. This is happening outside of test_expect_success,\n> which is run by test_log. So adding a && means that if the first test\n> fails, we do not bother to run the second one at all, which is not what\n> we want.\n\nRight; this function runs two independent tests and && is left out \nintentionally.\n\nRené\n"},{"id":"237513","messageId":"xmqqsiq7nt8x.fsf@gitster.dls.corp.google.com","threadId":"36275","inReplyTo":"5330A76E.7060907@web.de","subject":"Re: [PATCH 03/10] t4209: factor out helper function test_log_icase()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-24T22:09:50Z","receivedAt":"2014-03-24T22:09:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Am 24.03.2014 22:10, schrieb Jeff King:\n>> On Mon, Mar 24, 2014 at 11:22:30AM -0700, Junio C Hamano wrote:\n>>\n>>>> +test_log_icase() {\n>>>> +\ttest_log $@ --regexp-ignore-case\n>>>> +\ttest_log $@ -i\n>>>\n>>> &&-cascade broken?  Will squash in an obvious fix.\n>>\n>> I don't think so. This is happening outside of test_expect_success,\n>> which is run by test_log. So adding a && means that if the first test\n>> fails, we do not bother to run the second one at all, which is not what\n>> we want.\n>\n> Right; this function runs two independent tests and && is left out\n> intentionally.\n\nAhh, sorry and thanks.\n"}]}