git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com, jrnieder@gmail.com
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: [PATCH 7/8] short status: improve reporting for submodule changes
Date: Thu, 23 Mar 2017 14:09:23 -0700	[thread overview]
Message-ID: <20170323210924.26818-8-sbeller@google.com> (raw)
In-Reply-To: <20170323210924.26818-1-sbeller@google.com>

If I add an untracked file to a submodule or modify a tracked file,
currently "git status --short" treats the change in the same way as
changes to the current HEAD of the submodule:

        $ git clone --quiet --recurse-submodules https://gerrit.googlesource.com/gerrit
        $ echo hello >gerrit/plugins/replication/stray-file
        $ sed -i -e 's/.*//' gerrit/plugins/replication/.mailmap
        $ git -C gerrit status --short
         M plugins/replication

This is by analogy with ordinary files, where "M" represents a change
that has not been added yet to the index.  But this change cannot be
added to the index without entering the submodule, "git add"-ing it,
and running "git commit", so the analogy is counterproductive.

Introduce new status letters " ?" and " m" for this.  These are similar
to the existing "??" and " M" but mean that the submodule (not the
parent project) has new untracked files and modified files, respectively.
The user can use "git add" and "git commit" from within the submodule to
add them.

Changes to the submodule's HEAD commit can be recorded in the index with
a plain "git add -u" and are shown with " M", like today.

To avoid excessive clutter, show at most one of " ?", " m", and " M" for
the submodule.  They represent increasing levels of change --- the last
one that applies is shown (e.g., " m" if there are both modified files
and untracked files in the submodule, or " M" if the submodule's HEAD
has been modified and it has untracked files).

While making these changes, we need to make sure to not break porcelain
level 1, which shares code with "status --short".  We only change
"git status --short".

Non-short "git status" and "git status --porcelain=2" already handle
these cases by showing more detail:

        $ git -C gerrit status --porcelain=2
        1 .M S.MU 160000 160000 160000 305c864db28eb0c77c8499bc04c87de3f849cf3c 305c864db28eb0c77c8499bc04c87de3f849cf3c plugins/replication
        $ git -C gerrit status
[...]
        modified:   plugins/replication (modified content, untracked content)

Scripts caring about these distinctions should use --porcelain=2.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/git-status.txt |  9 +++++++
 t/t3600-rm.sh                | 18 ++++++++++----
 t/t7506-status-submodule.sh  | 57 ++++++++++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 13 ++++++++--
 4 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index ba873657cf..01b457c322 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -181,6 +181,13 @@ in which case `XY` are `!!`.
     !           !    ignored
     -------------------------------------------------
 
+Submodules have more state and instead report
+		M    the submodule has a different HEAD than
+		     recorded in the index
+		m    the submodule has modified content
+		?    the submodule has untracked files
+
+
 If -b is used the short-format status is preceded by a line
 
     ## branchname tracking info
@@ -210,6 +217,8 @@ field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
+Any submodule changes are reported as modified `M` instead of `m` or single `?`.
+
 Porcelain Format Version 2
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 5aa6db584c..a6e5c5bd56 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -268,6 +268,14 @@ cat >expect.modified <<EOF
  M submod
 EOF
 
+cat >expect.modified_inside <<EOF
+ m submod
+EOF
+
+cat >expect.modified_untracked <<EOF
+ ? submod
+EOF
+
 cat >expect.cached <<EOF
 D  submod
 EOF
@@ -421,7 +429,7 @@ test_expect_success 'rm of a populated submodule with modifications fails unless
 	test -d submod &&
 	test -f submod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	test_cmp expect.modified actual &&
+	test_cmp expect.modified_inside actual &&
 	git rm -f submod &&
 	test ! -d submod &&
 	git status -s -uno --ignore-submodules=none >actual &&
@@ -436,7 +444,7 @@ test_expect_success 'rm of a populated submodule with untracked files fails unle
 	test -d submod &&
 	test -f submod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	test_cmp expect.modified actual &&
+	test_cmp expect.modified_untracked actual &&
 	git rm -f submod &&
 	test ! -d submod &&
 	git status -s -uno --ignore-submodules=none >actual &&
@@ -621,7 +629,7 @@ test_expect_success 'rm of a populated nested submodule with different nested HE
 	test -d submod &&
 	test -f submod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	test_cmp expect.modified actual &&
+	test_cmp expect.modified_inside actual &&
 	git rm -f submod &&
 	test ! -d submod &&
 	git status -s -uno --ignore-submodules=none >actual &&
@@ -636,7 +644,7 @@ test_expect_success 'rm of a populated nested submodule with nested modification
 	test -d submod &&
 	test -f submod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	test_cmp expect.modified actual &&
+	test_cmp expect.modified_inside actual &&
 	git rm -f submod &&
 	test ! -d submod &&
 	git status -s -uno --ignore-submodules=none >actual &&
@@ -651,7 +659,7 @@ test_expect_success 'rm of a populated nested submodule with nested untracked fi
 	test -d submod &&
 	test -f submod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	test_cmp expect.modified actual &&
+	test_cmp expect.modified_inside actual &&
 	git rm -f submod &&
 	test ! -d submod &&
 	git status -s -uno --ignore-submodules=none >actual &&
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index d31b34da83..ad46384064 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -50,6 +50,15 @@ test_expect_success 'status with modified file in submodule (porcelain)' '
 	EOF
 '
 
+test_expect_success 'status with modified file in submodule (short)' '
+	(cd sub && git reset --hard) &&
+	echo "changed" >sub/foo &&
+	git status --short >output &&
+	diff output - <<-\EOF
+	 m sub
+	EOF
+'
+
 test_expect_success 'status with added file in submodule' '
 	(cd sub && git reset --hard && echo >foo && git add foo) &&
 	git status >output &&
@@ -64,6 +73,14 @@ test_expect_success 'status with added file in submodule (porcelain)' '
 	EOF
 '
 
+test_expect_success 'status with added file in submodule (short)' '
+	(cd sub && git reset --hard && echo >foo && git add foo) &&
+	git status --short >output &&
+	diff output - <<-\EOF
+	 m sub
+	EOF
+'
+
 test_expect_success 'status with untracked file in submodule' '
 	(cd sub && git reset --hard) &&
 	echo "content" >sub/new-file &&
@@ -83,6 +100,13 @@ test_expect_success 'status with untracked file in submodule (porcelain)' '
 	EOF
 '
 
+test_expect_success 'status with untracked file in submodule (short)' '
+	git status --short >output &&
+	diff output - <<-\EOF
+	 ? sub
+	EOF
+'
+
 test_expect_success 'status with added and untracked file in submodule' '
 	(cd sub && git reset --hard && echo >foo && git add foo) &&
 	echo "content" >sub/new-file &&
@@ -271,4 +295,37 @@ test_expect_success 'diff --submodule with merge conflict in .gitmodules' '
 	test_cmp diff_submodule_actual diff_submodule_expect
 '
 
+test_expect_success 'setup superproject with untracked file in nested submodule' '
+	(
+		cd super &&
+		git clean -dfx &&
+		rm .gitmodules &&
+		git submodule add -f ./sub1 &&
+		git submodule add -f ./sub2 &&
+		git commit -a -m "messy merge in superproject" &&
+		(
+			cd sub1 &&
+			git submodule add ../sub2 &&
+			git commit -a -m "add sub2 to sub1"
+		) &&
+		git add sub1 &&
+		git commit -a -m "update sub1 to contain nested sub"
+	) &&
+	echo untracked >super/sub1/sub2/untracked
+'
+
+test_expect_success 'status with untracked file in nested submodule (porcelain)' '
+	git -C super status --porcelain >output &&
+	diff output - <<-\EOF
+	 M sub1
+	EOF
+'
+
+test_expect_success 'status with untracked file in nested submodule (short)' '
+	git -C super status --short >output &&
+	diff output - <<-\EOF
+	 m sub1
+	EOF
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index 308cf3779e..9909fd0e57 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -431,10 +431,19 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 		}
 		if (!d->worktree_status)
 			d->worktree_status = p->status;
-		d->dirty_submodule = p->two->dirty_submodule;
-		if (S_ISGITLINK(p->two->mode))
+		if (S_ISGITLINK(p->two->mode)) {
+			d->dirty_submodule = p->two->dirty_submodule;
 			d->new_submodule_commits = !!oidcmp(&p->one->oid,
 							    &p->two->oid);
+			if (s->status_format == STATUS_FORMAT_SHORT) {
+				if (d->new_submodule_commits)
+					d->worktree_status = 'M';
+				else if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
+					d->worktree_status = 'm';
+				else if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
+					d->worktree_status = '?';
+			}
+		}
 
 		switch (p->status) {
 		case DIFF_STATUS_ADDED:
-- 
2.12.1.438.gb674c4c09c


  parent reply	other threads:[~2017-03-23 21:09 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-23  0:43 [PATCHv3 0/3] short status: improve reporting for submodule changes Stefan Beller
2017-03-23  0:43 ` [PATCH 1/3] submodule.c: port is_submodule_modified to use porcelain 2 Stefan Beller
2017-03-23  0:53   ` Jonathan Nieder
2017-03-23  6:09     ` Junio C Hamano
2017-03-23 18:47       ` Stefan Beller
2017-03-23  0:43 ` [PATCH 2/3] submodule.c, is_submodule_modified: stricter checking for submodules Stefan Beller
2017-03-23  0:54   ` Jonathan Nieder
2017-03-23  0:43 ` [PATCH 3/3] short status: improve reporting for submodule changes Stefan Beller
2017-03-23  1:06   ` Jonathan Nieder
2017-03-23 21:09 ` [PATCH 0/8] " Stefan Beller
2017-03-23 21:09   ` [PATCH 1/8] submodule.c: port is_submodule_modified to use porcelain 2 Stefan Beller
2017-03-23 21:09   ` [PATCH 2/8] submodule.c: use argv_array in is_submodule_modified Stefan Beller
2017-03-23 21:09   ` [PATCH 3/8] submodule.c: convert is_submodule_modified to use strbuf_getwholeline_fd Stefan Beller
2017-03-23 21:09   ` [PATCH 4/8] submodule.c: port is_submodule_modified to use porcelain 2 Stefan Beller
2017-03-23 21:09   ` [PATCH 5/8] submodule.c: factor out early loop termination in is_submodule_modified Stefan Beller
2017-03-23 21:09   ` [PATCH 6/8] submodule.c: stricter checking for submodules " Stefan Beller
2017-03-23 21:09   ` Stefan Beller [this message]
2017-03-23 21:09   ` [PATCH 8/8] submodule.c: correctly handle nested " Stefan Beller
2017-03-23 21:11   ` [PATCH 0/8] short status: improve reporting for submodule changes Stefan Beller
2017-03-23 22:33     ` [PATCH v5 0/7] " Stefan Beller
2017-03-23 22:33       ` [PATCH 1/7] submodule.c: use argv_array in is_submodule_modified Stefan Beller
2017-03-23 22:36         ` Jonathan Nieder
2017-03-23 22:33       ` [PATCH 2/7] submodule.c: convert is_submodule_modified to use strbuf_getwholeline_fd Stefan Beller
2017-03-23 22:50         ` Jonathan Nieder
2017-03-23 23:04           ` Stefan Beller
2017-03-23 23:11             ` Stefan Beller
2017-03-23 22:33       ` [PATCH 3/7] submodule.c: port is_submodule_modified to use porcelain 2 Stefan Beller
2017-03-23 22:33       ` [PATCH 4/7] submodule.c: factor out early loop termination in is_submodule_modified Stefan Beller
2017-03-23 22:33       ` [PATCH 5/7] submodule.c: stricter checking for submodules " Stefan Beller
2017-03-23 22:33       ` [PATCH 6/7] short status: improve reporting for submodule changes Stefan Beller
2017-03-24 18:28         ` [PATCH v6 0/7] " Stefan Beller
2017-03-24 18:28           ` [PATCH 1/7] submodule.c: use argv_array in is_submodule_modified Stefan Beller
2017-03-24 22:25             ` Jonathan Nieder
2017-03-24 18:28           ` [PATCH 2/7] submodule.c: factor out early loop termination " Stefan Beller
2017-03-24 22:30             ` Jonathan Nieder
2017-03-24 18:28           ` [PATCH 3/7] submodule.c: convert is_submodule_modified to use strbuf_getwholeline Stefan Beller
2017-03-24 22:38             ` Jonathan Nieder
2017-03-25  0:12               ` Stefan Beller
2017-03-24 18:28           ` [PATCH 4/7] submodule.c: port is_submodule_modified to use porcelain 2 Stefan Beller
2017-03-24 22:41             ` Jonathan Nieder
2017-03-24 18:29           ` [PATCH 5/7] submodule.c: stricter checking for submodules in is_submodule_modified Stefan Beller
2017-03-24 22:42             ` Jonathan Nieder
2017-03-24 18:29           ` [PATCH 6/7] short status: improve reporting for submodule changes Stefan Beller
2017-03-24 23:06             ` Jonathan Nieder
2017-03-24 18:29           ` [PATCH 7/7] submodule.c: correctly handle nested submodules in is_submodule_modified Stefan Beller
2017-03-24 23:31             ` Jonathan Nieder
2017-03-25  0:25               ` Stefan Beller
2017-03-25  0:36           ` [PATCH v7 0/7] short status: improve reporting for submodule changes Stefan Beller
2017-03-25  0:36             ` [PATCH 1/7] submodule.c: use argv_array in is_submodule_modified Stefan Beller
2017-03-25  0:36             ` [PATCH 2/7] submodule.c: factor out early loop termination " Stefan Beller
2017-03-25  0:36             ` [PATCH 3/7] submodule.c: convert is_submodule_modified to use strbuf_getwholeline Stefan Beller
2017-03-25  0:36             ` [PATCH 4/7] submodule.c: port is_submodule_modified to use porcelain 2 Stefan Beller
2017-03-25  0:36             ` [PATCH 5/7] submodule.c: stricter checking for submodules in is_submodule_modified Stefan Beller
2017-03-25  0:36             ` [PATCH 6/7] short status: improve reporting for submodule changes Stefan Beller
2017-03-25  0:36             ` [PATCH 7/7] submodule.c: correctly handle nested submodules in is_submodule_modified Stefan Beller
2017-03-27 21:46               ` Junio C Hamano
2017-03-28  1:05                 ` Jonathan Nieder
2017-03-30 18:18                   ` Junio C Hamano
2017-03-28 21:20                 ` Stefan Beller
2017-03-25  1:35             ` [PATCH v7 0/7] short status: improve reporting for submodule changes Jonathan Nieder
2017-03-28 23:09             ` [PATCH v8 " Stefan Beller
2017-03-28 23:09               ` [PATCH 1/2] " Stefan Beller
2017-03-28 23:24                 ` Jonathan Nieder
2017-03-28 23:09               ` [PATCH 2/2] submodule.c: correctly handle nested submodules in is_submodule_modified Stefan Beller
2017-03-28 23:42                 ` Jonathan Nieder
2017-03-29 22:00                   ` Stefan Beller
2017-03-29 22:26               ` [PATCHv9 (6,7)/7] short status: improve reporting for submodule changes Stefan Beller
2017-03-29 22:26                 ` [PATCH 1/2] " Stefan Beller
2017-03-29 23:09                   ` Jonathan Nieder
2017-03-29 22:26                 ` [PATCH 2/2] submodule.c: correctly handle nested submodules in is_submodule_modified Stefan Beller
2017-03-29 23:13                   ` Jonathan Nieder
2017-03-30 18:28                     ` Junio C Hamano
2017-03-23 22:33       ` [PATCH 7/7] " Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170323210924.26818-8-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).