Re: [PATCH] Refactor dirty submodule detection in diff-lib.c

[Junio C Hamano <gitster@pobox.com>]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Moving duplicated code into the new function match_stat_with_submodule().
> Replacing the implicit activation of detailed checks for the dirtiness of
> submodules when DIFF_FORMAT_PATCH was selected with explicitly setting
> the recently added DIFF_OPT_DIRTY_SUBMODULES option in diff_setup_done().
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> All tests run fine with this patch. All other callsites where the option
> DIFF_FORMAT_PATCH is set look like they don't deal with the work tree
> or they call diff_setup_done() shortly after.
> I am pretty sure about builtin/log.c, as this can't use the work tree
> but only already commited stuff, no?
> But then there is edit_patch() in buitin/add.c, I am not sure it makes
> sense to set DIRTY_SUBMODULES there too. Opinions?

I like this particular refactoring to remove duplicated obscure logic.

I am not sure if it is the right thing to do to return "changed" if the
change is only "submodule is dirty even though its HEAD is the same as
what is recorded in the index of the superproject", though.  See my
patches in response to your "[PATCH] git status: Fix false positive "new
commits" output for".  I think it would be conceptually the right thing to
do to feed the same object names in the sha1 field for both sides of the
filepair, with the working tree side marked with "dirty-submodule".  I
also suspect that you would not need to re-resolve the gitlinks in later
stages when you generate patches in textual form if you do it that way.