Re: [PATCHv3 3/7] show: honor --textconv for blobs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano venit, vidit, dixit 11.05.2013 19:36:
> Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes:
> 
>>>> +	if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
>>>> +	    !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
>>>> +		return stream_blob_to_fd(1, sha1, NULL, 0);
>>>
>>> It is surprising that the necessary change is only this, but I think
>>> it is correct ;-).  We ignore textconv when the command line did not
>>> mention --[no-]textconv, or the command line said --no-textconv
>>> explicitly.
>>>
>>> This (especially the first condition) may deserve an in-code comment
>>> for anybody who wonders where this default behaviour is implemented.
>>
>> It's not as if we would document behavior by in-code comments in
>> general, do we? The usual answer is "git log -S" or "git blame".
> 
> The comment and the future reader I had in mind was more like
> 
> 	Default to --no-textconv, even though cmd_log_init_defaults()
>         sets the bit, when the user did not explicitly ask for it.
> 
> sought by somebody who wonders _where_ in the code we ignore
> ALLOW_TEXTCONV that is set in cmd_log_init_defaults().
> 
> That is not something you can find with "log -S" or "blame", is it?
> 

I'll refactor and restructure anyways. That will also get this whole
default discussion out of the way:

I'll try out the "show attribute" route as indicated. I'm not sure what
to do about the object_array/context discussion, though.

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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]