threads / patch / 21872

patchAllow --quiet option to git remote, particularly for `git remote update`

Subject: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`

## tl;dr

6 messages between Dec 6, 2009 and Dec 7, 2009. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Alex Vandiver· Dec 6, 2009, 00:00 UTC · lore
Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 builtin-remote.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)
Show changes to builtin-remote.c +12 −7
diff --git a/builtin-remote.c b/builtin-remote.c
index a501939..a34006f 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -8,14 +8,14 @@
 #include "refs.h"
 
 static const char * const builtin_remote_usage[] = {
-	"git remote [-v | --verbose]",
+	"git remote [-v | --verbose] [-q | --quiet]",
 	"git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>",
 	"git remote rename <old> <new>",
 	"git remote rm <name>",
 	"git remote set-head <name> (-a | -d | <branch>)",
 	"git remote [-v | --verbose] show [-n] <name>",
 	"git remote prune [-n | --dry-run] <name>",
-	"git remote [-v | --verbose] update [-p | --prune] [group | remote]",
+	"git remote [-v | --verbose] [-q | --quiet] update [-p | --prune] [group]",
 	NULL
 };
 
@@ -58,7 +58,7 @@ static const char * const builtin_remote_update_usage[] = {
 #define GET_HEAD_NAMES (1<<1)
 #define GET_PUSH_REF_STATES (1<<2)
 
-static int verbose;
+static int verbosity;
 
 static int show_all(void);
 static int prune_remote(const char *remote, int dry_run);
@@ -84,9 +84,12 @@ static int opt_parse_track(const struct option *opt, const char *arg, int not)
 static int fetch_remote(const char *name)
 {
 	const char *argv[] = { "fetch", name, NULL, NULL };
-	if (verbose) {
+	if (verbosity > 0) {
 		argv[1] = "-v";
 		argv[2] = name;
+	} else if (verbosity < 0) {
+		argv[1] = "-q";
+		argv[2] = name;
 	}
 	printf("Updating %s\n", name);
 	if (run_command_v_opt(argv, RUN_GIT_CMD))
@@ -1236,8 +1239,10 @@ static int update(int argc, const char **argv)
 
 	if (prune)
 		fetch_argv[fetch_argc++] = "--prune";
-	if (verbose)
+	if (verbosity > 0)
 		fetch_argv[fetch_argc++] = "-v";
+	if (verbosity < 0)
+		fetch_argv[fetch_argc++] = "-q";
 	if (argc < 2) {
 		fetch_argv[fetch_argc++] = "default";
 	} else {
@@ -1301,7 +1306,7 @@ static int show_all(void)
 		sort_string_list(&list);
 		for (i = 0; i < list.nr; i++) {
 			struct string_list_item *item = list.items + i;
-			if (verbose)
+			if (verbosity > 0)
 				printf("%s\t%s\n", item->string,
 					item->util ? (const char *)item->util : "");
 			else {
@@ -1318,7 +1323,7 @@ static int show_all(void)
 int cmd_remote(int argc, const char **argv, const char *prefix)
 {
 	struct option options[] = {
-		OPT_BOOLEAN('v', "verbose", &verbose, "be verbose; must be placed before a subcommand"),
+		OPT__VERBOSITY(&verbosity),
 		OPT_END()
 	};
 	int result;

-- 
1.6.6.rc0.360.gc408
Junio C Hamano· Dec 6, 2009, 02:04 UTC · re: Alex Vandiver · lore

Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`

Alex Vandiver <alex@chmrr.net> writes:
> Signed-off-by: Alex Vandiver <alex@chmrr.net>

Sounds good as it makes the behaviour consistent with the underlying "git fetch".

> ...
>  	"git remote prune [-n | --dry-run] <name>",
> -	"git remote [-v | --verbose] update [-p | --prune] [group | remote]",
> +	"git remote [-v | --verbose] [-q | --quiet] update [-p | --prune] [group]",

We say "<name>" everywhere else except for "update" we call the parameter "group" or "remote" using different typography. It is not the fault of your patch, but has been this way since 1918278 (Allow git-remote to update named groups of remotes, 2007-02-20).

Three issues to consider:
 - shouldn't we use the same typography, i.e. <group>?
 - should we say <name> _if_ we are not going to say <group>|<remote>?
 - should we keep it as <group>|<remote> to make it clear that only this
   subcommand allows the group nickname?

The first two are easy and I expect the answers to be both yes. The third one needs some studying and further thought.

 - is "remote update" the only one that takes group nickname?
 - should "remote update" the only one? e.g. does "remote prune" also
   take group? if not, shouldn't it?
Alex Vandiver· Dec 7, 2009, 07:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`

At Sat Dec 05 21:04:14 -0500 2009, Junio C Hamano wrote:
Show 5 quoted lines
> Alex Vandiver <alex@chmrr.net> writes:
> > ...
> >      "git remote prune [-n | --dry-run] <name>",
> > -    "git remote [-v | --verbose] update [-p | --prune] [group | remote]",
> > +    "git remote [-v | --verbose] [-q | --quiet] update [-p | --prune] [group]",

Hm, I hadn't noticed that I'd changed "[group | remote]" to "[group]". I think this is due to a mismerge on my part -- apologies. As another data point, `git fetch` describes this as "[<repository> | <group>]".

Show 13 quoted lines
> Three issues to consider:
> 
>  - shouldn't we use the same typography, i.e. <group>?
> 
>  - should we say <name> _if_ we are not going to say <group>|<remote>?
> 
>  - should we keep it as <group>|<remote> to make it clear that only this
>    subcommand allows the group nickname?
> 
> The first two are easy and I expect the answers to be both yes.  The third
> one needs some studying and further thought.
> 
>  - is "remote update" the only one that takes group nickname?

My quick skim of the code says "yes" -- the other commands only deal with single remotes at a time, and prune is oblivious to groups.

>  - should "remote update" the only one? e.g. does "remote prune" also
>    take group? if not, shouldn't it?
Properly, it "ought" to, though I don't see much utility over `git
remote fetch --prune groupname`.  Probably at the same time, the
parallel pruning codepaths in builtin-fetch.c:prune_refs() and
builtin-remote.c:prune_remote() should be unified.
 - Alex
-- 
Networking -- only one letter away from not working
Jeff King· Dec 6, 2009, 14:50 UTC · re: Alex Vandiver · lore

Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`

On Sat, Dec 05, 2009 at 07:00:23PM -0500, Alex Vandiver wrote:
Show 13 quoted lines
> @@ -84,9 +84,12 @@ static int opt_parse_track(const struct option *opt, const char *arg, int not)
>  static int fetch_remote(const char *name)
>  {
>  	const char *argv[] = { "fetch", name, NULL, NULL };
> -	if (verbose) {
> +	if (verbosity > 0) {
>  		argv[1] = "-v";
>  		argv[2] = name;
> +	} else if (verbosity < 0) {
> +		argv[1] = "-q";
> +		argv[2] = name;
>  	}
>  	printf("Updating %s\n", name);
Should --quiet also affect this "Updating %s" line?

Actually, I have often wished for a way to shut up this line but keep fetch at its normal verbosity. Fetch very sanely says nothing if there is nothing to update, but you still get this "Updating" junk line, even if nothing is transferred. But that would probably need an extra "--quiet-remote" option to handle separately from what we pass to fetch.

-Peff
Alex Vandiver· Dec 7, 2009, 06:15 UTC · re: Jeff King · lore

Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`

At Sun Dec 06 09:50:00 -0500 2009, Jeff King wrote:
> Should --quiet also affect this "Updating %s" line?

Looking at it more closely, I think this patch should -- but for reasons unrelated o your argument below. The "Updating %s" line there is in fetch_remote, which is _only_ called during `git remote add -f`. It stands in for the "Fetching %s" line which `git fetch` (and `git remote update`) outputs, which (after this patch), _is_ controlled by --quiet.

Thus I feel like "Updating %s" should change to "Fetching %s" for both consistency and explicitness, and should also be controlled by --quiet. The --quiet-remote option is an entirely different bikeshed, which I don't have a strong opinion on, offhand.

 - Alex
-- 
Networking -- only one letter away from not working
Jeff King· Dec 7, 2009, 06:40 UTC · re: Alex Vandiver · lore

Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`

On Mon, Dec 07, 2009 at 01:15:05AM -0500, Alex Vandiver wrote:
Show 9 quoted lines
> At Sun Dec 06 09:50:00 -0500 2009, Jeff King wrote:
> > Should --quiet also affect this "Updating %s" line?
> 
> Looking at it more closely, I think this patch should -- but for
> reasons unrelated o your argument below.  The "Updating %s" line there
> is in fetch_remote, which is _only_ called during `git remote add -f`.
> It stands in for the "Fetching %s" line which `git fetch` (and `git
> remote update`) outputs, which (after this patch), _is_ controlled by
> --quiet.

Ah, sorry, this has actually changed since the last time I looked at it closely, as a result of 9c4a036 (Teach the --all option to 'git fetch', 2009-11-09). So nevermind my complaint...it has actually been addressed separately (I didn't notice because I have been suppressing the output by redirecting stdout for some time).

> Thus I feel like "Updating %s" should change to "Fetching %s" for both
> consistency and explicitness, and should also be controlled by
> --quiet.  The --quiet-remote option is an entirely different bikeshed,
> which I don't have a strong opinion on, offhand.

Yes, I agree (that it should be quieted, and that it should say "Fetching"). My --quiet-remote argument is now pointless as of 9c4a036b.

-Peff

← back to recent threads