threads / patch / 7389

patchRe: [PATCH] Removed the printf("rm 'file'") from git-rm.

Subject: Re: [PATCH] Removed the printf("rm 'file'") from git-rm.

## tl;dr

7 messages between Mar 25, 2007 and Mar 26, 2007. Diffs are folded; open one to read it.

replies: 6people: 6as markdown or json

Junio C Hamano· Mar 25, 2007, 06:22 UTC · lore
Tilman Sauerbeck <tilman@code-monkey.de> writes:
> We used to print that, because you actually had to run the output
> of git-rm to get rid of the files before Git 1.5. Now that git-rm
> really removes the files, it's not needed anymore.

Even though I admit I do not deeply care, as I never use 'git rm' myself, I do not necessarily agree with "because" part.

I suspect people are by now accustomed to see the assuring feedback from the command when used this way:

	$ git rm -r one
        rm 'one/1'
        rm 'one/2'
        rm 'one/3'

and even in non-recursive case, expect the similar output for consistecy's sake.

Anand Kumria· Mar 25, 2007, 16:38 UTC · re: Junio C Hamano · lore
On Sat, 24 Mar 2007 23:22:16 -0700, Junio C Hamano wrote:
Show 16 quoted lines
> Tilman Sauerbeck <tilman@code-monkey.de> writes:
> 
>> We used to print that, because you actually had to run the output of
>> git-rm to get rid of the files before Git 1.5. Now that git-rm really
>> removes the files, it's not needed anymore.
> 
> Even though I admit I do not deeply care, as I never use 'git rm'
> myself, I do not necessarily agree with "because" part.
> 
> I suspect people are by now accustomed to see the assuring feedback from
> the command when used this way:
> 
> 	$ git rm -r one
>         rm 'one/1'
>         rm 'one/2'
>         rm 'one/3'

Heh. I didn't even know there was a recursive option. So I'm definitely not 'accustomed' to any form of output.

If me being a data point helps at all.
Anand
Tilman Sauerbeck· Mar 25, 2007, 21:04 UTC · re: Junio C Hamano · lore
Junio C Hamano [2007-03-24 23:22]:
Show 12 quoted lines
> Tilman Sauerbeck <tilman@code-monkey.de> writes:
> 
> > We used to print that, because you actually had to run the output
> > of git-rm to get rid of the files before Git 1.5. Now that git-rm
> > really removes the files, it's not needed anymore.
> 
> Even though I admit I do not deeply care, as I never use 'git
> rm' myself, I do not necessarily agree with "because" part.
> 
> I suspect people are by now accustomed to see the assuring
> feedback from the command when used this way:
> [snip]
Too bad, I find it rather annoying and irritating.

Regards, Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
Johannes Schindelin· Mar 25, 2007, 21:36 UTC · re: Tilman Sauerbeck · lore
Hi,
On Sun, 25 Mar 2007, Tilman Sauerbeck wrote:
Show 15 quoted lines
> Junio C Hamano [2007-03-24 23:22]:
> > Tilman Sauerbeck <tilman@code-monkey.de> writes:
> > 
> > > We used to print that, because you actually had to run the output
> > > of git-rm to get rid of the files before Git 1.5. Now that git-rm
> > > really removes the files, it's not needed anymore.
> > 
> > Even though I admit I do not deeply care, as I never use 'git
> > rm' myself, I do not necessarily agree with "because" part.
> > 
> > I suspect people are by now accustomed to see the assuring
> > feedback from the command when used this way:
> > [snip]
> 
> Too bad, I find it rather annoying and irritating.

Why not do the common thing, and add a "--quiet" option? You can even add a config variable to enable it by default (for git-rm). It's not like git-rm is performance critical...

> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing on usenet and in e-mail?
Funny!

Ciao, Dscho

Eric Lesh· Mar 26, 2007, 10:28 UTC · re: Johannes Schindelin · lore

[PATCH] git-rm: add --quiet option to suppress "rm 'file'" messages

Signed-off-by: Eric Lesh <eclesh@ucla.edu>
---
On Sun, 2007-03-25 at 23:36 +0200, Johannes Schindelin wrote:
Show 6 quoted lines
> > Too bad, I find it rather annoying and irritating.
> 
> Why not do the common thing, and add a "--quiet" option? You can even add 
> a config variable to enable it by default (for git-rm). It's not like 
> git-rm is performance critical...
> 
Is something like this right?
 builtin-rm.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
Show changes to builtin-rm.c +5 −2
diff --git a/builtin-rm.c b/builtin-rm.c
index 00dbe39..d193fb0 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -114,7 +114,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;
 
@@ -142,6 +142,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, "-q") || !strcmp(arg, "--quiet"))
+			quiet = 1;
 		else
 			usage(builtin_rm_usage);
 	}
@@ -197,7 +199,8 @@ 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);

-- 
1.5.1-rc1.GIT
Junio C Hamano· Mar 26, 2007, 22:56 UTC · re: Eric Lesh · lore

Re: [PATCH] git-rm: add --quiet option to suppress "rm 'file'" messages

Eric Lesh <eclesh@ucla.edu> writes:
Show 39 quoted lines
> Signed-off-by: Eric Lesh <eclesh@ucla.edu>
>
> ---
>
> On Sun, 2007-03-25 at 23:36 +0200, Johannes Schindelin wrote:
>> > Too bad, I find it rather annoying and irritating.
>> 
>> Why not do the common thing, and add a "--quiet" option? You can even add 
>> a config variable to enable it by default (for git-rm). It's not like 
>> git-rm is performance critical...
>
> Is something like this right?
>
>  builtin-rm.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-rm.c b/builtin-rm.c
> index 00dbe39..d193fb0 100644
> --- a/builtin-rm.c
> +++ b/builtin-rm.c
> @@ -114,7 +114,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;
>  
> @@ -197,7 +199,8 @@ 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);
I wonder how this would interact with show_only...
Martin Waitz· Mar 26, 2007, 22:13 UTC · re: Johannes Schindelin · lore
hoi :)
On Sun, Mar 25, 2007 at 11:36:35PM +0200, Johannes Schindelin wrote:
> Why not do the common thing, and add a "--quiet" option? You can even add 
> a config variable to enable it by default (for git-rm). It's not like 
> git-rm is performance critical...

But when we have to add --quiet to all sorts of commands that may be the sign that they really are too chatty.

If I want a short output I don't want to type extra options. So adding a --verbose for those that really depend on more output makes more sense, IMHO. (Even when I don't see any useful information in the git-rm output, to be honest.)

-- 
Martin Waitz

← back to recent threads