[PATCH 1/2] rm: better error message on failure for multiple files

[Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>]

From: Mathieu Liénard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>

When 'git rm' fails, it now displays a single message
with the list of files involved, instead of displaying
a list of messages with one file each.

As an example, the old message:
	error: 'foo.txt' has changes staged in the index
	(use --cached to keep the file, or -f to force removal)
	error: 'bar.txt' has changes staged in the index
	(use --cached to keep the file, or -f to force removal)

would now be displayed as:
	error: the following files have changes staged in the index:
		foo.txt
		bar.txt
	(use --cached to keep the file, or -f to force removal)

Signed-off-by: Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>
Signed-off-by: Jorge Juan Garcia Garcia <Jorge-Juan.garcia-Garcia@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 builtin/rm.c  |   54 ++++++++++++++++++++++++++++++++++++++----------------
 t/t3600-rm.sh |   45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 16 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..5b2abd2 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -82,6 +82,11 @@ static int check_local_mod(unsigned char *head, int index_only)
 	int i, no_head;
 	int errs = 0;
 
+	struct strbuf files_staged = STRBUF_INIT;
+	struct strbuf files_cached = STRBUF_INIT;
+	struct strbuf files_submodule = STRBUF_INIT;
+	struct strbuf files_local = STRBUF_INIT;
+
 	no_head = is_null_sha1(head);
 	for (i = 0; i < list.nr; i++) {
 		struct stat st;
@@ -170,30 +175,47 @@ static int check_local_mod(unsigned char *head, int index_only)
 		 * "intent to add" entry.
 		 */
 		if (local_changes && staged_changes) {
-			if (!index_only || !(ce->ce_flags & CE_INTENT_TO_ADD))
-				errs = error(_("'%s' has staged content different "
-					     "from both the file and the HEAD\n"
-					     "(use -f to force removal)"), name);
+			if (!index_only || !(ce->ce_flags & CE_INTENT_TO_ADD)) {
+				strbuf_addstr(&files_staged, "\n	");
+				strbuf_addstr(&files_staged, name);
+			}
 		}
 		else if (!index_only) {
-			if (staged_changes)
-				errs = error(_("'%s' has changes staged in the index\n"
-					     "(use --cached to keep the file, "
-					     "or -f to force removal)"), name);
+			if (staged_changes) {
+				strbuf_addstr(&files_cached, "\n	");
+				strbuf_addstr(&files_cached, name);
+			}
 			if (local_changes) {
 				if (S_ISGITLINK(ce->ce_mode) &&
 				    !submodule_uses_gitfile(name)) {
-					errs = error(_("submodule '%s' (or one of its nested "
-						     "submodules) uses a .git directory\n"
-						     "(use 'rm -rf' if you really want to remove "
-						     "it including all of its history)"), name);
-				} else
-					errs = error(_("'%s' has local modifications\n"
-						     "(use --cached to keep the file, "
-						     "or -f to force removal)"), name);
+					strbuf_addstr(&files_submodule, "\n	");
+					strbuf_addstr(&files_submodule, name);
+				} else {
+					strbuf_addstr(&files_local, "\n	");
+					strbuf_addstr(&files_local, name);
+				}
 			}
 		}
 	}
+
+	if (files_staged.len)
+		errs = error(_("the following files have staged content "
+			       "different from both the\nfileand the HEAD:%s\n"
+			       "(use -f to force removal)"), files_staged.buf);
+	if (files_cached.len)
+		errs = error(_("the following files have changes staged "
+			       "in the index:%s\n(use --cached to keep the file, "
+			       "or -f to force removal)"), files_cached.buf);
+	if (files_submodule.len)
+		errs = error(_("the following submodules (or one of its nested "
+			       "submodule) use a .git directory:%s\n"
+			       "(use 'rm -rf' if you really want to remove "
+			       "it including all of its history)"), files_submodule.buf);
+	if (files_local.len)
+		errs = error(_("the following files have local modifications:"
+			       "%s\n(use --cached to keep the file, or -f to "
+			       "force removal)"), files_local.buf);
+
 	return errs;
 }
 
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 0c44e9f..e0f3166 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -687,4 +687,49 @@ test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' '
 	test_path_is_file e/f
 '
 
+test_expect_success 'setup for testing rm messages' '
+	>bar.txt &&
+	>foo.txt &&
+	git add bar.txt foo.txt
+'
+
+test_expect_success 'rm files with different staged content' '
+	cat >expect << EOF &&
+error: the following files have staged content different from both the
+file and the HEAD:
+	bar.txt
+	foo.txt
+(use -f to force removal)
+EOF
+	echo content1 >foo.txt &&
+	echo content1 >bar.txt &&
+	test_must_fail git rm foo.txt bar.txt 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm file with local modification' '
+	cat >expect << EOF &&
+error: the following files have local modifications:
+	foo.txt
+(use --cached to keep the file, or -f to force removal)
+EOF
+	git commit -m "testing rm 3" &&
+	echo content3 >foo.txt &&
+	test_must_fail git rm foo.txt 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm file with changes in the index' '
+	cat >expect << EOF &&
+error: the following files have changes staged in the index:
+	foo.txt
+(use --cached to keep the file, or -f to force removal)
+EOF
+	git reset --hard &&
+	echo content5 >foo.txt &&
+	git add foo.txt &&
+	test_must_fail git rm foo.txt 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.8