Subject: Re: [PATCH 0/6] grep with textconv

From: Michael J Gruber <[email-masked]>
Date: Sat, 20 Apr 2013 15:32:04 +0200

Junio C Hamano venit, vidit, dixit 19.04.2013 20:24:
> Michael J Gruber <[email-masked]> 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...

>>   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 ;)

>>   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: 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