Re: [PATCHv2 7/7] git grep: honor textconv by default
- From
Michael J Gruber <git@drmicha.warpmail.net>
- Date
- Apr 24, 2013, 10:05 UTC
- Message-ID
- <5177AE7F.1040400@drmicha.warpmail.net>
- In-Reply-To
- <7vwqrtjmtx.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 23.04.2013 17:20:
Show 13 quoted lines
> Michael J Gruber <git@drmicha.warpmail.net> writes: > >> Currently, "git grep" does not honor textconv settings by default. >> Make it honor them by default just like "git log --grep" does. > > "git log --grep" looks for strings in the log message which never > goes through textconv filters. > > Puzzled.... > > If you meant -S/-G, it justifies use of textconv because we are > generating diff and the user defines textconv to get a reasonable > output for otherwise undiffable contents.
Sorry, yes, I meant "log grep diff", aka "log -S/-G".
> I do not know if it is sensible to apply textconv by default for > "grep" (or for that matter "git show" that gives blob contents).
Well, that is the discussion that we were having, with no real end result, which is why I haven't implemented this differently yet.
My point is that we apply textconv on "log diff greps" already, so why should't we on content greps?
The question is really whether we should treat "content" similar to "diff", that's question both when comparing "git log -S" to "git grep" and "git show <commit>" to "git show <blob>".
My choice is clear, but others seem torn.
For "git grep", implementing a "no-textconv" default is simple, but for "git show <blob>" this appears to be cumbersome to me.
Show 65 quoted lines
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> >> --- >> Documentation/git-grep.txt | 2 +- >> grep.c | 2 ++ >> t/t7008-grep-binary.sh | 4 ++-- >> 3 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt >> index a5c5a27..f54ac0c 100644 >> --- a/Documentation/git-grep.txt >> +++ b/Documentation/git-grep.txt >> @@ -82,10 +82,10 @@ OPTIONS >> >> --textconv:: >> Honor textconv filter settings. >> + This is the default. >> >> --no-textconv:: >> Do not honor textconv filter settings. >> - This is the default. >> >> -i:: >> --ignore-case:: >> diff --git a/grep.c b/grep.c >> index c668034..161d3f0 100644 >> --- a/grep.c >> +++ b/grep.c >> @@ -31,6 +31,7 @@ void init_grep_defaults(void) >> opt->max_depth = -1; >> opt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED; >> opt->extended_regexp_option = 0; >> + opt->allow_textconv = 1; >> strcpy(opt->color_context, ""); >> strcpy(opt->color_filename, ""); >> strcpy(opt->color_function, ""); >> @@ -134,6 +135,7 @@ void grep_init(struct grep_opt *opt, const char *prefix) >> opt->pathname = def->pathname; >> opt->regflags = def->regflags; >> opt->relative = def->relative; >> + opt->allow_textconv = def->allow_textconv; >> >> strcpy(opt->color_context, def->color_context); >> strcpy(opt->color_filename, def->color_filename); >> diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh >> index 10b2c8b..2fc9d9c 100755 >> --- a/t/t7008-grep-binary.sh >> +++ b/t/t7008-grep-binary.sh >> @@ -156,7 +156,7 @@ test_expect_success 'setup textconv filters' ' >> git config diff.foo.textconv "\"$(pwd)\""/nul_to_q_textconv >> ' >> >> -test_expect_failure 'grep does not honor textconv' ' >> +test_expect_success 'grep does honor textconv' ' >> echo "a:binaryQfile" >expect && >> git grep Qfile >actual && >> test_cmp expect actual >> @@ -172,7 +172,7 @@ test_expect_success 'grep --no-textconv does not honor textconv' ' >> test_must_fail git grep --no-textconv Qfile >> ' >> >> -test_expect_failure 'grep blob does not honor textconv' ' >> +test_expect_success 'grep blob does honor textconv' ' >> echo "HEAD:a:binaryQfile" >expect && >> git grep Qfile HEAD:a >actual && >> test_cmp expect actual