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

[Alex Vandiver]

Signed-off-by: Alex Vandiver <***@chmrr.net>
---
builtin-remote.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)

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;

[Junio C Hamano]

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

Post by Alex Vandiver
...
"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]

Post by Junio C Hamano

Post by Alex Vandiver
...
"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>]".

Post by Junio C Hamano
- 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.

Post by Junio C Hamano
- 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

[Jeff King]

Post by Alex Vandiver
@@ -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]

Post by Jeff King
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

[Jeff King]

Post by Alex Vandiver

Post by Jeff King
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).

Post by Alex Vandiver
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