Jens Lehmann <Jens.Lehmann@xxxxxx> writes:
> Thanks for your review, here is the updated patch. Changes to the RFC
> version are:
>
> - Removed check for a dangling HEAD (now the testsuite runs fine)
> - Reworded the commit message
> - Inlined is_submodule_working_directory_dirty() into
> is_submodule_modified()
> - The new code will only be called when refs did match (when they
> didn't the submodule will already show up as modified)
Looking good.
I had to squash in '#include "submodule.h"' in diff-lib.c just after it
includes "refs.h", though.
And a patch to add:
>> * It doesn't give detailed output when doing a "git diff* -p" with or
>> without the --submodule option. It should show something like
>>
>> diff --git a/sub b/sub
>> index 5431f52..3f35670 160000
>> --- a/sub
>> +++ b/sub
>> @@ -1 +1 @@
>> -Subproject commit 5431f529197f3831cdfbba1354a819a79f948f6f
>> +Subproject commit 3f356705649b5d566d97ff843cf193359229a453-dirty
>>
would look like the attached.
I think a reasonable next step would be
- Move the check for your condition (c) that we dropped from this round
to wt-status.c;
- Add wt_status_print_dangling_submodules() to wt-status.c, and use the
above logic to produce a section "Submodules with Dangling HEAD" or
something.
- Call it in wt_status_print(), immediately before we check s->verbose
and show the patch text under -v option. "git status" now will warn
about the condition (c).
- Add a similar wt_shortstatus_print_dangling_submodules() and call it at
the end of wt_shortstatus_print().
- Update is_submodule_modified() in your patch thats reads the output
from "status --porcelain", to *ignore* information about dangling
submodules. As we discussed, dangling submodules may be something the
user cares about, but that is not something "diff" should.
-- >8 --
Subject: Teach diff that modified submodule directory is dirty
A diff run in superproject only compares the name of the commit object
bound at the submodule paths. When we compare with a work tree and the
checked out submodule directory is dirty (e.g. has either staged or
unstaged changes, or has new files the user forgot to add to the index),
show the work tree side as "dirty".
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
diff.c | 9 ++++++-
t/t4027-diff-submodule.sh | 49 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index 04beb26..750c066 100644
--- a/diff.c
+++ b/diff.c
@@ -2029,9 +2029,14 @@ static int populate_from_stdin(struct diff_filespec *s)
static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
{
int len;
- char *data = xmalloc(100);
+ char *data = xmalloc(100), *dirty = "";
+
+ /* Are we looking at the work tree? */
+ if (!s->sha1_valid && is_submodule_modified(s->path))
+ dirty = "-dirty";
+
len = snprintf(data, 100,
- "Subproject commit %s\n", sha1_to_hex(s->sha1));
+ "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty);
s->data = data;
s->size = len;
s->should_free = 1;
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 5cf8924..bf8c980 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -32,7 +32,8 @@ test_expect_success setup '
cd sub &&
git rev-list HEAD
) &&
- echo ":160000 160000 $3 $_z40 M sub" >expect
+ echo ":160000 160000 $3 $_z40 M sub" >expect &&
+ subtip=$3 subprev=$2
'
test_expect_success 'git diff --raw HEAD' '
@@ -50,6 +51,52 @@ test_expect_success 'git diff-files --raw' '
test_cmp expect actual.files
'
+expect_from_to () {
+ printf "%sSubproject commit %s\n+Subproject commit %s\n" \
+ "-" "$1" "$2"
+}
+
+test_expect_success 'git diff HEAD' '
+ git diff HEAD >actual &&
+ sed -e "1,/^@@/d" actual >actual.body &&
+ expect_from_to >expect.body $subtip $subprev &&
+ test_cmp expect.body actual.body
+'
+
+test_expect_success 'git diff HEAD with dirty submodule (work tree)' '
+ echo >>sub/world &&
+ git diff HEAD >actual &&
+ sed -e "1,/^@@/d" actual >actual.body &&
+ expect_from_to >expect.body $subtip $subprev-dirty &&
+ test_cmp expect.body actual.body
+'
+
+test_expect_success 'git diff HEAD with dirty submodule (index)' '
+ (
+ cd sub &&
+ git reset --hard &&
+ echo >>world &&
+ git add world
+ ) &&
+ git diff HEAD >actual &&
+ sed -e "1,/^@@/d" actual >actual.body &&
+ expect_from_to >expect.body $subtip $subprev-dirty &&
+ test_cmp expect.body actual.body
+'
+
+test_expect_success 'git diff HEAD with dirty submodule (untracked)' '
+ (
+ cd sub &&
+ git reset --hard &&
+ git clean -qfdx &&
+ >cruft
+ ) &&
+ git diff HEAD >actual &&
+ sed -e "1,/^@@/d" actual >actual.body &&
+ expect_from_to >expect.body $subtip $subprev-dirty &&
+ test_cmp expect.body actual.body
+'
+
test_expect_success 'git diff (empty submodule dir)' '
: >empty &&
rm -rf sub/* sub/.git &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html