git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] rm: better error message on failure for multiple files
@ 2013-06-10 12:51 Mathieu Lienard--Mayor
  2013-06-10 12:51 ` [PATCH 2/2] rm: introduce advice.rmHints to shorten messages Mathieu Lienard--Mayor
  2013-06-10 12:53 ` [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Liénard--Mayor
  0 siblings, 2 replies; 14+ messages in thread
From: Mathieu Lienard--Mayor @ 2013-06-10 12:51 UTC (permalink / raw)
  To: git
  Cc: gitster, Mathieu Lienard--Mayor, Jorge Juan Garcia Garcia, Matthieu Moy

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 |   93 +++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..1bff656 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 string_list files_staged = STRING_LIST_INIT_NODUP;
+	struct string_list files_cached = STRING_LIST_INIT_NODUP;
+	struct string_list files_submodule = STRING_LIST_INIT_NODUP;
+	struct string_list files_local = STRING_LIST_INIT_NODUP;
+
 	no_head = is_null_sha1(head);
 	for (i = 0; i < list.nr; i++) {
 		struct stat st;
@@ -171,29 +176,89 @@ static int check_local_mod(unsigned char *head, int index_only)
 		 */
 		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);
+				string_list_append(&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);
+				string_list_append(&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);
+					string_list_append(&files_submodule,
+							   name);
+				} else {
+					string_list_append(&files_local, name);
+				}
 			}
 		}
 	}
+	if (files_staged.nr) {
+		struct strbuf msg_staged = STRBUF_INIT;
+		int j;
+		strbuf_addstr(
+			&msg_staged,
+			"the following files have staged content different "
+			"from both the\nfile and the HEAD:");
+		for (j = 0; j < files_staged.nr; j++) {
+			strbuf_addf(&msg_staged,
+				    "\n	%s",
+				    files_staged.items[j].string);
+		}
+		strbuf_addstr(&msg_staged,
+			      "\n(use -f to force removal)");
+		errs = error(_("%s"), msg_staged.buf);
+	}
+	if (files_cached.nr) {
+		struct strbuf msg_cached = STRBUF_INIT;
+		int j;
+		strbuf_addstr(
+			&msg_cached,
+			"the following files have changes staged "
+			"in the index:");
+		for (j = 0; j < files_cached.nr; j++) {
+			strbuf_addf(&msg_cached,
+				    "\n	%s",
+				    files_cached.items[j].string);
+		}
+		strbuf_addstr(&msg_cached,
+			      "\n(use --cached to keep the file, "
+			      "or -f to force removal)");
+		errs = error(_("%s"), msg_cached.buf);
+	}
+	if (files_submodule.nr) {
+		struct strbuf msg_sub = STRBUF_INIT;
+		int j;
+		strbuf_addstr(
+			&msg_sub,
+			"the following submodules (or one of its nested "
+			"submodule) use a .git directory:");
+		for (j = 0; j < files_submodule.nr; j++) {
+			strbuf_addf(&msg_sub,
+				    "\n	%s",
+				    files_submodule.items[j].string);
+		}
+		strbuf_addstr(&msg_sub,
+			      "\n(use 'rm -rf' if you really want "
+			      "to remove i including all "
+			      "of its history)");
+		errs = error(_("%s"), msg_sub.buf);
+	}
+	if (files_local.nr) {
+		struct strbuf msg_local = STRBUF_INIT;
+		int j;
+		strbuf_addstr(&msg_local,
+			      "the following files have local modifications:");
+		for (j = 0; j < files_local.nr; j++) {
+			strbuf_addf(&msg_local,
+				    "\n	%s",
+				    files_local.items[j].string);
+		}
+		strbuf_addstr(&msg_local,
+			      "\n(use --cached to keep the file, "
+			      "or -f to force removal)");
+		errs = error(_("%s"), msg_local.buf);
+	}
+
 	return errs;
 }
 
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-10 12:51 [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Lienard--Mayor
@ 2013-06-10 12:51 ` Mathieu Lienard--Mayor
  2013-06-10 12:54   ` Mathieu Liénard--Mayor
  2013-06-10 12:53 ` [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Liénard--Mayor
  1 sibling, 1 reply; 14+ messages in thread
From: Mathieu Lienard--Mayor @ 2013-06-10 12:51 UTC (permalink / raw)
  To: git
  Cc: gitster, Mathieu Lienard--Mayor, Jorge Juan Garcia Garcia, Matthieu Moy

Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.

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

would look like, with advice.rmHints=false:
	error: 'foo.txt' has changes staged in the index

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>
---
 Documentation/config.txt |    3 ++
 advice.c                 |    2 +
 advice.h                 |    1 +
 builtin/rm.c             |   36 ++++++++++++---------
 t/t3600-rm.sh            |   77 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 104 insertions(+), 15 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.
+	rmHints::
+		In case of failure in the output of linkgit:git-rm[1],
+		show directions on how to proceed from the current state.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
 int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
 
 static struct {
 	const char *name;
@@ -33,6 +34,7 @@ static struct {
 	{ "implicitidentity", &advice_implicit_identity },
 	{ "detachedhead", &advice_detached_head },
 	{ "setupstreamfailure", &advice_set_upstream_failure },
+	{ "rmhints", &advice_rm_hints },
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
 extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
 
 int git_default_advice_config(const char *var, const char *value);
 void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 1bff656..c9081cd 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
 
 		if (!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);
+				       "submodules) uses a .git directory%s"), name,
+				       advice_rm_hints
+				       ? "\n(use 'rm -rf' if you really want to remove "
+				       "it including all of its history)"
+				       : "");
 	}
 
 	return errs;
@@ -204,8 +206,9 @@ static int check_local_mod(unsigned char *head, int index_only)
 				    "\n	%s",
 				    files_staged.items[j].string);
 		}
-		strbuf_addstr(&msg_staged,
-			      "\n(use -f to force removal)");
+		if (advice_rm_hints)
+			strbuf_addstr(&msg_staged,
+				   "\n(use -f to force removal)");
 		errs = error(_("%s"), msg_staged.buf);
 	}
 	if (files_cached.nr) {
@@ -220,9 +223,10 @@ static int check_local_mod(unsigned char *head, int index_only)
 				    "\n	%s",
 				    files_cached.items[j].string);
 		}
-		strbuf_addstr(&msg_cached,
-			      "\n(use --cached to keep the file, "
-			      "or -f to force removal)");
+		if (advice_rm_hints)
+			strbuf_addstr(&msg_cached,
+				      "\n(use --cached to keep the file, "
+				      "or -f to force removal)");
 		errs = error(_("%s"), msg_cached.buf);
 	}
 	if (files_submodule.nr) {
@@ -237,10 +241,11 @@ static int check_local_mod(unsigned char *head, int index_only)
 				    "\n	%s",
 				    files_submodule.items[j].string);
 		}
-		strbuf_addstr(&msg_sub,
-			      "\n(use 'rm -rf' if you really want "
-			      "to remove i including all "
-			      "of its history)");
+		if (advice_rm_hints)
+			strbuf_addstr(&msg_sub,
+				      "\n(use 'rm -rf' if you really want "
+				      "to remove i including all "
+				      "of its history)");
 		errs = error(_("%s"), msg_sub.buf);
 	}
 	if (files_local.nr) {
@@ -253,9 +258,10 @@ static int check_local_mod(unsigned char *head, int index_only)
 				    "\n	%s",
 				    files_local.items[j].string);
 		}
-		strbuf_addstr(&msg_local,
-			      "\n(use --cached to keep the file, "
-			      "or -f to force removal)");
+		if (advice_rm_hints)
+			strbuf_addstr(&msg_local,
+				      "\n(use --cached to keep the file, "
+				      "or -f to force removal)");
 		errs = error(_("%s"), msg_local.buf);
 	}
 
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 0c44e9f..ab10cc6 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -687,4 +687,81 @@ 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 files with different staged content without hints' '
+	cat >expect << EOF &&
+error: the following files have staged content different from both the
+file and the HEAD:
+	bar.txt
+	foo.txt
+EOF
+	echo content2 >foo.txt &&
+	echo content2 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false 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 local modification without hints' '
+	cat >expect << EOF &&
+error: the following files have local modifications:
+	bar.txt
+EOF
+	echo content4 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false rm bar.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_expect_success 'rm file with changes in the index without hints' '
+	cat >expect << EOF &&
+error: the following files have changes staged in the index:
+	foo.txt
+EOF
+	test_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] rm: better error message on failure for multiple  files
  2013-06-10 12:51 [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Lienard--Mayor
  2013-06-10 12:51 ` [PATCH 2/2] rm: introduce advice.rmHints to shorten messages Mathieu Lienard--Mayor
@ 2013-06-10 12:53 ` Mathieu Liénard--Mayor
  1 sibling, 0 replies; 14+ messages in thread
From: Mathieu Liénard--Mayor @ 2013-06-10 12:53 UTC (permalink / raw)
  To: Mathieu Lienard--Mayor
  Cc: git, gitster, Jorge Juan Garcia Garcia, Matthieu Moy

Please ignore this, manipulation error while in the git send-email 
command line.

Le 2013-06-10 14:51, Mathieu Lienard--Mayor a écrit :
> 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 |   93 
> +++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 79 insertions(+), 14 deletions(-)
>
> diff --git a/builtin/rm.c b/builtin/rm.c
> index 7b91d52..1bff656 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 string_list files_staged = STRING_LIST_INIT_NODUP;
> +	struct string_list files_cached = STRING_LIST_INIT_NODUP;
> +	struct string_list files_submodule = STRING_LIST_INIT_NODUP;
> +	struct string_list files_local = STRING_LIST_INIT_NODUP;
> +
>  	no_head = is_null_sha1(head);
>  	for (i = 0; i < list.nr; i++) {
>  		struct stat st;
> @@ -171,29 +176,89 @@ static int check_local_mod(unsigned char *head,
> int index_only)
>  		 */
>  		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);
> +				string_list_append(&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);
> +				string_list_append(&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);
> +					string_list_append(&files_submodule,
> +							   name);
> +				} else {
> +					string_list_append(&files_local, name);
> +				}
>  			}
>  		}
>  	}
> +	if (files_staged.nr) {
> +		struct strbuf msg_staged = STRBUF_INIT;
> +		int j;
> +		strbuf_addstr(
> +			&msg_staged,
> +			"the following files have staged content different "
> +			"from both the\nfile and the HEAD:");
> +		for (j = 0; j < files_staged.nr; j++) {
> +			strbuf_addf(&msg_staged,
> +				    "\n	%s",
> +				    files_staged.items[j].string);
> +		}
> +		strbuf_addstr(&msg_staged,
> +			      "\n(use -f to force removal)");
> +		errs = error(_("%s"), msg_staged.buf);
> +	}
> +	if (files_cached.nr) {
> +		struct strbuf msg_cached = STRBUF_INIT;
> +		int j;
> +		strbuf_addstr(
> +			&msg_cached,
> +			"the following files have changes staged "
> +			"in the index:");
> +		for (j = 0; j < files_cached.nr; j++) {
> +			strbuf_addf(&msg_cached,
> +				    "\n	%s",
> +				    files_cached.items[j].string);
> +		}
> +		strbuf_addstr(&msg_cached,
> +			      "\n(use --cached to keep the file, "
> +			      "or -f to force removal)");
> +		errs = error(_("%s"), msg_cached.buf);
> +	}
> +	if (files_submodule.nr) {
> +		struct strbuf msg_sub = STRBUF_INIT;
> +		int j;
> +		strbuf_addstr(
> +			&msg_sub,
> +			"the following submodules (or one of its nested "
> +			"submodule) use a .git directory:");
> +		for (j = 0; j < files_submodule.nr; j++) {
> +			strbuf_addf(&msg_sub,
> +				    "\n	%s",
> +				    files_submodule.items[j].string);
> +		}
> +		strbuf_addstr(&msg_sub,
> +			      "\n(use 'rm -rf' if you really want "
> +			      "to remove i including all "
> +			      "of its history)");
> +		errs = error(_("%s"), msg_sub.buf);
> +	}
> +	if (files_local.nr) {
> +		struct strbuf msg_local = STRBUF_INIT;
> +		int j;
> +		strbuf_addstr(&msg_local,
> +			      "the following files have local modifications:");
> +		for (j = 0; j < files_local.nr; j++) {
> +			strbuf_addf(&msg_local,
> +				    "\n	%s",
> +				    files_local.items[j].string);
> +		}
> +		strbuf_addstr(&msg_local,
> +			      "\n(use --cached to keep the file, "
> +			      "or -f to force removal)");
> +		errs = error(_("%s"), msg_local.buf);
> +	}
> +
>  	return errs;
>  }

-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-10 12:51 ` [PATCH 2/2] rm: introduce advice.rmHints to shorten messages Mathieu Lienard--Mayor
@ 2013-06-10 12:54   ` Mathieu Liénard--Mayor
  2013-06-10 16:57     ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Mathieu Liénard--Mayor @ 2013-06-10 12:54 UTC (permalink / raw)
  To: Mathieu Lienard--Mayor
  Cc: git, gitster, Jorge Juan Garcia Garcia, Matthieu Moy

Please ignore this, manipulation error while in the git send-email 
command line.

Le 2013-06-10 14:51, Mathieu Lienard--Mayor a écrit :
> Introduce advice.rmHints to choose whether to display advice or not
> when git rm fails. Defaults to true, in order to preserve current 
> behavior.
>
> As an example, the message:
> 	error: 'foo.txt' has changes staged in the index
> 	(use --cached to keep the file, or -f to force removal)
>
> would look like, with advice.rmHints=false:
> 	error: 'foo.txt' has changes staged in the index
>
> 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>
> ---
>  Documentation/config.txt |    3 ++
>  advice.c                 |    2 +
>  advice.h                 |    1 +
>  builtin/rm.c             |   36 ++++++++++++---------
>  t/t3600-rm.sh            |   77 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 104 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 6e53fc5..eb04479 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -199,6 +199,9 @@ advice.*::
>  	amWorkDir::
>  		Advice that shows the location of the patch file when
>  		linkgit:git-am[1] fails to apply it.
> +	rmHints::
> +		In case of failure in the output of linkgit:git-rm[1],
> +		show directions on how to proceed from the current state.
>  --
>
>  core.fileMode::
> diff --git a/advice.c b/advice.c
> index a8deee6..a4c169c 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
>  int advice_implicit_identity = 1;
>  int advice_detached_head = 1;
>  int advice_set_upstream_failure = 1;
> +int advice_rm_hints = 1;
>
>  static struct {
>  	const char *name;
> @@ -33,6 +34,7 @@ static struct {
>  	{ "implicitidentity", &advice_implicit_identity },
>  	{ "detachedhead", &advice_detached_head },
>  	{ "setupstreamfailure", &advice_set_upstream_failure },
> +	{ "rmhints", &advice_rm_hints },
>
>  	/* make this an alias for backward compatibility */
>  	{ "pushnonfastforward", &advice_push_update_rejected }
> diff --git a/advice.h b/advice.h
> index 94caa32..36104c4 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
>  extern int advice_implicit_identity;
>  extern int advice_detached_head;
>  extern int advice_set_upstream_failure;
> +extern int advice_rm_hints;
>
>  int git_default_advice_config(const char *var, const char *value);
>  void advise(const char *advice, ...);
> diff --git a/builtin/rm.c b/builtin/rm.c
> index 1bff656..c9081cd 100644
> --- a/builtin/rm.c
> +++ b/builtin/rm.c
> @@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
>
>  		if (!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);
> +				       "submodules) uses a .git directory%s"), name,
> +				       advice_rm_hints
> +				       ? "\n(use 'rm -rf' if you really want to remove "
> +				       "it including all of its history)"
> +				       : "");
>  	}
>
>  	return errs;
> @@ -204,8 +206,9 @@ static int check_local_mod(unsigned char *head,
> int index_only)
>  				    "\n	%s",
>  				    files_staged.items[j].string);
>  		}
> -		strbuf_addstr(&msg_staged,
> -			      "\n(use -f to force removal)");
> +		if (advice_rm_hints)
> +			strbuf_addstr(&msg_staged,
> +				   "\n(use -f to force removal)");
>  		errs = error(_("%s"), msg_staged.buf);
>  	}
>  	if (files_cached.nr) {
> @@ -220,9 +223,10 @@ static int check_local_mod(unsigned char *head,
> int index_only)
>  				    "\n	%s",
>  				    files_cached.items[j].string);
>  		}
> -		strbuf_addstr(&msg_cached,
> -			      "\n(use --cached to keep the file, "
> -			      "or -f to force removal)");
> +		if (advice_rm_hints)
> +			strbuf_addstr(&msg_cached,
> +				      "\n(use --cached to keep the file, "
> +				      "or -f to force removal)");
>  		errs = error(_("%s"), msg_cached.buf);
>  	}
>  	if (files_submodule.nr) {
> @@ -237,10 +241,11 @@ static int check_local_mod(unsigned char *head,
> int index_only)
>  				    "\n	%s",
>  				    files_submodule.items[j].string);
>  		}
> -		strbuf_addstr(&msg_sub,
> -			      "\n(use 'rm -rf' if you really want "
> -			      "to remove i including all "
> -			      "of its history)");
> +		if (advice_rm_hints)
> +			strbuf_addstr(&msg_sub,
> +				      "\n(use 'rm -rf' if you really want "
> +				      "to remove i including all "
> +				      "of its history)");
>  		errs = error(_("%s"), msg_sub.buf);
>  	}
>  	if (files_local.nr) {
> @@ -253,9 +258,10 @@ static int check_local_mod(unsigned char *head,
> int index_only)
>  				    "\n	%s",
>  				    files_local.items[j].string);
>  		}
> -		strbuf_addstr(&msg_local,
> -			      "\n(use --cached to keep the file, "
> -			      "or -f to force removal)");
> +		if (advice_rm_hints)
> +			strbuf_addstr(&msg_local,
> +				      "\n(use --cached to keep the file, "
> +				      "or -f to force removal)");
>  		errs = error(_("%s"), msg_local.buf);
>  	}
>
> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
> index 0c44e9f..ab10cc6 100755
> --- a/t/t3600-rm.sh
> +++ b/t/t3600-rm.sh
> @@ -687,4 +687,81 @@ 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 files with different staged content without 
> hints' '
> +	cat >expect << EOF &&
> +error: the following files have staged content different from both 
> the
> +file and the HEAD:
> +	bar.txt
> +	foo.txt
> +EOF
> +	echo content2 >foo.txt &&
> +	echo content2 >bar.txt &&
> +	test_must_fail git -c advice.rmhints=false 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 local modification without hints' 
> '
> +	cat >expect << EOF &&
> +error: the following files have local modifications:
> +	bar.txt
> +EOF
> +	echo content4 >bar.txt &&
> +	test_must_fail git -c advice.rmhints=false rm bar.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_expect_success 'rm file with changes in the index without 
> hints' '
> +	cat >expect << EOF &&
> +error: the following files have changes staged in the index:
> +	foo.txt
> +EOF
> +	test_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done

-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-10 12:54   ` Mathieu Liénard--Mayor
@ 2013-06-10 16:57     ` Junio C Hamano
  2013-06-10 17:17       ` Mathieu Liénard--Mayor
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2013-06-10 16:57 UTC (permalink / raw)
  To: Mathieu Liénard--Mayor
  Cc: Mathieu Lienard--Mayor, git, Jorge Juan Garcia Garcia, Matthieu Moy

Mathieu Liénard--Mayor  <mathieu.lienard--mayor@ensimag.fr> writes:

> Please ignore this, manipulation error while in the git send-email
> command line.

Here is what my mailbox looks like (the penultimate one with 252
lines is what I am responding to).  

  R. [ 146: Mathieu Lienard--Mayor ] [PATCH 1/2] rm: better error messa
  R. [ 231: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce advice.r
  R. [ 157: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce advice.r
  R. [ 198: Mathieu Lienard--Mayor ] [PATCH v2 1/2] rm: better error me
  R. [ 157: Mathieu Lienard--Mayor ] [PATCH v2 2/2] rm: introduce advic
   . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce advic
   . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2]  rm: better error m
   . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2]  rm: better error m
   . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce advic
  R  [  33: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error m
  O  [  38: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce advi
  R. [ 156: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error m
  R. [ 252: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce advi
   . [  84: Mathieu Liénard--Mayor ] Re: [PATCH v2 1/2] rm: better erro

I am guessing that [v3 1/2] and [v3 2/2] are the final ones but it
that is not the case please holler.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-10 16:57     ` Junio C Hamano
@ 2013-06-10 17:17       ` Mathieu Liénard--Mayor
  0 siblings, 0 replies; 14+ messages in thread
From: Mathieu Liénard--Mayor @ 2013-06-10 17:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Mathieu Lienard--Mayor, git, Jorge Juan Garcia Garcia, Matthieu Moy

Le 2013-06-10 18:57, Junio C Hamano a écrit :
> Mathieu Liénard--Mayor  <mathieu.lienard--mayor@ensimag.fr> writes:
>
>> Please ignore this, manipulation error while in the git send-email
>> command line.
>
> Here is what my mailbox looks like (the penultimate one with 252
> lines is what I am responding to).
>
>   R. [ 146: Mathieu Lienard--Mayor ] [PATCH 1/2] rm: better error 
> messa
>   R. [ 231: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce 
> advice.r
>   R. [ 157: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce 
> advice.r
>   R. [ 198: Mathieu Lienard--Mayor ] [PATCH v2 1/2] rm: better error 
> me
>   R. [ 157: Mathieu Lienard--Mayor ] [PATCH v2 2/2] rm: introduce 
> advic
>    . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce 
> advic
>    . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2]  rm: better error 
> m
>    . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2]  rm: better error 
> m
>    . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce 
> advic
>   R  [  33: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error 
> m
>   O  [  38: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce 
> advi
>   R. [ 156: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error 
> m
>   R. [ 252: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce 
> advi
>    . [  84: Mathieu Liénard--Mayor ] Re: [PATCH v2 1/2] rm: better 
> erro
>
> I am guessing that [v3 1/2] and [v3 2/2] are the final ones but it
> that is not the case please holler.
Yes, [v3 1/2] and [v3 2/2] are the final ones.
i'm sorry, i really don't know how i managed to create such a mess, i'm 
still not familiar with the send-email tool =/
-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-12  8:06 [PATCH v5 " Mathieu Lienard--Mayor
@ 2013-06-12  8:06 ` Mathieu Lienard--Mayor
  0 siblings, 0 replies; 14+ messages in thread
From: Mathieu Lienard--Mayor @ 2013-06-12  8:06 UTC (permalink / raw)
  To: git
  Cc: gitster, Mathieu Lienard--Mayor, Jorge Juan Garcia Garcia, Matthieu Moy

Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.

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

would look like, with advice.rmHints=false:
	error: 'foo.txt' has changes staged in the index

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>
---

No changes since v4

 Documentation/config.txt |    3 +++
 advice.c                 |    2 ++
 advice.h                 |    1 +
 builtin/rm.c             |    3 ++-
 t/t3600-rm.sh            |   29 +++++++++++++++++++++++++++++
 5 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.
+	rmHints::
+		In case of failure in the output of linkgit:git-rm[1],
+		show directions on how to proceed from the current state.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
 int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
 
 static struct {
 	const char *name;
@@ -33,6 +34,7 @@ static struct {
 	{ "implicitidentity", &advice_implicit_identity },
 	{ "detachedhead", &advice_detached_head },
 	{ "setupstreamfailure", &advice_set_upstream_failure },
+	{ "rmhints", &advice_rm_hints },
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
 extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
 
 int git_default_advice_config(const char *var, const char *value);
 void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 18df253..11ad53a 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -50,7 +50,8 @@ static void print_error_files(struct string_list *files_list,
 			strbuf_addf(&err_msg,
 				    "\n    %s",
 				    files_list->items[i].string);
-		strbuf_addstr(&err_msg, hints_msg);
+		if (advice_rm_hints)
+			strbuf_addstr(&err_msg, hints_msg);
 		*errs = error("%s", err_msg.buf);
 		strbuf_release(&err_msg);
 	}
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 902993b..5c87b55 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -707,6 +707,18 @@ test_expect_success 'rm files with different staged content' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'rm files with different staged content without hints' '
+	cat >expect <<-\EOF &&
+	error: the following files have staged content different from both the
+	file and the HEAD:
+	    bar.txt
+	    foo.txt
+	EOF
+	echo content2 >foo.txt &&
+	echo content2 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false rm foo.txt bar.txt 2>actual &&
+	test_i18ncmp expect actual
+'
 
 test_expect_success 'rm file with local modification' '
 	cat >expect <<-\EOF &&
@@ -720,6 +732,15 @@ test_expect_success 'rm file with local modification' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'rm file with local modification without hints' '
+	cat >expect <<-\EOF &&
+	error: the following file has local modifications:
+	    bar.txt
+	EOF
+	echo content4 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false rm bar.txt 2>actual &&
+	test_i18ncmp expect actual
+'
 
 test_expect_success 'rm file with changes in the index' '
 	cat >expect <<-\EOF &&
@@ -734,6 +755,14 @@ test_expect_success 'rm file with changes in the index' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'rm file with changes in the index without hints' '
+	cat >expect <<-\EOF &&
+	error: the following file has changes staged in the index:
+	    foo.txt
+	EOF
+	test_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&
+	test_i18ncmp expect actual
+'
 
 test_expect_success 'rm files with two different errors' '
 	cat >expect <<-\EOF &&
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
       [not found] <1370874127-4326-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr>
@ 2013-06-10 14:22 ` Mathieu Lienard--Mayor
  0 siblings, 0 replies; 14+ messages in thread
From: Mathieu Lienard--Mayor @ 2013-06-10 14:22 UTC (permalink / raw)
  To: git
  Cc: gitster, Mathieu Lienard--Mayor, Jorge Juan Garcia Garcia, Matthieu Moy

Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.

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

would look like, with advice.rmHints=false:
	error: 'foo.txt' has changes staged in the index

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>
---

Changes since v1:
 -corrected the commit message where "rmHints=true" was supposed to be "false"
 -better use of English tenses in the commit message

 Documentation/config.txt |    3 +++
 advice.c                 |    2 ++
 advice.h                 |    1 +
 builtin/rm.c             |   11 +++++++----
 t/t3600-rm.sh            |   29 +++++++++++++++++++++++++++++
 5 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.
+	rmHints::
+		In case of failure in the output of linkgit:git-rm[1],
+		show directions on how to proceed from the current state.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
 int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
 
 static struct {
 	const char *name;
@@ -33,6 +34,7 @@ static struct {
 	{ "implicitidentity", &advice_implicit_identity },
 	{ "detachedhead", &advice_detached_head },
 	{ "setupstreamfailure", &advice_set_upstream_failure },
+	{ "rmhints", &advice_rm_hints },
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
 extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
 
 int git_default_advice_config(const char *var, const char *value);
 void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 76dfc5b..2226037 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
 
 		if (!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);
+				       "submodules) uses a .git directory%s"), name,
+				       advice_rm_hints
+				       ? "\n(use 'rm -rf' if you really want to remove "
+				       "it including all of its history)"
+				       : "");
 	}
 
 	return errs;
@@ -85,7 +87,8 @@ static int print_error_files(struct string_list *files_list,
 		strbuf_addf(&err_msg,
 			    "\n    %s",
 			    files_list->items[i].string);
-	strbuf_addstr(&err_msg, hints_msg);
+	if (advice_rm_hints)
+		strbuf_addstr(&err_msg, hints_msg);
 	errs = error("%s", err_msg.buf);
 
 	return errs;
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 08bc9bb..92f6146 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -707,6 +707,18 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'rm files with different staged content without hints' '
+	cat >expect << EOF &&
+error: the following files have staged content different from both the
+file and the HEAD:
+    bar.txt
+    foo.txt
+EOF
+	echo content2 >foo.txt &&
+	echo content2 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false rm foo.txt bar.txt 2>actual &&
+	test_cmp expect actual
+'
 
 test_expect_success 'rm file with local modification' '
 	cat >expect << EOF &&
@@ -720,6 +732,15 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'rm file with local modification without hints' '
+	cat >expect << EOF &&
+error: the following files have local modifications:
+    bar.txt
+EOF
+	echo content4 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false rm bar.txt 2>actual &&
+	test_cmp expect actual
+'
 
 test_expect_success 'rm file with changes in the index' '
     cat >expect << EOF &&
@@ -734,5 +755,13 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'rm file with changes in the index without hints' '
+	cat >expect << EOF &&
+error: the following files have changes staged in the index:
+    foo.txt
+EOF
+	test_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&
+	test_cmp expect actual
+'
 
 test_done
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-10  8:24         ` Matthieu Moy
@ 2013-06-10  8:26           ` Ramkumar Ramachandra
  0 siblings, 0 replies; 14+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-10  8:26 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Mathieu Liénard--Mayor, Mathieu Lienard--Mayor, git,
	gitster, Jorge Juan Garcia Garcia

Matthieu Moy wrote:
> I don't see why add and rm hints should be correlated, or I don't have
> the same advice as you in mind.
>
> Both have completely different meanings: the first is about .gitignore,
> and the second about not loosing data.

Right, my bad.  Please continue with rmHints, and optionally write an
addHints while at it.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-10  7:55       ` Ramkumar Ramachandra
@ 2013-06-10  8:24         ` Matthieu Moy
  2013-06-10  8:26           ` Ramkumar Ramachandra
  0 siblings, 1 reply; 14+ messages in thread
From: Matthieu Moy @ 2013-06-10  8:24 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Mathieu Liénard--Mayor, Mathieu Lienard--Mayor, git,
	gitster, Jorge Juan Garcia Garcia

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Mathieu Liénard--Mayor wrote:
>> I'm not so sure i understand. Do you mean rmHints should deactivate addHints
>> aswell, or do you mean that since we're introducing rmHints it would be
>> natural to introduce addHints ?
>
> More the latter, but I'm tilting towards addRmHints (or something)
> which affects both add and rm hints.

I don't see why add and rm hints should be correlated, or I don't have
the same advice as you in mind.

$ git add foo.txt
The following paths are ignored by one of your .gitignore files:
foo.txt
Use -f if you really want to add them.
fatal: no files added

$ git rm foo.txt 
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)

Both have completely different meanings: the first is about .gitignore,
and the second about not loosing data.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-10  7:52     ` Mathieu Liénard--Mayor
@ 2013-06-10  7:55       ` Ramkumar Ramachandra
  2013-06-10  8:24         ` Matthieu Moy
  0 siblings, 1 reply; 14+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-10  7:55 UTC (permalink / raw)
  To: Mathieu Liénard--Mayor
  Cc: Mathieu Lienard--Mayor, git, gitster, Jorge Juan Garcia Garcia,
	Matthieu Moy

Mathieu Liénard--Mayor wrote:
> I'm not so sure i understand. Do you mean rmHints should deactivate addHints
> aswell, or do you mean that since we're introducing rmHints it would be
> natural to introduce addHints ?

More the latter, but I'm tilting towards addRmHints (or something)
which affects both add and rm hints.

> Sorry about that, we'll work on it.

Nothing to be sorry about.  You're doing good work, and we're helping
you make it even better :)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-08 14:01   ` Ramkumar Ramachandra
@ 2013-06-10  7:52     ` Mathieu Liénard--Mayor
  2013-06-10  7:55       ` Ramkumar Ramachandra
  0 siblings, 1 reply; 14+ messages in thread
From: Mathieu Liénard--Mayor @ 2013-06-10  7:52 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Mathieu Lienard--Mayor, git, gitster, Jorge Juan Garcia Garcia,
	Matthieu Moy

Le 2013-06-08 16:01, Ramkumar Ramachandra a écrit :
> Mathieu Lienard--Mayor wrote:
>> As an example, the message:
>>         error: 'foo.txt' has changes staged in the index
>>         (use --cached to keep the file, or -f to force removal)
>>
>> would look like, with advice.rmHints=true:
>>         error: 'foo.txt' has changes staged in the index
>
> Um, have you switched the true with false?  advice.* variables are
> true by default, and I turn off all of them.
Whoops, my bad, I obviously meant false.
>
> Also, I think you can extend this to also remove add-advice.  Why
> would someone want to turn off advice from rm, but not add?  (Unsure
> about this)
I'm not so sure i understand. Do you mean rmHints should deactivate 
addHints aswell, or do you mean that since we're introducing rmHints it 
would be natural to introduce addHints ?
>
>> Similarly to advice.*, advice.rmHints has been added
>> to the config variables. By default, it is set to false, in order to
>> keep the messages the same as before. When set to true,  advice
>> are no longer included in the error messages.
>
> Ugh, why this roundabout-passive-past tone?  Use imperative tone like 
> this:
Sorry about that, we'll work on it.
>
> Introduce advice.rmHints to control the whether to display advice 
> when
> using 'git rm'.  Defaults to true, preserving current behavior.
-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-08  8:33 ` [PATCH 2/2] rm: introduce advice.rmHints to shorten messages Mathieu Lienard--Mayor
@ 2013-06-08 14:01   ` Ramkumar Ramachandra
  2013-06-10  7:52     ` Mathieu Liénard--Mayor
  0 siblings, 1 reply; 14+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-08 14:01 UTC (permalink / raw)
  To: Mathieu Lienard--Mayor
  Cc: git, gitster, Jorge Juan Garcia Garcia, Matthieu Moy

Mathieu Lienard--Mayor wrote:
> As an example, the message:
>         error: 'foo.txt' has changes staged in the index
>         (use --cached to keep the file, or -f to force removal)
>
> would look like, with advice.rmHints=true:
>         error: 'foo.txt' has changes staged in the index

Um, have you switched the true with false?  advice.* variables are
true by default, and I turn off all of them.

Also, I think you can extend this to also remove add-advice.  Why
would someone want to turn off advice from rm, but not add?  (Unsure
about this)

> Similarly to advice.*, advice.rmHints has been added
> to the config variables. By default, it is set to false, in order to
> keep the messages the same as before. When set to true,  advice
> are no longer included in the error messages.

Ugh, why this roundabout-passive-past tone?  Use imperative tone like this:

Introduce advice.rmHints to control the whether to display advice when
using 'git rm'.  Defaults to true, preserving current behavior.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
  2013-06-08  8:33 [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Lienard--Mayor
@ 2013-06-08  8:33 ` Mathieu Lienard--Mayor
  2013-06-08 14:01   ` Ramkumar Ramachandra
  0 siblings, 1 reply; 14+ messages in thread
From: Mathieu Lienard--Mayor @ 2013-06-08  8:33 UTC (permalink / raw)
  To: git
  Cc: gitster, Mathieu Liénard--Mayor, Jorge Juan Garcia Garcia,
	Matthieu Moy

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

Similarly to advice.*, advice.rmHints has been added
to the config variables. By default, it is set to false, in order to
keep the messages the same as before. When set to true,  advice
are no longer included in the error messages.

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

would look like, with advice.rmHints=true:
	error: 'foo.txt' has changes staged in the index

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>
---
 Documentation/config.txt |    3 +++
 advice.c                 |    2 ++
 advice.h                 |    1 +
 builtin/rm.c             |   38 ++++++++++++++++++++++++++------------
 t/t3600-rm.sh            |   32 ++++++++++++++++++++++++++++++++
 5 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.
+	rmHints::
+		In case of failure in the output of linkgit:git-rm[1],
+		show directions on how to proceed from the current state.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
 int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
 
 static struct {
 	const char *name;
@@ -33,6 +34,7 @@ static struct {
 	{ "implicitidentity", &advice_implicit_identity },
 	{ "detachedhead", &advice_detached_head },
 	{ "setupstreamfailure", &advice_set_upstream_failure },
+	{ "rmhints", &advice_rm_hints },
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
 extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
 
 int git_default_advice_config(const char *var, const char *value);
 void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 5b2abd2..38ceb73 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
 
 		if (!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);
+				       "submodules) uses a .git directory%s"), name,
+				       advice_rm_hints
+				       ? "\n(use 'rm -rf' if you really want to remove "
+				       "it including all of its history)"
+				       : "");
 	}
 
 	return errs;
@@ -200,21 +202,33 @@ static int check_local_mod(unsigned char *head, int index_only)
 
 	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);
+			       "different from both the\nfile and the HEAD:%s%s"
+			       ), files_staged.buf,
+			       advice_rm_hints
+			       ? "\n(use -f to force removal)"
+			       : "");
 	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);
+			       "in the index:%s%s"), files_cached.buf,
+			       advice_rm_hints
+			       ? "\n(use --cached to keep the file, "
+			       "or -f to force removal)"
+			       : "");
 	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);
+			       "submodule) use a .git directory:%s%s"),
+			       files_submodule.buf,
+			       advice_rm_hints
+			       ? "\n(use 'rm -rf' if you really want to remove "
+			       "it including all of its history)"
+			       : "");
 	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);
+			       "%s%s"), files_local.buf,
+			       advice_rm_hints
+			       ? "\n(use --cached to keep the file, or -f to "
+			       "force removal)"
+			       : "");
 
 	return errs;
 }
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index e0f3166..ab10cc6 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -707,6 +707,19 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'rm files with different staged content without hints' '
+	cat >expect << EOF &&
+error: the following files have staged content different from both the
+file and the HEAD:
+	bar.txt
+	foo.txt
+EOF
+	echo content2 >foo.txt &&
+	echo content2 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false 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:
@@ -719,6 +732,16 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'rm file with local modification without hints' '
+	cat >expect << EOF &&
+error: the following files have local modifications:
+	bar.txt
+EOF
+	echo content4 >bar.txt &&
+	test_must_fail git -c advice.rmhints=false rm bar.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:
@@ -732,4 +755,13 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'rm file with changes in the index without hints' '
+	cat >expect << EOF &&
+error: the following files have changes staged in the index:
+	foo.txt
+EOF
+	test_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-06-12  8:07 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-10 12:51 [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Lienard--Mayor
2013-06-10 12:51 ` [PATCH 2/2] rm: introduce advice.rmHints to shorten messages Mathieu Lienard--Mayor
2013-06-10 12:54   ` Mathieu Liénard--Mayor
2013-06-10 16:57     ` Junio C Hamano
2013-06-10 17:17       ` Mathieu Liénard--Mayor
2013-06-10 12:53 ` [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Liénard--Mayor
  -- strict thread matches above, loose matches on Subject: below --
2013-06-12  8:06 [PATCH v5 " Mathieu Lienard--Mayor
2013-06-12  8:06 ` [PATCH 2/2] rm: introduce advice.rmHints to shorten messages Mathieu Lienard--Mayor
     [not found] <1370874127-4326-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr>
2013-06-10 14:22 ` Mathieu Lienard--Mayor
2013-06-08  8:33 [PATCH 1/2] rm: better error message on failure for multiple files Mathieu Lienard--Mayor
2013-06-08  8:33 ` [PATCH 2/2] rm: introduce advice.rmHints to shorten messages Mathieu Lienard--Mayor
2013-06-08 14:01   ` Ramkumar Ramachandra
2013-06-10  7:52     ` Mathieu Liénard--Mayor
2013-06-10  7:55       ` Ramkumar Ramachandra
2013-06-10  8:24         ` Matthieu Moy
2013-06-10  8:26           ` Ramkumar Ramachandra

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 NNTP newsgroup(s).