{"thread":{"id":"29784","subject":"[PATCH v2 0/2] \"log --regexp-ignore-case -S/-G<string>\"","startedAt":"2012-02-29T00:20:29Z","lastAt":"2012-02-29T18:05:34Z","messageCount":10,"participants":["Junio C Hamano","Jeff King","Thomas Rast"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"185684","messageId":"1330474831-9030-1-git-send-email-gitster@pobox.com","threadId":"29784","inReplyTo":null,"subject":"[PATCH v2 0/2] \"log --regexp-ignore-case -S/-G<string>\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T00:20:29Z","receivedAt":"2012-02-29T00:20:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is a re-roll of the previous attempt to teach pickaxe to match\ncase insensitively.\n\nJunio C Hamano (2):\n  grep: use static trans-case table\n  pickaxe: allow -i to search in patch case-insensitively\n\n cache.h                |    3 ++\n ctype.c                |   36 +++++++++++++++\n diff.h                 |    1 +\n diffcore-pickaxe.c     |    9 +++-\n grep.c                 |   11 ++---\n revision.c             |    1 +\n t/t4209-log-pickaxe.sh |  119 ++++++++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 170 insertions(+), 10 deletions(-)\n create mode 100755 t/t4209-log-pickaxe.sh\n\n-- \n1.7.9.2.344.g3e8c86\n"},{"id":"185685","messageId":"1330474831-9030-2-git-send-email-gitster@pobox.com","threadId":"29784","inReplyTo":"1330474831-9030-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 1/2] grep: use static trans-case table","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T00:20:30Z","receivedAt":"2012-02-29T00:20:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In order to prepare the kwset machinery for a case-insensitive search, we\nused to use a static table of 256 elements and filled it every time before\ncalling kwsalloc().  Because the kwset machinery will never modify this\ntable, just allocate a single instance globally and fill it at the compile\ntime.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache.h |    3 +++\n ctype.c |   36 ++++++++++++++++++++++++++++++++++++\n grep.c  |   11 +++--------\n 3 files changed, 42 insertions(+), 8 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 79c612f..79dc305 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1258,4 +1258,7 @@ extern struct startup_info *startup_info;\n /* builtin/merge.c */\n int checkout_fast_forward(const unsigned char *from, const unsigned char *to);\n \n+/* in ctype.c, for kwset users */\n+extern const char tolower_trans_tbl[256];\n+\n #endif /* CACHE_H */\ndiff --git a/ctype.c b/ctype.c\nindex b5d856f..7c14d85 100644\n--- a/ctype.c\n+++ b/ctype.c\n@@ -25,3 +25,39 @@ unsigned char sane_ctype[256] = {\n \tA, A, A, A, A, A, A, A, A, A, A, R, R, 0, P, 0,\t\t/* 112..127 */\n \t/* Nothing in the 128.. range */\n };\n+\n+/* For case-insensitive kwset */\n+const char tolower_trans_tbl[256] = {\n+\t0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,\n+\t0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,\n+\t0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,\n+\t0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,\n+\t ' ',  '!',  '\"',  '#',  '$',  '%',  '&', 0x27,\n+\t '(',  ')',  '*',  '+',  ',',  '-',  '.',  '/',\n+\t '0',  '1',  '2',  '3',  '4',  '5',  '6',  '7',\n+\t '8',  '9',  ':',  ';',  '<',  '=',  '>',  '?',\n+\t '@',  'a',  'b',  'c',  'd',  'e',  'f',  'g',\n+\t 'h',  'i',  'j',  'k',  'l',  'm',  'n',  'o',\n+\t 'p',  'q',  'r',  's',  't',  'u',  'v',  'w',\n+\t 'x',  'y',  'z',  '[', 0x5c,  ']',  '^',  '_',\n+\t '`',  'a',  'b',  'c',  'd',  'e',  'f',  'g',\n+\t 'h',  'i',  'j',  'k',  'l',  'm',  'n',  'o',\n+\t 'p',  'q',  'r',  's',  't',  'u',  'v',  'w',\n+\t 'x',  'y',  'z',  '{',  '|',  '}',  '~', 0x7f,\n+\t0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,\n+\t0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f,\n+\t0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97,\n+\t0x98, 0x99, 0x9a, 0x9b, 0x9c, 0x9d, 0x9e, 0x9f,\n+\t0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,\n+\t0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf,\n+\t0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5, 0xb6, 0xb7,\n+\t0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbd, 0xbe, 0xbf,\n+\t0xc0, 0xc1, 0xc2, 0xc3, 0xc4, 0xc5, 0xc6, 0xc7,\n+\t0xc8, 0xc9, 0xca, 0xcb, 0xcc, 0xcd, 0xce, 0xcf,\n+\t0xd0, 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7,\n+\t0xd8, 0xd9, 0xda, 0xdb, 0xdc, 0xdd, 0xde, 0xdf,\n+\t0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xe7,\n+\t0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef,\n+\t0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,\n+\t0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff,\n+};\ndiff --git a/grep.c b/grep.c\nindex b29d09c..1030f38 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -168,15 +168,10 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \t\tp->fixed = 0;\n \n \tif (p->fixed) {\n-\t\tif (opt->regflags & REG_ICASE || p->ignore_case) {\n-\t\t\tstatic char trans[256];\n-\t\t\tint i;\n-\t\t\tfor (i = 0; i < 256; i++)\n-\t\t\t\ttrans[i] = tolower(i);\n-\t\t\tp->kws = kwsalloc(trans);\n-\t\t} else {\n+\t\tif (opt->regflags & REG_ICASE || p->ignore_case)\n+\t\t\tp->kws = kwsalloc(tolower_trans_tbl);\n+\t\telse\n \t\t\tp->kws = kwsalloc(NULL);\n-\t\t}\n \t\tkwsincr(p->kws, p->pattern, p->patternlen);\n \t\tkwsprep(p->kws);\n \t\treturn;\n-- \n1.7.9.2.344.g3e8c86\n"},{"id":"185686","messageId":"1330474831-9030-3-git-send-email-gitster@pobox.com","threadId":"29784","inReplyTo":"1330474831-9030-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 2/2] pickaxe: allow -i to search in patch case-insensitively","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T00:20:31Z","receivedAt":"2012-02-29T00:20:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git log -S<string>\" is a useful way to find the last commit in the\ncodebase that touched the <string>. As it was designed to be used by a\nporcelain script to dig the history starting from a block of text that\nappear in the starting commit, it never had to look for anything but an\nexact match.\n\nWhen used by an end user who wants to look for the last commit that\nremoved a string (e.g. name of a variable) that he vaguely remembers,\nhowever, it is useful to support case insensitive match.\n\nWhen given the \"--regexp-ignore-case\" (or \"-i\") option, which originally\nwas designed to affect case sensitivity of the search done in the commit\nlog part, e.g. \"log --grep\", the matches made with -S/-G pickaxe search is\ndone case insensitively now.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.h                 |    1 +\n diffcore-pickaxe.c     |    9 +++-\n revision.c             |    1 +\n t/t4209-log-pickaxe.sh |  119 ++++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 128 insertions(+), 2 deletions(-)\n create mode 100755 t/t4209-log-pickaxe.sh\n\ndiff --git a/diff.h b/diff.h\nindex 0c51724..436b574 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -80,6 +80,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)\n #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)\n #define DIFF_OPT_FUNCCONTEXT         (1 << 29)\n+#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n \n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 380a837..ed23eb4 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -138,8 +138,12 @@ 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-\terr = regcomp(&regex, o->pickaxe, REG_EXTENDED | REG_NEWLINE);\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@@ -237,7 +241,8 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\t}\n \t\tregexp = &regex;\n \t} else {\n-\t\tkws = kwsalloc(NULL);\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\tkwsprep(kws);\n \t}\ndiff --git a/revision.c b/revision.c\nindex 8764dde..971b7dc 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1559,6 +1559,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->grep_filter.regflags |= REG_EXTENDED;\n \t} else if (!strcmp(arg, \"--regexp-ignore-case\") || !strcmp(arg, \"-i\")) {\n \t\trevs->grep_filter.regflags |= REG_ICASE;\n+\t\tDIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);\n \t} else if (!strcmp(arg, \"--fixed-strings\") || !strcmp(arg, \"-F\")) {\n \t\trevs->grep_filter.fixed = 1;\n \t} else if (!strcmp(arg, \"--all-match\")) {\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nnew file mode 100755\nindex 0000000..eed7273\n--- /dev/null\n+++ b/t/t4209-log-pickaxe.sh\n@@ -0,0 +1,119 @@\n+#!/bin/sh\n+\n+test_description='log --grep/--author/--regexp-ignore-case/-S/-G'\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t>file &&\n+\tgit add file &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\n+\techo Picked >file &&\n+\ttest_tick &&\n+\tgit commit -a --author=\"Another Person <another@example.com>\" -m 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+'\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+'\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+'\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+'\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+'\n+\n+test_expect_success 'log -G (nomatch)' '\n+\tgit log -Gpicked --format=%H >actual &&\n+\t>expect &&\n+\ttest_cmp expect 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+'\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+'\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+'\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+'\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+'\n+\n+test_expect_success 'log -S (nomatch)' '\n+\tgit log -Spicked --format=%H >actual &&\n+\t>expect &&\n+\ttest_cmp expect 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+'\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+'\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+'\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+'\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+'\n+\n+test_done\n-- \n1.7.9.2.344.g3e8c86\n"},{"id":"185707","messageId":"20120229082814.GB14181@sigill.intra.peff.net","threadId":"29784","inReplyTo":"1330474831-9030-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v2 1/2] grep: use static trans-case table","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-29T08:28:14Z","receivedAt":"2012-02-29T08:28:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 28, 2012 at 04:20:30PM -0800, Junio C Hamano wrote:\n\n> In order to prepare the kwset machinery for a case-insensitive search, we\n> used to use a static table of 256 elements and filled it every time before\n> calling kwsalloc().  Because the kwset machinery will never modify this\n> table, just allocate a single instance globally and fill it at the compile\n> time.\n\nHmm. I was going to complain that the original code used tolower() to\ngenerate the table at run-time, and therefore respected the current\nlocale. But of course we have replaced tolower() with a\nlocale-independent version, so it should behave identically.\n\nBut that does make me wonder. Do people expect their case-insensitive\nsearches to work on non-ASCII characters? I would think yes, but I do\nnot use non-ASCII characters in the first place, so my opinion may not\nmean much.\n\nFor that matter, does REG_ICASE respect locales? The glibc code appears\nto consider it, but I couldn't make it work in some simple tests. But if\nit does, that raises another weirdness: we fall back to kwset\ntransparently when a grep pattern contains no metacharacters. So you\nwould get different results for \"-i --grep=é\" versus \"-i --grep=é.*\".\n\nOf course, even if we used a locale-respecting version of tolower in the\noriginal code, I suspect that a byte table would be fundamentally\ninsufficient, anyway, in the face of multi-byte encodings like utf8.\n\nSo I don't think your patch is making the problem any worse. And even if\nsomebody wants to tackle the problem later, the solution would look so\nunlike the original code that your change is not hurting their effort.\n\n-Peff\n"},{"id":"185708","messageId":"20120229083534.GC14181@sigill.intra.peff.net","threadId":"29784","inReplyTo":"1330474831-9030-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v2 2/2] pickaxe: allow -i to search in patch case-insensitively","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-29T08:35:34Z","receivedAt":"2012-02-29T08:35:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 28, 2012 at 04:20:31PM -0800, Junio C Hamano wrote:\n\n> \"git log -S<string>\" is a useful way to find the last commit in the\n> codebase that touched the <string>. As it was designed to be used by a\n> porcelain script to dig the history starting from a block of text that\n> appear in the starting commit, it never had to look for anything but an\n> exact match.\n> \n> When used by an end user who wants to look for the last commit that\n> removed a string (e.g. name of a variable) that he vaguely remembers,\n> however, it is useful to support case insensitive match.\n> \n> When given the \"--regexp-ignore-case\" (or \"-i\") option, which originally\n> was designed to affect case sensitivity of the search done in the commit\n> log part, e.g. \"log --grep\", the matches made with -S/-G pickaxe search is\n> done case insensitively now.\n\nI can't imagine anybody would want to have different case-sensitivity\noptions for grepping the commit message versus pickaxe. But even if they\ndo, and we later add options to control them individually, we would\nstill want the short-and-sweet \"-i\" to cover the common case of setting\nboth. So I think the approach is good.\n\nThe patch itself looks fine to me.\n\n-Peff\n"},{"id":"185711","messageId":"7v399uuio0.fsf@alter.siamese.dyndns.org","threadId":"29784","inReplyTo":"20120229082814.GB14181@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/2] grep: use static trans-case table","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T08:51:11Z","receivedAt":"2012-02-29T08:51:11Z","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> So I don't think your patch is making the problem any worse. And even if\n> somebody wants to tackle the problem later, the solution would look so\n> unlike the original code that your change is not hurting their effort.\n\nYes.\n"},{"id":"185713","messageId":"7vy5rmt3w8.fsf@alter.siamese.dyndns.org","threadId":"29784","inReplyTo":"20120229083534.GC14181@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/2] pickaxe: allow -i to search in patch case-insensitively","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T08:55:35Z","receivedAt":"2012-02-29T08:55:35Z","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>> When given the \"--regexp-ignore-case\" (or \"-i\") option, which originally\n>> was designed to affect case sensitivity of the search done in the commit\n>> log part, e.g. \"log --grep\", the matches made with -S/-G pickaxe search is\n>> done case insensitively now.\n>\n> I can't imagine anybody would want to have different case-sensitivity\n> options for grepping the commit message versus pickaxe. But even if they\n> do, and we later add options to control them individually, we would\n> still want the short-and-sweet \"-i\" to cover the common case of setting\n> both. So I think the approach is good.\n\nWhat you didn't read in the above is that the devilq around \"-i\" is not in\nthe case insensitivity switch between log-part grep (--grep/--author) and\npatch part grep (-S/-G), but how it interacts with generating the patch\npart case insensitively (i.e. \"log -p --ignore-case\", which is also \"-i\").\n\nYou can see what I decided to do in an evil merge in 'pu'.\n\nIn short,\n\n  * The short-and-sweet \"-i\" means both --regexp-ignore-case (grep) and\n    --ignore-case (diff); and\n\n  * The long-hand can be used to ask for case\n    insensitive grep but case sensitive patch, or vice versa.\n"},{"id":"185714","messageId":"20120229091855.GE14181@sigill.intra.peff.net","threadId":"29784","inReplyTo":"7vy5rmt3w8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/2] pickaxe: allow -i to search in patch case-insensitively","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-29T09:18:55Z","receivedAt":"2012-02-29T09:18:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 29, 2012 at 12:55:35AM -0800, Junio C Hamano wrote:\n\n> > I can't imagine anybody would want to have different case-sensitivity\n> > options for grepping the commit message versus pickaxe. But even if they\n> > do, and we later add options to control them individually, we would\n> > still want the short-and-sweet \"-i\" to cover the common case of setting\n> > both. So I think the approach is good.\n> \n> What you didn't read in the above is that the devilq around \"-i\" is not in\n> the case insensitivity switch between log-part grep (--grep/--author) and\n> patch part grep (-S/-G), but how it interacts with generating the patch\n> part case insensitively (i.e. \"log -p --ignore-case\", which is also \"-i\").\n\nHmm. So there are actually three potential options to flip. However, I\nthink the reasoning above is still sound. We could later split\n--regexp-ignore-case into two sub-options if we wanted (but like I said,\nI doubt anybody will want that; I was more concerned with making sure\nthat if somebody _does_ want it, we have not painted ourselves into a\ncorner).\n\n> You can see what I decided to do in an evil merge in 'pu'.\n> \n> In short,\n> \n>   * The short-and-sweet \"-i\" means both --regexp-ignore-case (grep) and\n>     --ignore-case (diff); and\n> \n>   * The long-hand can be used to ask for case\n>     insensitive grep but case sensitive patch, or vice versa.\n\nYes, the evil merge looks sane, assuming both topics implement the\ndesired behavior.\n\nI am a little dubious of the decision in jc/diff-ignore-case to have\n\"-i\" imply \"--ignore-case\". For \"git diff\", it makes perfect sense. But\nfor \"git log\", it feels wrong. Ignoring case for the regexps is very\ncommon, and ignoring case for the diffs is uncommon (it is, after all, a\nfeature we have gone many years without, and I don't remember anyone\nbringing it up until recently).\n\nAs a user, I would be surprised that something common like \"git log\n--author=junio -i -p\" would change diff generation between versions of\ngit.  It's probably not a huge regression, as patches which actually\nlook different with --ignore-case are relatively rare, so you are\nunlikely to see any difference if you trigger it accidentally. But that\njust argues to me that it is a feature that one would want to turn on\nexplicitly, anyway.\n\n-Peff\n"},{"id":"185721","messageId":"87wr75q34f.fsf@thomas.inf.ethz.ch","threadId":"29784","inReplyTo":"20120229091855.GE14181@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/2] pickaxe: allow -i to search in patch case-insensitively","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-29T11:40:32Z","receivedAt":"2012-02-29T11:40:32Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I am a little dubious of the decision in jc/diff-ignore-case to have\n> \"-i\" imply \"--ignore-case\". For \"git diff\", it makes perfect sense. But\n> for \"git log\", it feels wrong. Ignoring case for the regexps is very\n> common, and ignoring case for the diffs is uncommon (it is, after all, a\n> feature we have gone many years without, and I don't remember anyone\n> bringing it up until recently).\n\nDoubly so because (to the best of my list-reading efforts) when it was\nbrought up recently, the interpretation as \"case-insensitive diff\ngeneration\" was by mistake/misreading.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"185726","messageId":"7vpqcxtt01.fsf@alter.siamese.dyndns.org","threadId":"29784","inReplyTo":"20120229091855.GE14181@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/2] pickaxe: allow -i to search in patch case-insensitively","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T18:05:34Z","receivedAt":"2012-02-29T18:05:34Z","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> Hmm. So there are actually three potential options to flip. However, I\n> think the reasoning above is still sound.\n\nYes, the grep side of the things is easier to understand and explain if\nonly one option controlled the case insensitivity or whatever aspect of\nthe search: \"It is like searching things in 'log -p' output!\"\n"}]}