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 <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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Mathieu Lienard--Mayor committed Jun 12, 2013 at 10:06 UTC 914dc0289d1df75cfa744cea8ec84cb529cbc791
2 files changed +148 -19
builtin/rm.c
+81 -19
@@ -9,6 +9,7 @@
9 #include "cache-tree.h"
10 #include "tree-walk.h"
11 #include "parse-options.h"
12 +#include "string-list.h"
13 #include "submodule.h"
14
15 static const char * const builtin_rm_usage[] = {
@@ -36,10 +37,31 @@ static int get_ours_cache_pos(const char *path, int pos)
37 return -1;
38 }
39
40 +static void print_error_files(struct string_list *files_list,
41 + const char *main_msg,
42 + const char *hints_msg,
43 + int *errs)
44 +{
45 + if (files_list->nr) {
46 + int i;
47 + struct strbuf err_msg = STRBUF_INIT;
48 +
49 + strbuf_addstr(&err_msg, main_msg);
50 + for (i = 0; i < files_list->nr; i++)
51 + strbuf_addf(&err_msg,
52 + "\n %s",
53 + files_list->items[i].string);
54 + strbuf_addstr(&err_msg, hints_msg);
55 + *errs = error("%s", err_msg.buf);
56 + strbuf_release(&err_msg);
57 + }
58 +}
59 +
60 static int check_submodules_use_gitfiles(void)
61 {
62 int i;
63 int errs = 0;
64 + struct string_list files = STRING_LIST_INIT_NODUP;
65
66 for (i = 0; i < list.nr; i++) {
67 const char *name = list.entry[i].name;
@@ -61,11 +83,18 @@ static int check_submodules_use_gitfiles(void)
83 continue;
84
85 if (!submodule_uses_gitfile(name))
64 - errs = error(_("submodule '%s' (or one of its nested "
65 - "submodules) uses a .git directory\n"
66 - "(use 'rm -rf' if you really want to remove "
67 - "it including all of its history)"), name);
86 + string_list_append(&files, name);
87 }
88 + print_error_files(&files,
89 + Q_("the following submodule (or one of its nested "
90 + "submodules)\n uses a .git directory:",
91 + "the following submodules (or one of its nested "
92 + "submodules)\n use a .git directory:",
93 + files.nr),
94 + _("\n(use 'rm -rf' if you really want to remove "
95 + "it including all of its history)"),
96 + &errs);
97 + string_list_clear(&files, 0);
98
99 return errs;
100 }
@@ -81,6 +110,10 @@ static int check_local_mod(unsigned char *head, int index_only)
110 */
111 int i, no_head;
112 int errs = 0;
113 + struct string_list files_staged = STRING_LIST_INIT_NODUP;
114 + struct string_list files_cached = STRING_LIST_INIT_NODUP;
115 + struct string_list files_submodule = STRING_LIST_INIT_NODUP;
116 + struct string_list files_local = STRING_LIST_INIT_NODUP;
117
118 no_head = is_null_sha1(head);
119 for (i = 0; i < list.nr; i++) {
@@ -171,29 +204,58 @@ static int check_local_mod(unsigned char *head, int index_only)
204 */
205 if (local_changes && staged_changes) {
206 if (!index_only || !(ce->ce_flags & CE_INTENT_TO_ADD))
174 - errs = error(_("'%s' has staged content different "
175 - "from both the file and the HEAD\n"
176 - "(use -f to force removal)"), name);
207 + string_list_append(&files_staged, name);
208 }
209 else if (!index_only) {
210 if (staged_changes)
180 - errs = error(_("'%s' has changes staged in the index\n"
181 - "(use --cached to keep the file, "
182 - "or -f to force removal)"), name);
211 + string_list_append(&files_cached, name);
212 if (local_changes) {
213 if (S_ISGITLINK(ce->ce_mode) &&
185 - !submodule_uses_gitfile(name)) {
186 - errs = error(_("submodule '%s' (or one of its nested "
187 - "submodules) uses a .git directory\n"
188 - "(use 'rm -rf' if you really want to remove "
189 - "it including all of its history)"), name);
190 - } else
191 - errs = error(_("'%s' has local modifications\n"
192 - "(use --cached to keep the file, "
193 - "or -f to force removal)"), name);
214 + !submodule_uses_gitfile(name))
215 + string_list_append(&files_submodule, name);
216 + else
217 + string_list_append(&files_local, name);
218 }
219 }
220 }
221 + print_error_files(&files_staged,
222 + Q_("the following file has staged content different "
223 + "from both the\nfile and the HEAD:",
224 + "the following files have staged content different"
225 + " from both the\nfile and the HEAD:",
226 + files_staged.nr),
227 + _("\n(use -f to force removal)"),
228 + &errs);
229 + string_list_clear(&files_staged, 0);
230 + print_error_files(&files_cached,
231 + Q_("the following file has changes "
232 + "staged in the index:",
233 + "the following files have changes "
234 + "staged in the index:", files_cached.nr),
235 + _("\n(use --cached to keep the file,"
236 + " or -f to force removal)"),
237 + &errs);
238 + string_list_clear(&files_cached, 0);
239 + print_error_files(&files_submodule,
240 + Q_("the following submodule (or one of its nested "
241 + "submodule)\nuses a .git directory:",
242 + "the following submodules (or one of its nested "
243 + "submodule)\nuse a .git directory:",
244 + files_submodule.nr),
245 + _("\n(use 'rm -rf' if you really "
246 + "want to remove it including all "
247 + "of its history)"),
248 + &errs);
249 + string_list_clear(&files_submodule, 0);
250 + print_error_files(&files_local,
251 + Q_("the following file has local modifications:",
252 + "the following files have local modifications:",
253 + files_local.nr),
254 + _("\n(use --cached to keep the file,"
255 + " or -f to force removal)"),
256 + &errs);
257 + string_list_clear(&files_local, 0);
258 +
259 return errs;
260 }
261
t/t3600-rm.sh
+67
@@ -687,4 +687,71 @@ test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' '
687 test_path_is_file e/f
688 '
689
690 +test_expect_success 'setup for testing rm messages' '
691 + >bar.txt &&
692 + >foo.txt &&
693 + git add bar.txt foo.txt
694 +'
695 +
696 +test_expect_success 'rm files with different staged content' '
697 + cat >expect <<-\EOF &&
698 + error: the following files have staged content different from both the
699 + file and the HEAD:
700 + bar.txt
701 + foo.txt
702 + (use -f to force removal)
703 + EOF
704 + echo content1 >foo.txt &&
705 + echo content1 >bar.txt &&
706 + test_must_fail git rm foo.txt bar.txt 2>actual &&
707 + test_i18ncmp expect actual
708 +'
709 +
710 +
711 +test_expect_success 'rm file with local modification' '
712 + cat >expect <<-\EOF &&
713 + error: the following file has local modifications:
714 + foo.txt
715 + (use --cached to keep the file, or -f to force removal)
716 + EOF
717 + git commit -m "testing rm 3" &&
718 + echo content3 >foo.txt &&
719 + test_must_fail git rm foo.txt 2>actual &&
720 + test_i18ncmp expect actual
721 +'
722 +
723 +
724 +test_expect_success 'rm file with changes in the index' '
725 + cat >expect <<-\EOF &&
726 + error: the following file has changes staged in the index:
727 + foo.txt
728 + (use --cached to keep the file, or -f to force removal)
729 + EOF
730 + git reset --hard &&
731 + echo content5 >foo.txt &&
732 + git add foo.txt &&
733 + test_must_fail git rm foo.txt 2>actual &&
734 + test_i18ncmp expect actual
735 +'
736 +
737 +
738 +test_expect_success 'rm files with two different errors' '
739 + cat >expect <<-\EOF &&
740 + error: the following file has staged content different from both the
741 + file and the HEAD:
742 + foo1.txt
743 + (use -f to force removal)
744 + error: the following file has changes staged in the index:
745 + bar1.txt
746 + (use --cached to keep the file, or -f to force removal)
747 + EOF
748 + echo content >foo1.txt &&
749 + git add foo1.txt &&
750 + echo content6 >foo1.txt &&
751 + echo content6 >bar1.txt &&
752 + git add bar1.txt &&
753 + test_must_fail git rm bar1.txt foo1.txt 2>actual &&
754 + test_i18ncmp expect actual
755 +'
756 +
757 test_done