Re: [PATCH] Show submodules as modified when they contain a dirty work tree

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

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

> Subject: Show a modified submodule directory as dirty even if the refs match
>
> When the submodules HEAD and the ref committed in the HEAD of the
> superproject were the same, "git diff[-index] HEAD" did not show the
> submodule as dirty when it should.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>  diff-lib.c                |    3 ++-
>  t/t4027-diff-submodule.sh |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 1 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index 5ce226b..9cdf6da 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -233,7 +233,8 @@ static int get_stat_data(struct cache_entry *ce,
>  			return -1;
>  		}
>  		changed = ce_match_stat(ce, &st, 0);
> -		if (changed) {
> +		if (changed
> +		    || (S_ISGITLINK(ce->ce_mode) && is_submodule_modified(ce->name))) {

You had a check in your previous patch that decides to call or skip
diff_change() based on is_submodule_modified() for diff-files, but forgot
to have the same for diff-index, which this patch does.  Perhaps we want
to squash this into 4519d9c (Show submodules as modified when they contain
a dirty work tree, 2010-01-13).

The existing code is a bit unfortunate; by the time we come to the output
routine, the information we found from is_submodule_modified() is lost;
that is why my "would look like this" patch calls is_submodule_modified().

We may want to add one parameter to diff_change() and diff_addremove(), to
tell them if the work-tree side (if we are comparing something with the
work tree) is a modified submodule, and add one bit to the diff_filespec
structure to record that in diff_change() and diff_addremove() (obviously
only when adding).  That way, my "would looks like this" patch needs to
check the result of is_submodule_modified() the front-ends left in the
filespec, instead of running it again.