Re: 'git range-diff' does not detect submodule changes if 'diff.submodule=log'
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 22, 2021, 13:19 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2107221516240.55@tvgsbejvaqbjf.bet>
- In-Reply-To
- <e469038c-d78c-cd4b-0214-7094746b9281@gmail.com>
Hi Philippe,
On Mon, 19 Jul 2021, Philippe Blain wrote:
Show 36 quoted lines
> 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.
Show 7 quoted lines
> 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
Ciao, Dscho