From 914dc0289d1df75cfa744cea8ec84cb529cbc791 Mon Sep 17 00:00:00 2001 From: Mathieu Lienard--Mayor Date: Wed, 12 Jun 2013 10:06:43 +0200 Subject: [PATCH 1/2] rm: better error message on failure for multiple files 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 Signed-off-by: Jorge Juan Garcia Garcia Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- builtin/rm.c | 100 ++++++++++++++++++++++++++++++++++++++++---------- t/t3600-rm.sh | 67 +++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 19 deletions(-) diff --git a/builtin/rm.c b/builtin/rm.c index 7b91d52f39..5d0c0683da 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -9,6 +9,7 @@ #include "cache-tree.h" #include "tree-walk.h" #include "parse-options.h" +#include "string-list.h" #include "submodule.h" static const char * const builtin_rm_usage[] = { @@ -36,10 +37,31 @@ static int get_ours_cache_pos(const char *path, int pos) return -1; } +static void print_error_files(struct string_list *files_list, + const char *main_msg, + const char *hints_msg, + int *errs) +{ + if (files_list->nr) { + int i; + struct strbuf err_msg = STRBUF_INIT; + + strbuf_addstr(&err_msg, main_msg); + for (i = 0; i < files_list->nr; i++) + strbuf_addf(&err_msg, + "\n %s", + files_list->items[i].string); + strbuf_addstr(&err_msg, hints_msg); + *errs = error("%s", err_msg.buf); + strbuf_release(&err_msg); + } +} + static int check_submodules_use_gitfiles(void) { int i; int errs = 0; + struct string_list files = STRING_LIST_INIT_NODUP; for (i = 0; i < list.nr; i++) { const char *name = list.entry[i].name; @@ -61,11 +83,18 @@ static int check_submodules_use_gitfiles(void) continue; 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); + string_list_append(&files, name); } + print_error_files(&files, + Q_("the following submodule (or one of its nested " + "submodules)\n uses a .git directory:", + "the following submodules (or one of its nested " + "submodules)\n use a .git directory:", + files.nr), + _("\n(use 'rm -rf' if you really want to remove " + "it including all of its history)"), + &errs); + string_list_clear(&files, 0); return errs; } @@ -81,6 +110,10 @@ 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++) { @@ -171,29 +204,58 @@ 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); + !submodule_uses_gitfile(name)) + string_list_append(&files_submodule, name); + else + string_list_append(&files_local, name); } } } + print_error_files(&files_staged, + Q_("the following file has staged content different " + "from both the\nfile and the HEAD:", + "the following files have staged content different" + " from both the\nfile and the HEAD:", + files_staged.nr), + _("\n(use -f to force removal)"), + &errs); + string_list_clear(&files_staged, 0); + print_error_files(&files_cached, + Q_("the following file has changes " + "staged in the index:", + "the following files have changes " + "staged in the index:", files_cached.nr), + _("\n(use --cached to keep the file," + " or -f to force removal)"), + &errs); + string_list_clear(&files_cached, 0); + print_error_files(&files_submodule, + Q_("the following submodule (or one of its nested " + "submodule)\nuses a .git directory:", + "the following submodules (or one of its nested " + "submodule)\nuse a .git directory:", + files_submodule.nr), + _("\n(use 'rm -rf' if you really " + "want to remove it including all " + "of its history)"), + &errs); + string_list_clear(&files_submodule, 0); + print_error_files(&files_local, + Q_("the following file has local modifications:", + "the following files have local modifications:", + files_local.nr), + _("\n(use --cached to keep the file," + " or -f to force removal)"), + &errs); + string_list_clear(&files_local, 0); + return errs; } diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 0c44e9f5d0..902993bb65 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -687,4 +687,71 @@ 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_i18ncmp expect actual +' + + +test_expect_success 'rm file with local modification' ' + cat >expect <<-\EOF && + error: the following file has 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_i18ncmp expect actual +' + + +test_expect_success 'rm file with changes in the index' ' + cat >expect <<-\EOF && + error: the following file has 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_i18ncmp expect actual +' + + +test_expect_success 'rm files with two different errors' ' + cat >expect <<-\EOF && + error: the following file has staged content different from both the + file and the HEAD: + foo1.txt + (use -f to force removal) + error: the following file has changes staged in the index: + bar1.txt + (use --cached to keep the file, or -f to force removal) + EOF + echo content >foo1.txt && + git add foo1.txt && + echo content6 >foo1.txt && + echo content6 >bar1.txt && + git add bar1.txt && + test_must_fail git rm bar1.txt foo1.txt 2>actual && + test_i18ncmp expect actual +' + test_done From 7e30944622573ebdf87beed057b098af7360234c Mon Sep 17 00:00:00 2001 From: Mathieu Lienard--Mayor Date: Wed, 12 Jun 2013 10:06:44 +0200 Subject: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages 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 Signed-off-by: Jorge Juan Garcia Garcia Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- 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 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 6e53fc5074..eb04479804 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 a8deee6e64..a4c169ca38 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 94caa32f92..36104c4776 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 5d0c0683da..06025a2e75 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -51,7 +51,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 902993bb65..5c87b55645 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 &&