Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Feb 5, 2013, 08:48 UTC
Message-ID: <5110C745.9050306@drmicha.warpmail.net>
In-Reply-To: <7vtxps2eb2.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 04.02.2013 18:12:

Show 26 quoted lines

> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> 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 a binary mode "--textconv" (in addition to "--text" and "-I")
>> which makes git grep use any configured textconv filters for grepping
>> and output purposes.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>>
>> Notes:
>>     I'm somehow stuck in textconv/filespec/... hell, so I'm sending this out
>>     in request for help. I'm sure there are people for whom it's a breeze to
>>     get this right.
> 
> Looks like the patch touches the right places in the codepaths that
> need to be updated from a quick read.

I should have added that this coredumps when used on blobs and that textconv isn't invoked in any case (because it crashes on blobs, and I haven't implemented tetxconv on worktree files yet).

I'm somehow struggling to use just the right bits from the more diff centric helpers like fill_textconv or fill_one (which is static so far).

Show 6 quoted lines

>>     The difficulty is in getting the different cases (blob/sha1 vs.
>>     worktree) right, and in making the changes minimally invasive.
> 
> I think grep_source abstraction was intended to be a way for that,
> and if what it gives you is not sufficient for your needs, extending
> it should not be seen as "invasive" at all.

It seems to me that we textconvified the diff versions of these abstractions, but not the grep_source abstractions. Doing it at the source appears to be the right thing.

Michael