Merge branch 'mm/rm-coalesce-errors'
authorJunio C Hamano <gitster@pobox.com>
Mon, 24 Jun 2013 20:48:35 +0000 (13:48 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 24 Jun 2013 20:48:35 +0000 (13:48 -0700)
Give a single message followed by list of paths from "git rm" to
report multiple paths that cannot be removed.

* mm/rm-coalesce-errors:
rm: introduce advice.rmHints to shorten messages
rm: better error message on failure for multiple files

Documentation/config.txt
advice.c
advice.h
builtin/rm.c
t/t3600-rm.sh
index 311fcebaf51034a0788df971749fe80dbc3c8782..1153585aa2e2d19dd41f8cf81465e8b6d858a028 100644 (file)
@@ -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::
index 54315cbd0a554bdeb1ace160a5df7cf6c485e559..2a52098a29dd1c2192edb6fff8ca6979383e77c6 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -15,6 +15,7 @@ int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
 int advice_object_name_warning = 1;
+int advice_rm_hints = 1;
 
 static struct {
        const char *name;
@@ -35,6 +36,7 @@ static struct {
        { "detachedhead", &advice_detached_head },
        { "setupstreamfailure", &advice_set_upstream_failure },
        { "object_name_warning", &advice_object_name_warning },
+       { "rmhints", &advice_rm_hints },
 
        /* make this an alias for backward compatibility */
        { "pushnonfastforward", &advice_push_update_rejected }
index fefe39ac5caf5a1e8b95683a4c8dcc443df16d32..93a7d110ea94b055476644fd5ecbec43bb0aaf92 100644 (file)
--- a/advice.h
+++ b/advice.h
@@ -18,6 +18,7 @@ extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
 extern int advice_object_name_warning;
+extern int advice_rm_hints;
 
 int git_default_advice_config(const char *var, const char *value);
 void advise(const char *advice, ...);
index 7b91d52f39004dbfcc9b0dc5b8aadf3df5c6d320..06025a2e751fbc2b2de3b024919c381cd2176c51 100644 (file)
@@ -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,32 @@ 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);
+               if (advice_rm_hints)
+                       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 +84,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 +111,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 +205,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;
 }
 
index 0c44e9f5d04af8d038a1e78ed5fdf892ce4dd5b5..5c87b55645e249929585c2ed0e7bcf98a1b62da3 100755 (executable)
@@ -687,4 +687,100 @@ 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 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 &&
+       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 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 &&
+       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 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 &&
+       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