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

From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Jul 19, 2021, 20:43 UTC
Message-ID: <e469038c-d78c-cd4b-0214-7094746b9281@gmail.com>

Hi Johannes,

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 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 ?

Thanks,
Philippe.

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...