Re: [PATCH] range-diff: show submodule changes irrespective of diff.submodule

[Philippe Blain <levraiphilippeblain@gmail.com>]

Hi Junio,

Le 2022-06-02 à 13:36, Junio C Hamano a écrit :
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>>> Force the submodule diff format to its default ("short") when invoking
>>> 'git log' to generate the patches for each range, such that submodule
>>> changes are always shown.
>>
>> Full disclosure: I do not see much value in range-diffs in the presence of
>> submodules. Nothing in the design of range-diffs is prepared for
>> submodules.
>>
>> But since `--submodules=short` does not change anything when running
>> `range-diff` in repositories without submodules, I don't mind this change.
> 
> IOW, "I wrote it for the purpose of doing X, I do not care those who
> have been using it for doing Y, I am OK with changing behaviour on
> them".
> 
> Philippe, do you have a good guess on other users and workflows that
> may benefit from the current behaviour?  I suspect in the longer term
> this might have to become configurable, and I am having a hard time
> judging if (1) a temporary regression (to them) is acceptable or (2)
> the new feature to also show submodule changes is not urgent enough
> that it may be better to make it configurable from day one, instead
> of using a different hardcoded and only setting like this patch does.
> 
Just to be clear: the "out of the box" behaviour (i.e. nothing in the config)
is correct, i.e. submodule changes are detected and shown by 'git range-diff'.

It's only if someone has 'diff.submodule={log,diff}' in their config that submodule changes are
quietly ignored (log) or might crash 'git range-diff' (diff). So I do not think
of any user or workflow that benefit from the current behaviour, no. If you have 
diff.submodule={log,diff} set in your config, it's most probably because you work
in projects that involve submodules and you do care about submodule changes. So 
having these changes "hidden" by range-diff (or having range-diff crash) just because the output format 
of 'git -c diff.submodule={log,diff} log' does not use a 'diff --git' header for submodules is really
not expected. So I do not think we need to make that configurable. I think hardcoding
'--submodule=short' is an easy fix and a good first step in making 'git range-diff'
more useful for submodule users. 


Thanks,

Philippe.