From: Michael J Gruber <git@drmicha•warpmail.net>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, Jeff King <peff@peff•net>,
Matthieu Moy <Matthieu.Moy@imag•fr>
Subject: Re: [PATCHv3 3/7] show: honor --textconv for blobs
Date: Sun, 12 May 2013 14:13:57 +0200 [thread overview]
Message-ID: <518F8785.4010401@drmicha.warpmail.net> (raw)
In-Reply-To: <7vr4hdxvtl.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 11.05.2013 19:36:
> Michael J Gruber <git@drmicha•warpmail.net> 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
next prev parent reply other threads:[~2013-05-12 12:14 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-19 16:44 [PATCH 0/6] grep with textconv Michael J Gruber
2013-04-19 16:44 ` [PATCH 1/6] t4030: demonstrate behavior of show " Michael J Gruber
2013-04-20 4:04 ` Jeff King
2013-04-20 13:35 ` Michael J Gruber
2013-04-19 16:44 ` [PATCH 2/6] show: obey --textconv for blobs Michael J Gruber
2013-04-20 4:06 ` Jeff King
2013-04-20 13:38 ` Michael J Gruber
2013-04-21 3:37 ` Jeff King
2013-04-22 10:29 ` Michael J Gruber
2013-04-22 15:25 ` Junio C Hamano
2013-04-22 15:29 ` Jeff King
2013-04-22 15:37 ` Jeremy Rosen
2013-04-22 15:54 ` Matthieu Moy
2013-04-23 8:58 ` Michael J Gruber
2013-04-19 16:44 ` [PATCH 3/6] cat-file: do not die on --textconv without textconv filters Michael J Gruber
2013-04-19 18:15 ` Junio C Hamano
2013-04-20 4:17 ` Jeff King
2013-04-20 14:27 ` Michael J Gruber
2013-04-19 16:44 ` [PATCH 4/6] t7008: demonstrate behavior of grep with textconv Michael J Gruber
2013-04-19 16:44 ` [PATCH 5/6] grep: allow to use textconv filters Michael J Gruber
2013-04-20 4:31 ` Jeff King
2013-04-19 16:44 ` [PATCH 6/6] grep: obey --textconv for the case rev:path Michael J Gruber
2013-04-20 4:24 ` Jeff King
2013-04-20 14:42 ` Michael J Gruber
2013-04-21 3:41 ` Jeff King
2013-04-19 18:24 ` [PATCH 0/6] grep with textconv Junio C Hamano
2013-04-20 4:26 ` Jeff King
2013-04-20 13:32 ` Michael J Gruber
2013-04-23 12:11 ` [PATCHv2 0/7] " Michael J Gruber
2013-04-23 12:11 ` [PATCHv2 1/7] t4030: demonstrate behavior of show " Michael J Gruber
2013-04-23 15:11 ` Junio C Hamano
2013-04-23 12:11 ` [PATCHv2 2/7] show: obey --textconv for blobs Michael J Gruber
2013-04-23 15:14 ` Junio C Hamano
2013-04-24 10:09 ` Michael J Gruber
2013-04-24 17:30 ` Junio C Hamano
2013-04-23 12:11 ` [PATCHv2 3/7] cat-file: do not die on --textconv without textconv filters Michael J Gruber
2013-04-23 15:15 ` Junio C Hamano
2013-04-23 12:11 ` [PATCHv2 4/7] t7008: demonstrate behavior of grep with textconv Michael J Gruber
2013-04-23 15:16 ` Junio C Hamano
2013-04-24 10:09 ` Michael J Gruber
2013-04-24 17:29 ` Junio C Hamano
2013-04-23 12:11 ` [PATCHv2 5/7] grep: allow to use textconv filters Michael J Gruber
2013-04-23 12:11 ` [PATCHv2 6/7] grep: honor --textconv for the case rev:path Michael J Gruber
2013-04-23 12:11 ` [PATCHv2 7/7] git grep: honor textconv by default Michael J Gruber
2013-04-23 15:20 ` Junio C Hamano
2013-04-24 10:05 ` Michael J Gruber
2013-04-24 17:35 ` Junio C Hamano
2013-04-24 17:57 ` Matthieu Moy
2013-04-24 18:55 ` Junio C Hamano
2013-04-26 11:59 ` Michael J Gruber
2013-04-26 13:23 ` Matthieu Moy
2013-04-29 9:04 ` Michael J Gruber
2013-04-29 15:04 ` Junio C Hamano
2013-05-10 15:08 ` [PATCHv3 0/7] textconv with grep and show Michael J Gruber
2013-05-10 15:10 ` [PATCHv3 1/7] t4030: demonstrate behavior of show with textconv Michael J Gruber
2013-05-10 15:10 ` [PATCHv3 2/7] diff_opt: track whether flags have been set explicitly Michael J Gruber
2013-05-10 15:31 ` Eric Sunshine
2013-05-10 15:10 ` [PATCHv3 3/7] show: honor --textconv for blobs Michael J Gruber
2013-05-10 17:02 ` Junio C Hamano
2013-05-10 17:34 ` Jeff King
2013-05-10 18:04 ` Junio C Hamano
2013-05-11 0:25 ` Jeff King
2013-05-11 19:54 ` Junio C Hamano
2013-05-11 8:54 ` Michael J Gruber
2013-05-11 10:00 ` Michael J Gruber
2013-05-13 5:01 ` Junio C Hamano
2013-05-13 11:55 ` Jeff King
2013-05-13 14:57 ` Michael J Gruber
2013-05-13 15:41 ` Junio C Hamano
2013-05-16 3:31 ` Jeff King
2013-05-11 17:36 ` Junio C Hamano
2013-05-12 12:13 ` Michael J Gruber [this message]
2013-05-10 15:10 ` [PATCHv3 4/7] cat-file: do not die on --textconv without textconv filters Michael J Gruber
2013-05-10 15:10 ` [PATCHv3 5/7] t7008: demonstrate behavior of grep with textconv Michael J Gruber
2013-05-10 15:10 ` [PATCHv3 6/7] grep: allow to use textconv filters Michael J Gruber
2013-05-10 15:10 ` [PATCHv3 7/7] grep: honor --textconv for the case rev:path Michael J Gruber
2013-05-10 18:11 ` Junio C Hamano
2013-05-10 18:31 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=518F8785.4010401@drmicha.warpmail.net \
--to=git@drmicha$(echo .)warpmail.net \
--cc=Matthieu.Moy@imag$(echo .)fr \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox