[PATCHv9 (6,7)/7] short status: improve reporting for submodule changes

[Stefan Beller <sbeller@google.com>]

v9:
* This is a resend of the last two patches, i.e. these two patches apply
  at 5c896f7c3ec (origin/sb/submodule-short-status^^)
* below is a diff of this patch series against origin/sb/submodule-short-status
* better tests, refined documentation, thanks for the review, Jonathan!

Thanks,
Stefan

previous work:
https://public-inbox.org/git/20170328230938.9887-1-sbeller@google.com/

Stefan Beller (2):
  short status: improve reporting for submodule changes
  submodule.c: correctly handle nested submodules in
    is_submodule_modified

 Documentation/git-status.txt |  13 +++++
 submodule.c                  |  21 +++++++-
 t/t3600-rm.sh                |  18 +++++--
 t/t7506-status-submodule.sh  | 117 +++++++++++++++++++++++++++++++++++++++++++
 wt-status.c                  |  17 ++++++-
 5 files changed, 177 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 01b457c322..d70abc6afe 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -186,7 +186,11 @@ Submodules have more state and instead report
 		     recorded in the index
 		m    the submodule has modified content
 		?    the submodule has untracked files
+since modified content or untracked files in a submodule cannot be added
+via `git add` in the superproject to prepare a commit.
 
+'m' and '?' are applied recursively. For example if a nested submodule
+in a submodule contains an untracked file, this is reported as '?' as well.
 
 If -b is used the short-format status is preceded by a line
 
diff --git a/submodule.c b/submodule.c
index 730cc9513a..3da65100e3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1082,20 +1082,18 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
 		if (buf.buf[0] == 'u' ||
 		    buf.buf[0] == '1' ||
 		    buf.buf[0] == '2') {
-			/*
-			 * T XY SSSS:
-			 * T = line type, XY = status, SSSS = submodule state
-			 */
-			if (buf.len < 1 + 1 + 2 + 1 + 4)
+			/* T = line type, XY = status, SSSS = submodule state */
+			if (buf.len < strlen("T XY SSSS"))
 				die("BUG: invalid status --porcelain=2 line %s",
 				    buf.buf);
 
-			/* regular unmerged and renamed files */
 			if (buf.buf[5] == 'S' && buf.buf[8] == 'U')
 				/* nested untracked file */
 				dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
 
-			if (memcmp(buf.buf + 5, "S..U", 4))
+			if (buf.buf[0] == 'u' ||
+			    buf.buf[0] == '2' ||
+			    memcmp(buf.buf + 5, "S..U", 4))
 				/* other change */
 				dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
 		}
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index ab822c79e6..055c90736e 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -17,6 +17,12 @@ test_create_repo_with_commit () {
 	)
 }
 
+sanitize_output () {
+	sed -e "s/$_x40/HASH/" -e "s/$_x40/HASH/" output >output2 &&
+	mv output2 output
+}
+
+
 test_expect_success 'setup' '
 	test_create_repo_with_commit sub &&
 	echo output > .gitignore &&
@@ -311,6 +317,10 @@ test_expect_success 'diff --submodule with merge conflict in .gitmodules' '
 	test_cmp diff_submodule_actual diff_submodule_expect
 '
 
+# We'll setup different cases for further testing:
+# sub1 will contain a nested submodule,
+# sub2 will have an untracked file
+# sub3 will have an untracked repository
 test_expect_success 'setup superproject with untracked file in nested submodule' '
 	(
 		cd super &&
@@ -318,6 +328,7 @@ test_expect_success 'setup superproject with untracked file in nested submodule'
 		rm .gitmodules &&
 		git submodule add -f ./sub1 &&
 		git submodule add -f ./sub2 &&
+		git submodule add -f ./sub1 sub3 &&
 		git commit -a -m "messy merge in superproject" &&
 		(
 			cd sub1 &&
@@ -327,13 +338,27 @@ test_expect_success 'setup superproject with untracked file in nested submodule'
 		git add sub1 &&
 		git commit -a -m "update sub1 to contain nested sub"
 	) &&
-	echo untracked >super/sub1/sub2/untracked
+	echo content >super/sub1/sub2/file &&
+	echo content >super/sub2/file &&
+	git -C super/sub3 clone ../../sub2 untracked_repository
 '
 
 test_expect_success 'status with untracked file in nested submodule (porcelain)' '
 	git -C super status --porcelain >output &&
 	diff output - <<-\EOF
 	 M sub1
+	 M sub2
+	 M sub3
+	EOF
+'
+
+test_expect_success 'status with untracked file in nested submodule (porcelain=2)' '
+	git -C super status --porcelain=2 >output &&
+	sanitize_output output &&
+	diff output - <<-\EOF
+	1 .M S..U 160000 160000 160000 HASH HASH sub1
+	1 .M S..U 160000 160000 160000 HASH HASH sub2
+	1 .M S..U 160000 160000 160000 HASH HASH sub3
 	EOF
 '
 
@@ -341,6 +366,41 @@ test_expect_success 'status with untracked file in nested submodule (short)' '
 	git -C super status --short >output &&
 	diff output - <<-\EOF
 	 ? sub1
+	 ? sub2
+	 ? sub3
+	EOF
+'
+
+test_expect_success 'setup superproject with modified file in nested submodule' '
+	git -C super/sub1/sub2 add file &&
+	git -C super/sub2 add file
+'
+
+test_expect_success 'status with added file in nested submodule (porcelain)' '
+	git -C super status --porcelain >output &&
+	diff output - <<-\EOF
+	 M sub1
+	 M sub2
+	 M sub3
+	EOF
+'
+
+test_expect_success 'status with added file in nested submodule (porcelain=2)' '
+	git -C super status --porcelain=2 >output &&
+	sanitize_output output &&
+	diff output - <<-\EOF
+	1 .M S.M. 160000 160000 160000 HASH HASH sub1
+	1 .M S.M. 160000 160000 160000 HASH HASH sub2
+	1 .M S..U 160000 160000 160000 HASH HASH sub3
+	EOF
+'
+
+test_expect_success 'status with added file in nested submodule (short)' '
+	git -C super status --short >output &&
+	diff output - <<-\EOF
+	 m sub1
+	 m sub2
+	 ? sub3
 	EOF
 '