Re: [PATCH 0/6] grep with textconv

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Apr 20, 2013, 13:32 UTC
Message-ID: <517298D4.3030802@drmicha.warpmail.net>
In-Reply-To: <7vhaj21ir3.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 19.04.2013 20:24:

Show 14 quoted lines

> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> This series teaches show and grep to obey textconv: show by
>> default (like diff), grep only on request (--textconv).  We might
>> switch the default for the latter also, of course.  I'd actually
>> like that.
>>
>> Compared to an earlier (historic) series this one comes with tests.
> 
> It would have been nicer if you referred to the previous thread
> 
> cf.
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/215385

Yes, sorry, I was on a slow mobile connection due to DSL breakage...

Show 7 quoted lines

>>   grep: allow to use textconv filters
> 
> This looked mostly sensible except for one minor "eh, do we really
> need to assume textconv output is text, or wouldn't using the same
> codepath for raw blob and textconv result to make them consistently
> honor opt->binary easier to explain?".
>

I think we assume in general that textconv produces text, which is maybe not completely surprising given its name ;)

Show 5 quoted lines

>>   t4030: demonstrate behavior of show with textconv
>>   t7008: demonstrate behavior of grep with textconv
> 
> It somehow felt they are better together in the patches that
> implement the features they exercise.

I added them after the fact. They can be squashed in, of course. On the other hand you don't see the change in behavior that the latter patches introduce any more if you that; which is why I left them separate at least for review purposes and for camparing to the previous series which I had failed to reference.

Show 10 quoted lines

>>   show: obey --textconv for blobs
>>   cat-file: do not die on --textconv without textconv filters
>>   grep: obey --textconv for the case rev:path
> 
> I just let my eyes coast over these but didn't see anything
> obviously wrong.
> 
> By the way, "git log --no-merges | grep obey | wc -l" shows that we
> say "honor an option" a lot more than "obey an option".  We may want
> to be consistent here.

Okay, let's be honorable rather than obedient.

Michael