Add -q option to "git rm" to suppress output when there aren't errors.

This suppresses the output of "rm" commands, and also exits with a zero exit code when no files match. This allows "git rm" (and more importantly, "git rm -r") to be used as an index filter with cg-admin-rewritehist.

---
 Documentation/git-rm.txt |    4 ++++
 builtin-rm.c             |   26 ++++++++++++++++++++------
 t/t3600-rm.sh            |   28 ++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 6 deletions(-)

index 6feebc0..c354134 100644
@@ -33,6 +33,10 @@ OPTIONS
         Don't actually remove the file(s), just show if they exist in
         the index.
 
+-q::
+	Don't output the names of the files being removed, and exit
+	with a zero status even if no files matched.
+
 -r::
         Allow recursive removal when a leading directory name is
         given.

index 8a0738f..3d438de 100644
@@ -10,7 +10,7 @@
 #include "tree-walk.h"
 
 static const char builtin_rm_usage[] =
-"git-rm [-f] [-n] [-r] [--cached] [--] <file>...";
+"git-rm [-f] [-n] [-q] [-r] [--cached] [--] <file>...";
 
 static struct {
 	int nr, alloc;

@@ -104,7 +104,7 @@ static struct lock_file lock_file;
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
-	int show_only = 0, force = 0, index_only = 0, recursive = 0;
+	int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
 	const char **pathspec;
 	char *seen;
 

@@ -130,6 +130,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			index_only = 1;
 		else if (!strcmp(arg, "-f"))
 			force = 1;
+		else if (!strcmp(arg, "-q"))
+			quiet = 1;
 		else if (!strcmp(arg, "-r"))
 			recursive = 1;
 		else

@@ -153,14 +155,24 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	if (pathspec) {
 		const char *match;
+		int seen_any = 0;
 		for (i = 0; (match = pathspec[i]) != NULL ; i++) {
-			if (!seen[i])
-				die("pathspec '%s' did not match any files",
-				    match);
+			if (!seen[i]) {
+				if (! quiet) {
+					die("pathspec '%s' did not match any files",
+					    match);
+				}
+			}
+			else {
+				seen_any = 1;
+			}
 			if (!recursive && seen[i] == MATCHED_RECURSIVELY)
 				die("not removing '%s' recursively without -r",
 				    *match ? match : ".");
 		}
+
+		if (! seen_any)
+			exit(0);
 	}
 
 	/*

@@ -187,7 +199,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 */
 	for (i = 0; i < list.nr; i++) {
 		const char *path = list.name[i];
-		printf("rm '%s'\n", path);
+		if (!quiet) {
+			printf("rm '%s'\n", path);
+		}
 
 		if (remove_file_from_cache(path))
 			die("git-rm: unable to remove %s", path);

index e31cf93..bd72feb 100755
@@ -84,6 +84,30 @@ test_expect_success \
     'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
     'git-ls-files --error-unmatch baz'
 
+test_expect_success 'Remove nonexistent file with -q returns zero exit status' '
+	git rm -q nonexistent
+'
+
+test_expect_success '"rm" command printed' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm test" &&
+	git rm test-file > rm-output &&
+	test `egrep "^rm " rm-output | wc -l` = 1 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm test"
+'
+
+test_expect_success '"rm" command suppressed with -q' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm -q test" &&
+	git rm -q test-file > rm-output &&
+	test `wc -l < rm-output` = 0 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm test"
+'
+
 # Now, failure cases.
 test_expect_success 'Re-add foo and baz' '
 	git add foo baz &&

@@ -154,4 +178,8 @@ test_expect_success 'Recursive with -r -f' '
 	! test -d frotz
 '
 
+test_expect_failure 'Remove nonexistent file returns nonzero exit status' '
+	git rm nonexistent
+'
+
 test_done

1.5.1.1.99.g0ea98

The status is unusable as is, actually, for the particular use case of cg-admin-rewritehist. If you try to use git-rm as an index filter, cg-admin-rewritehist will stop running as soon as you hit a revision that doesn't contain the file you're looking to filter out. (If the file doesn't exist in the first revision in your repo, that means it will do no work at all.)

My justification was that the "-f" option to the normal UNIX "rm" command does the same thing: it suppresses output and causes the command to exit with a zero status code even if no files match. But "git rm -f" was already taken.

If you like I'll be happy to split it into two separate options.

-Steve

Probably "git-grep foo" wouldn't be suitable as the index filter for admin-rewritehist, either, nor "git-fsck", nor many other things.

What does it have to do with anything?

Saying "git rm --quiet foo" from the command line, wishing to supress the output, is very understandable.

Saying "git rm --ignore-unmatch foo bar baz", wishing to remove bar (which exists) even when foo does not exist, is also very understandable.

I think they are pretty much independent options.

---

Splitting this into two different options (and two patches) per Junio's comments.

 Documentation/git-rm.txt |    4 ++++
 builtin-rm.c             |   10 +++++++---
 t/t3600-rm.sh            |   24 ++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

index 6feebc0..b051ccb 100644
@@ -47,6 +47,10 @@ OPTIONS
 	the paths only from the index, leaving working tree
 	files.
 
+\--quiet::
+	git-rm normally outputs one line (in the form of an "rm" command)
+	for each file removed. This option suppresses that output.
+
 
 DISCUSSION
 ----------

index 8a0738f..7eb9a42 100644
@@ -10,7 +10,7 @@
 #include "tree-walk.h"
 
 static const char builtin_rm_usage[] =
-"git-rm [-f] [-n] [-r] [--cached] [--] <file>...";
+"git-rm [-f] [-n] [-r] [--cached] [--quiet] [--] <file>...";
 
 static struct {
 	int nr, alloc;

@@ -104,7 +104,7 @@ static struct lock_file lock_file;
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
-	int show_only = 0, force = 0, index_only = 0, recursive = 0;
+	int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
 	const char **pathspec;
 	char *seen;
 

@@ -132,6 +132,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			force = 1;
 		else if (!strcmp(arg, "-r"))
 			recursive = 1;
+		else if (!strcmp(arg, "--quiet"))
+			quiet = 1;
 		else
 			usage(builtin_rm_usage);
 	}

@@ -187,7 +189,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 */
 	for (i = 0; i < list.nr; i++) {
 		const char *path = list.name[i];
-		printf("rm '%s'\n", path);
+		if (!quiet) {
+			printf("rm '%s'\n", path);
+		}
 
 		if (remove_file_from_cache(path))
 			die("git-rm: unable to remove %s", path);

index e31cf93..da9da92 100755
@@ -84,6 +84,26 @@ test_expect_success \
     'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
     'git-ls-files --error-unmatch baz'
 
+test_expect_success '"rm" command printed' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm test" &&
+	git rm test-file > rm-output &&
+	test `egrep "^rm " rm-output | wc -l` = 1 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm test"
+'
+
+test_expect_success '"rm" command suppressed with --quiet' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm --quiet test" &&
+	git rm --quiet test-file > rm-output &&
+	test `wc -l < rm-output` = 0 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm --quiet test"
+'
+
 # Now, failure cases.
 test_expect_success 'Re-add foo and baz' '
 	git add foo baz &&

@@ -154,4 +174,8 @@ test_expect_success 'Recursive with -r -f' '
 	! test -d frotz
 '
 
+test_expect_failure 'Remove nonexistent file returns nonzero exit status' '
+	git rm nonexistent
+'
+
 test_done

1.5.1.1.99.g0ea98

---

This allows "git rm -r --ignore-notfound" to be used as an index filter with cg-admin-rewritehist. The documentation for that command recommends using git-update-index --remove to filter files out of a tree's history, but that doesn't support recursive deletion like git-rm does, making it less convenient to filter directories from history.

 Documentation/git-rm.txt |    3 +++
 builtin-rm.c             |   21 +++++++++++++++++----
 t/t3600-rm.sh            |    4 ++++
 3 files changed, 24 insertions(+), 4 deletions(-)

index b051ccb..9ffb515 100644
@@ -47,6 +47,9 @@ OPTIONS
 	the paths only from the index, leaving working tree
 	files.
 
+\--ignore-notfound::
+	Exit with a zero status even if no files matched.
+
 \--quiet::
 	git-rm normally outputs one line (in the form of an "rm" command)
 	for each file removed. This option suppresses that output.

index 7eb9a42..71166fb 100644
@@ -10,7 +10,7 @@
 #include "tree-walk.h"
 
 static const char builtin_rm_usage[] =
-"git-rm [-f] [-n] [-r] [--cached] [--quiet] [--] <file>...";
+"git-rm [-f] [-n] [-r] [--cached] [--ignore-notfound] [--quiet] [--] <file>...";
 
 static struct {
 	int nr, alloc;

@@ -105,6 +105,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
 	int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
+	int ignore_notfound = 0;
 	const char **pathspec;
 	char *seen;
 

@@ -134,6 +135,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			recursive = 1;
 		else if (!strcmp(arg, "--quiet"))
 			quiet = 1;
+		else if (!strcmp(arg, "--ignore-notfound"))
+			ignore_notfound = 1;
 		else
 			usage(builtin_rm_usage);
 	}

@@ -155,14 +158,24 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	if (pathspec) {
 		const char *match;
+		int seen_any = 0;
 		for (i = 0; (match = pathspec[i]) != NULL ; i++) {
-			if (!seen[i])
-				die("pathspec '%s' did not match any files",
-				    match);
+			if (!seen[i]) {
+				if (!ignore_notfound) {
+					die("pathspec '%s' did not match any files",
+					    match);
+				}
+			}
+			else {
+				seen_any = 1;
+			}
 			if (!recursive && seen[i] == MATCHED_RECURSIVELY)
 				die("not removing '%s' recursively without -r",
 				    *match ? match : ".");
 		}
+
+		if (! seen_any)
+			exit(0);
 	}
 
 	/*

index da9da92..665b8b0 100755
@@ -84,6 +84,10 @@ test_expect_success \
     'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
     'git-ls-files --error-unmatch baz'
 
+test_expect_success 'Remove nonexistent file with --ignore-notfound' '
+	git rm --ignore-notfound nonexistent
+'
+
 test_expect_success '"rm" command printed' '
 	echo frotz > test-file &&
 	git add test-file &&

1.5.1.1.99.g0ea98

Having said that, in some sense the problem here is arguably with cg-admin-rewritehist bailing out when a filter command exits with a nonzero status; given that git-rm is likely to be a common filter people will want to use, ideally cg-admin-rewritehist should do the right thing with git-rm's normal exit codes. But it appears Cogito is not being actively maintained any more (correct me if I'm wrong; the latest stable release is half a year old at this point, while git has changed substantially since then) so it didn't seem too worthwhile to try to patch things from that side.

I think it makes sense to be able to optionally treat a lack of matches as a non-error condition like "rm -f" does, so I'm comfortable submitting this patch regardless.

On a related note, last time I asked a question that caused someone to point me to cg-admin-rewritehist, Linus said he'd like to see its functionality folded into git proper. I have to agree with that idea; there are some obvious enhancements that program could use, and it'll be easier to do that if it's part of an actively maintained code base.

-Steve

I find it easier to see what is going on with two independent features separated out, especially with tests. Much nicer.

Isn't this quite similar to the '-k' option to git-mv?

-Peff

Heh, I never use 'git-mv' and 'git-rm' myself, so I did not know about that option, but from the Documentation/git-mv.txt, it sounds similar. What does it stand for, I wonder?

As you are so fond of telling everyone else, use git-blame to find out. :)

Looks like '-k' got added in the move from git-rename to git-mv. Josef, can you shed any light on the choice of '-k'?

-Peff

That is similiar to "-k" option to "make", synonym for "--keep-going". I thought this was quite natural; at least I am used to this option for "make".

Josef

Sure, UI consistency is a good thing. Feel free to amend.

-Steve

How about doing what rm(1) does? Something like "rm -f"? It returns 0 even if nothing given in command-line, but any error (like permissions) is reported and the status is not 0.

It is often that it is not the commands output which is annoying, but the _unwanted_ output. No one wants
to see "rm 'file'". No one besides people debugging git-rm (you can't even use it for copy and paste of the filenames:
it has "rm"'s in the text).