Subject: Re: [PATCH 4/5] diff: introduce diff.submoduleFormat configuration variable

From: Jens Lehmann <[email-masked]>
Date: Tue, 30 Oct 2012 22:26:07 +0100

Am 29.10.2012 11:30, schrieb Ramkumar Ramachandra:
> Jens Lehmann wrote:
>> Am 02.10.2012 21:44, schrieb Jens Lehmann:
>>> Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:
>>>> Introduce a diff.submoduleFormat configuration variable corresponding
>>>> to the '--submodule' command-line option of 'git diff'.
>>>
>>> Nice. Maybe a better name would be "diff.submodule", as this sets the
>>> default for the "--submodule" option of diff?
>>>
>>> And I think you should also test in t4041 that "--submodule=short"
>>> overrides the config setting.
>>
>> We also need tests which show that setting that config to "log" does
>> not break one of the many users of "git diff" ("stash", "rebase" and
>> "format-patch" come to mind, most probably I missed some others). I
>> suspect we'll have to add "--submodule=short" options to some call
>> sites to keep them working with submodule changes.
> 
> Um, why would "stash", "rebase" or "format-patch" be affected by this
> setting?  They don't operate on submodules at all.  To be sure, I ran
> all the tests with the following diff and nothing broke.

They do operate on the submodule commits too (while they don't touch
submodule work trees) and IIRC rebase applies diffs, so that could
break when the output of diff changes due to the new config option.
But it looks like your test did prove that nothing goes wrong there,
I assume they they use plumbing diff commands which aren't affected
by the new option.