{"thread":{"id":"34095","subject":"[PATCH v3 1/2] rm: better error message on failure for multiple files","startedAt":"2013-06-10T16:05:13Z","lastAt":"2013-06-10T16:05:14Z","messageCount":2,"participants":["Mathieu Lienard--Mayor"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"220309","messageId":"1370880314-4825-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr","threadId":"34095","inReplyTo":null,"subject":"[PATCH v3 1/2] rm: better error message on failure for multiple files","fromName":"Mathieu Lienard--Mayor","fromEmail":"mathieu.lienard--mayor@ensimag.imag.fr","sentAt":"2013-06-10T16:05:13Z","receivedAt":"2013-06-10T16:05:13Z","isPatch":true,"sender":{"key":"mathieu.lienard--mayor@ensimag.imag.fr","avatar":null},"body":"When 'git rm' fails, it now displays a single message\nwith the list of files involved, instead of displaying\na list of messages with one file each.\n\nAs an example, the old message:\n\terror: 'foo.txt' has changes staged in the index\n\t(use --cached to keep the file, or -f to force removal)\n\terror: 'bar.txt' has changes staged in the index\n\t(use --cached to keep the file, or -f to force removal)\n\nwould now be displayed as:\n\terror: the following files have changes staged in the index:\n\t    foo.txt\n\t    bar.txt\n\t(use --cached to keep the file, or -f to force removal)\n\nSigned-off-by: Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>\nSigned-off-by: Jorge Juan Garcia Garcia <Jorge-Juan.Garcia-Garcia@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n---\n\nChanges since v2:\n -couple typo in commit message and in code\n -rename and redefinition of the intermediate function\n -move if(....nr) inside the function\n\n builtin/rm.c  |   71 +++++++++++++++++++++++++++++++++++++++++++++-----------\n t/t3600-rm.sh |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 124 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex 7b91d52..07306eb 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -70,6 +70,24 @@ static int check_submodules_use_gitfiles(void)\n \treturn errs;\n }\n \n+static void print_eventual_error_files(struct string_list *files_list,\n+\t\t\t\t       const char *main_msg,\n+\t\t\t\t       const char *hints_msg,\n+\t\t\t\t       int *errs)\n+{\n+\tif (files_list->nr) {\n+\t\tstruct strbuf err_msg = STRBUF_INIT;\n+\t\tint i;\n+\t\tstrbuf_addstr(&err_msg, main_msg);\n+\t\tfor (i = 0; i < files_list->nr; i++)\n+\t\t\tstrbuf_addf(&err_msg,\n+\t\t\t\t    \"\\n    %s\",\n+\t\t\t\t    files_list->items[i].string);\n+\t\tstrbuf_addstr(&err_msg, hints_msg);\n+\t\t*errs = error(\"%s\", err_msg.buf);\n+\t}\n+}\n+\n static int check_local_mod(unsigned char *head, int index_only)\n {\n \t/*\n@@ -82,6 +100,11 @@ static int check_local_mod(unsigned char *head, int index_only)\n \tint i, no_head;\n \tint errs = 0;\n \n+\tstruct string_list files_staged = STRING_LIST_INIT_NODUP;\n+\tstruct string_list files_cached = STRING_LIST_INIT_NODUP;\n+\tstruct string_list files_submodule = STRING_LIST_INIT_NODUP;\n+\tstruct string_list files_local = STRING_LIST_INIT_NODUP;\n+\n \tno_head = is_null_sha1(head);\n \tfor (i = 0; i < list.nr; i++) {\n \t\tstruct stat st;\n@@ -171,29 +194,49 @@ static int check_local_mod(unsigned char *head, int index_only)\n \t\t */\n \t\tif (local_changes && staged_changes) {\n \t\t\tif (!index_only || !(ce->ce_flags & CE_INTENT_TO_ADD))\n-\t\t\t\terrs = error(_(\"'%s' has staged content different \"\n-\t\t\t\t\t     \"from both the file and the HEAD\\n\"\n-\t\t\t\t\t     \"(use -f to force removal)\"), name);\n+\t\t\t\tstring_list_append(&files_staged, name);\n \t\t}\n \t\telse if (!index_only) {\n \t\t\tif (staged_changes)\n-\t\t\t\terrs = error(_(\"'%s' has changes staged in the index\\n\"\n-\t\t\t\t\t     \"(use --cached to keep the file, \"\n-\t\t\t\t\t     \"or -f to force removal)\"), name);\n+\t\t\t\tstring_list_append(&files_cached, name);\n \t\t\tif (local_changes) {\n \t\t\t\tif (S_ISGITLINK(ce->ce_mode) &&\n \t\t\t\t    !submodule_uses_gitfile(name)) {\n-\t\t\t\t\terrs = error(_(\"submodule '%s' (or one of its nested \"\n-\t\t\t\t\t\t     \"submodules) uses a .git directory\\n\"\n-\t\t\t\t\t\t     \"(use 'rm -rf' if you really want to remove \"\n-\t\t\t\t\t\t     \"it including all of its history)\"), name);\n-\t\t\t\t} else\n-\t\t\t\t\terrs = error(_(\"'%s' has local modifications\\n\"\n-\t\t\t\t\t\t     \"(use --cached to keep the file, \"\n-\t\t\t\t\t\t     \"or -f to force removal)\"), name);\n+\t\t\t\t\tstring_list_append(&files_submodule,\n+\t\t\t\t\t\t\t   name);\n+\t\t\t\t} else {\n+\t\t\t\t\tstring_list_append(&files_local, name);\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n \t}\n+\tprint_eventual_error_files(&files_staged,\n+\t\t\t\t   _(\"the following files have staged \"\n+\t\t\t\t     \"content different from both the\"\n+\t\t\t\t     \"\\nfile and the HEAD:\"),\n+\t\t\t\t   _(\"\\n(use -f to force removal)\"),\n+\t\t\t\t   &errs);\n+\tprint_eventual_error_files(&files_cached,\n+\t\t\t\t   _(\"the following files have changes \"\n+\t\t\t\t     \"staged in the index:\"),\n+\t\t\t\t   _(\"\\n(use --cached to keep the file,\"\n+\t\t\t\t     \" or -f to force removal)\"),\n+\t\t\t\t   &errs);\n+\tprint_eventual_error_files(&files_submodule,\n+\t\t\t\t   _(\"the following submodules (or one \"\n+\t\t\t\t     \"of its nested submodule) use a \"\n+\t\t\t\t     \".git directory:\"),\n+\t\t\t\t   _(\"\\n(use 'rm -rf' if you really \"\n+\t\t\t\t     \"want to remove it including all \"\n+\t\t\t\t     \"of its history)\"),\n+\t\t\t\t   &errs);\n+\tprint_eventual_error_files(&files_local,\n+\t\t\t\t   _(\"the following files have \"\n+\t\t\t\t     \"local modifications:\"),\n+\t\t\t\t   _(\"\\n(use --cached to keep the file,\"\n+\t\t\t\t     \" or -f to force removal)\"),\n+\t\t\t\t   &errs);\n+\n \treturn errs;\n }\n \ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 0c44e9f..10dd380 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -687,4 +687,71 @@ test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' '\n \ttest_path_is_file e/f\n '\n \n+test_expect_success 'setup for testing rm messages' '\n+\t>bar.txt &&\n+\t>foo.txt &&\n+\tgit add bar.txt foo.txt\n+'\n+\n+test_expect_success 'rm files with different staged content' '\n+\tcat >expect << EOF &&\n+error: the following files have staged content different from both the\n+file and the HEAD:\n+    bar.txt\n+    foo.txt\n+(use -f to force removal)\n+EOF\n+\techo content1 >foo.txt &&\n+\techo content1 >bar.txt &&\n+\ttest_must_fail git rm foo.txt bar.txt 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+\n+test_expect_success 'rm file with local modification' '\n+\tcat >expect << EOF &&\n+error: the following files have local modifications:\n+    foo.txt\n+(use --cached to keep the file, or -f to force removal)\n+EOF\n+\tgit commit -m \"testing rm 3\" &&\n+\techo content3 >foo.txt &&\n+\ttest_must_fail git rm foo.txt 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+\n+test_expect_success 'rm file with changes in the index' '\n+    cat >expect << EOF &&\n+error: the following files have changes staged in the index:\n+    foo.txt\n+(use --cached to keep the file, or -f to force removal)\n+EOF\n+\tgit reset --hard &&\n+\techo content5 >foo.txt &&\n+\tgit add foo.txt &&\n+\ttest_must_fail git rm foo.txt 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+\n+test_expect_success 'rm files with two different errors' '\n+\tcat >expect << EOF &&\n+error: the following files have staged content different from both the\n+file and the HEAD:\n+    foo1.txt\n+(use -f to force removal)\n+error: the following files have changes staged in the index:\n+    bar1.txt\n+(use --cached to keep the file, or -f to force removal)\n+EOF\n+\techo content >foo1.txt &&\n+\tgit add foo1.txt &&\n+\techo content6 >foo1.txt &&\n+\techo content6 >bar1.txt &&\n+\tgit add bar1.txt &&\n+\ttest_must_fail git rm bar1.txt foo1.txt 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.7.8\n"},{"id":"220310","messageId":"1370880314-4825-2-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr","threadId":"34095","inReplyTo":"1370880314-4825-1-git-send-email-Mathieu.Lienard--Mayor@ensimag.imag.fr","subject":"[PATCH v3 2/2] rm: introduce advice.rmHints to shorten messages","fromName":"Mathieu Lienard--Mayor","fromEmail":"mathieu.lienard--mayor@ensimag.imag.fr","sentAt":"2013-06-10T16:05:14Z","receivedAt":"2013-06-10T16:05:14Z","isPatch":true,"sender":{"key":"mathieu.lienard--mayor@ensimag.imag.fr","avatar":null},"body":"Introduce advice.rmHints to choose whether to display advice or not\nwhen git rm fails. Defaults to true, in order to preserve current behavior.\n\nAs an example, the message:\n\terror: 'foo.txt' has changes staged in the index\n\t(use --cached to keep the file, or -f to force removal)\n\nwould look like, with advice.rmHints=false:\n\terror: 'foo.txt' has changes staged in the index\n\nSigned-off-by: Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>\nSigned-off-by: Jorge Juan Garcia Garcia <Jorge-Juan.Garcia-Garcia@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n---\n Documentation/config.txt |    3 +++\n advice.c                 |    2 ++\n advice.h                 |    1 +\n builtin/rm.c             |   11 +++++++----\n t/t3600-rm.sh            |   29 +++++++++++++++++++++++++++++\n 5 files changed, 42 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 6e53fc5..eb04479 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -199,6 +199,9 @@ advice.*::\n \tamWorkDir::\n \t\tAdvice that shows the location of the patch file when\n \t\tlinkgit:git-am[1] fails to apply it.\n+\trmHints::\n+\t\tIn case of failure in the output of linkgit:git-rm[1],\n+\t\tshow directions on how to proceed from the current state.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex a8deee6..a4c169c 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;\n int advice_implicit_identity = 1;\n int advice_detached_head = 1;\n int advice_set_upstream_failure = 1;\n+int advice_rm_hints = 1;\n \n static struct {\n \tconst char *name;\n@@ -33,6 +34,7 @@ static struct {\n \t{ \"implicitidentity\", &advice_implicit_identity },\n \t{ \"detachedhead\", &advice_detached_head },\n \t{ \"setupstreamfailure\", &advice_set_upstream_failure },\n+\t{ \"rmhints\", &advice_rm_hints },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushnonfastforward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex 94caa32..36104c4 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;\n extern int advice_implicit_identity;\n extern int advice_detached_head;\n extern int advice_set_upstream_failure;\n+extern int advice_rm_hints;\n \n int git_default_advice_config(const char *var, const char *value);\n void advise(const char *advice, ...);\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex 07306eb..c991fe6 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)\n \n \t\tif (!submodule_uses_gitfile(name))\n \t\t\terrs = error(_(\"submodule '%s' (or one of its nested \"\n-\t\t\t\t     \"submodules) uses a .git directory\\n\"\n-\t\t\t\t     \"(use 'rm -rf' if you really want to remove \"\n-\t\t\t\t     \"it including all of its history)\"), name);\n+\t\t\t\t       \"submodules) uses a .git directory%s\"), name,\n+\t\t\t\t       advice_rm_hints\n+\t\t\t\t       ? \"\\n(use 'rm -rf' if you really want to remove \"\n+\t\t\t\t       \"it including all of its history)\"\n+\t\t\t\t       : \"\");\n \t}\n \n \treturn errs;\n@@ -83,7 +85,8 @@ static void print_eventual_error_files(struct string_list *files_list,\n \t\t\tstrbuf_addf(&err_msg,\n \t\t\t\t    \"\\n    %s\",\n \t\t\t\t    files_list->items[i].string);\n-\t\tstrbuf_addstr(&err_msg, hints_msg);\n+\t\tif (advice_rm_hints)\n+\t\t\tstrbuf_addstr(&err_msg, hints_msg);\n \t\t*errs = error(\"%s\", err_msg.buf);\n \t}\n }\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 10dd380..74f048c 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -707,6 +707,18 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'rm files with different staged content without hints' '\n+\tcat >expect << EOF &&\n+error: the following files have staged content different from both the\n+file and the HEAD:\n+    bar.txt\n+    foo.txt\n+EOF\n+\techo content2 >foo.txt &&\n+\techo content2 >bar.txt &&\n+\ttest_must_fail git -c advice.rmhints=false rm foo.txt bar.txt 2>actual &&\n+\ttest_cmp expect actual\n+'\n \n test_expect_success 'rm file with local modification' '\n \tcat >expect << EOF &&\n@@ -720,6 +732,15 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'rm file with local modification without hints' '\n+\tcat >expect << EOF &&\n+error: the following files have local modifications:\n+    bar.txt\n+EOF\n+\techo content4 >bar.txt &&\n+\ttest_must_fail git -c advice.rmhints=false rm bar.txt 2>actual &&\n+\ttest_cmp expect actual\n+'\n \n test_expect_success 'rm file with changes in the index' '\n     cat >expect << EOF &&\n@@ -734,6 +755,14 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'rm file with changes in the index without hints' '\n+\tcat >expect << EOF &&\n+error: the following files have changes staged in the index:\n+    foo.txt\n+EOF\n+\ttest_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&\n+\ttest_cmp expect actual\n+'\n \n test_expect_success 'rm files with two different errors' '\n \tcat >expect << EOF &&\n-- \n1.7.8\n"}]}