Re: 'git range-diff' does not detect submodule changes if 'diff.submodule=log'

> Hi Philippe,
> 
> On Mon, 19 Jul 2021, Philippe Blain wrote:
> 
>> I noticed that 'git range-diff' silently "drops" submodule changes if
>> 'diff.submodule=log' is set in the config. This is because the 'diff --git'
>> header is not shown in that case, and the code in range-diff.c::read_patches
>> expects that header to be found to detect changes.
>>
>> If 'diff.submodule' is instead set to 'diff', the range-diff outright errors
>> with
>> (at least in my case):
>>
>> error: git apply: bad git-diff - inconsistent old filename on line 1
>> error: could not parse git header 'diff --git
>> error: path/to/submodule/and/some/file/within
>> '
>> error: could not parse log for '@{u}..@{1}'
>>
>>
>> I think it would make sense to force '--submodule=short' for range-diff,
>> something like:
>>
>> diff --git a/range-diff.c b/range-diff.c
>> index 1a4471fe4c..d74b9c7a55 100644
>> --- a/range-diff.c
>> +++ b/range-diff.c
>> @@ -54,7 +54,7 @@ static int read_patches(const char *range, struct
>> string_list *list,
>>
>>          strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
>>                       "--reverse", "--date-order", "--decorate=no",
>> -                    "--no-prefix",
>> +                    "--no-prefix", "--submodule=short",
>>                       /*
>>                        * Choose indicators that are not used anywhere
>>                        * else in diffs, but still look reasonable
>>
>>
>> What do you think ?
> 
> Sure. I never thought that `range-diff` would be useful in the context of
> submodules. But then, I am an anti-fan of submodules anyway, so don't put
> too much stock into my opinion about anything submodule-related.

> 
>> P.S. As an aside,
>> I'm  not sure why you chose to skip the 'diff --git' header with
>> '--submodule=log'
>> (I did not search the list), but I think that in general 'git diff' and
>> friends,
>> it would be nice to be able to still see this header even with
>> --submodule=log...
> 
> I don't remember the details. It might be that `git-tbdiff` did it that
> way, and I copied it without questioning.
> 
> *clicketyclick*
> 
> Yep, that seems plausible:
> https://github.com/trast/tbdiff/blob/047d1c79dfada57522a42f307cd4b0ddcb098934/git-tbdiff.py#L48