Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
writes:
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--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v2:
-couple typo in commit message and in code
-rename and redefinition of the intermediate function
-move the 4 if(nr) inside the function
builtin/rm.c | 71 +---
t/t3600-rm.sh | 67 +
2 files changed, 124 insertions(+), 14 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..07306eb 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -70,6 +70,24 @@ static int check_submodules_use_gitfiles(void)
return errs;
}
+static void print_eventual_error_files(struct string_list *files_list,
+const char *main_msg,
+const char *hints_msg,
+int *errs)
Hrm, I do not see the point of eventual there, by the way. Are
there other kinds of error files?
+{
+ if (files_list-nr) {
+ struct strbuf err_msg = STRBUF_INIT;
+ int i;
+ strbuf_addstr(err_msg, main_msg);
+ for (i = 0; i files_list-nr; i++)
+ strbuf_addf(err_msg,
+ \n%s,
Is there an implication of having always 4 spaces here to l10n/i18n
here? I am wondering if it should be _(\n%s).
+ files_list-items[i].string);
+ strbuf_addstr(err_msg, hints_msg);
+ *errs = error(%s, err_msg.buf);
There needs a strbuf_release(err_msg) somewhere before leaving this
scope to avoid leaking its buffer, no?
+ }
+}
+
static int check_local_mod(unsigned char *head, int index_only)
{
/*
@@ -82,6 +100,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 +194,49 @@ 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))
+ 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)) {
+ string_list_append(files_submodule,
+name);
+ } else {
+ string_list_append(files_local, name);
+ }
The innermost if/else no longer needs braces. Also even though it
may push it slightly over 80-column, I think the files_submodule
side of string_list_append() is easier to read if it were on a
single line.
+ print_eventual_error_files(files_staged,
+_(the following files have staged
+ content different from both the
+ \nfile and the HEAD:),
+_(\n(use -f to force removal)),
+errs);
Hmph. I wonder if we want to properly i18n plurals, depending on
the number of files, e.g.
