On Fri, Apr 19, 2013 at 06:44:48PM +0200, Michael J Gruber wrote:

> From: Jeff King <p...@peff.net>
> 
> Recently and not so recently, we made sure that log/grep type operations
> use textconv filters when a userfacing diff would do the same:
> 
> ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
> b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
> 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
> 
> "git grep" currently does not use textconv filters at all, that is
> neither for displaying the match and context nor for the actual grepping.
> 
> Introduce an option "--textconv" which makes git grep use any configured
> textconv filters for grepping and output purposes. It is off by default.
> 
> Signed-off-by: Michael J Gruber <g...@drmicha.warpmail.net>
> ---
>  builtin/grep.c         |   2 +
>  grep.c                 | 100 
> ++++++++++++++++++++++++++++++++++++++++++-------
>  grep.h                 |   1 +
>  t/t7008-grep-binary.sh |  18 +++++++++
>  4 files changed, 107 insertions(+), 14 deletions(-)

This patch, of course, is flawless. :)

Feel free to add:

  Signed-off-by: Jeff King <p...@peff.net>

> +     /*
> +      * We know the result of a textconv is text, so we only have to care
> +      * about binary handling if we are not using it.
> +      */
> +     if (!textconv) {
> +             switch (opt->binary) {
> +             case GREP_BINARY_DEFAULT:
> +                     if (grep_source_is_binary(gs))
> +                             binary_match_only = 1;
> +                     break;
> +             case GREP_BINARY_NOMATCH:
> +                     if (grep_source_is_binary(gs))
> +                             return 0; /* Assume unmatch */
> +                     break;
> +             case GREP_BINARY_TEXT:
> +                     break;
> +             default:
> +                     die("bug: unknown binary handling mode");
> +             }
>       }

Junio mentioned checking the textconv output for binary-ness. Doing that
would involve removing the outer conditional here. But it's not quite so
simple, as we don't load the textconv results until later, and
grep_source_is_binary will lazily load the contents. I think it would be
sufficient to fill_textconv_grep() right before, and then
grep_source_is_binary would rely on the cached buffer.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to