[PATCH 00/10] teach --progress to transport-related builtins

[PATCH 00/10] teach --progress to transport-related builtins

[Tay Ray Chuan]

This patch series adds --progress to:

  - git-fetch
  - git-pull
  - git-push

I suspect the most contentious issue in this patch series would be the
logic that determines whether progress reporting is done. This is found
in patch 6 for transport.c::transport_set_verbosity().

As a guide, I used Jeff's message (gmane#121065). The rules used are as
follows (processing aborts when a rule is satisfied):

  1. Report progress, if force_progress is 1 (ie. --progress).
  2. Don't report progress, if verbosity < 0 (ie. -q/--quiet).
  3. Report progress if isatty(2) is 1.

This changes the current implementation such that if both --progress
and --quiet are specified, progress is reported. I don't think this is
a very significant change, but I think it makes sense, since I expect
--progress to be mostly used by script writers or IDE integrators (to
force progress reporting even if stderr is not a terminal).

Contents:

[PATCH 01/10] Documentation/git-pull.txt: mention --quiet and --verbose for fetching
[PATCH 02/10] Documentation/git-push.txt: put --quiet before --verbose
[PATCH 03/10] fetch: refactor verbosity option handling into transport.[ch]
[PATCH 04/10] push: support multiple levels of verbosity
[PATCH 05/10] clone: support multiple levels of verbosity
[PATCH 06/10] transport->progress: use flag authoritatively
[PATCH 07/10] push: learn --progress
[PATCH 08/10] fetch: learn --progress
[PATCH 09/10] pull: learn --progress
[PATCH 10/10] transport: update flags to be in running order

 Documentation/fetch-options.txt |   11 ++++++++---
 Documentation/git-push.txt      |   15 +++++++++++----
 builtin-clone.c                 |   19 ++++++-------------
 builtin-fetch.c                 |    7 +++----
 builtin-push.c                  |   11 ++++++++---
 git-pull.sh                     |    6 ++++--
 transport-helper.c              |    4 +---
 transport.c                     |   31 ++++++++++++++++++++++++++-----
 transport.h                     |   15 ++++++++++-----
 9 files changed, 77 insertions(+), 42 deletions(-)

--
Cheers,
Ray Chuan

[PATCH 01/10] Documentation/git-pull.txt: mention --quiet and --verbose for fetching

[Tay Ray Chuan]

This reverts 90e4311 (git-pull: do not mention --quiet and --verbose
twice, Mon Sep 7 2009).

Then, the subtitles "Options related to merging" and "Options related to
fetching" weren't present, and the options for git-merge and git-fetch
options were in a monolithic block.

After 3f7a9b5 (Documentation/git-pull.txt: Add subtitles above included
option files, Thu Oct 22 2009), it is ok to repeat options in the
same document, since they are distinguished between those for git-merge
and git-fetch.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 Documentation/fetch-options.txt |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index fe716b2..83606f4 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -74,7 +74,6 @@ endif::git-pull[]
 	the command to specify non-default path for the command
 	run on the other end.
 
-ifndef::git-pull[]
 -q::
 --quiet::
 	Pass --quiet to git-fetch-pack and silence any other internally
@@ -83,4 +82,3 @@ ifndef::git-pull[]
 -v::
 --verbose::
 	Be verbose.
-endif::git-pull[]
-- 
1.7.0.27.g5d71b

Re: [PATCH 01/10] Documentation/git-pull.txt: mention --quiet and --verbose for fetching

[Junio C Hamano]

Tay Ray Chuan <rctay89@gmail.com> writes:

> This reverts 90e4311 (git-pull: do not mention --quiet and --verbose
> twice, Mon Sep 7 2009).
>
> Then, the subtitles "Options related to merging" and "Options related to
> fetching" weren't present, and the options for git-merge and git-fetch
> options were in a monolithic block.
>
> After 3f7a9b5 (Documentation/git-pull.txt: Add subtitles above included
> option files, Thu Oct 22 2009), it is ok to repeat options in the
> same document, since they are distinguished between those for git-merge
> and git-fetch.
>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>

As the result of the change history, currently we mention -q/-v as if it
only applies to the invocation of merge, giving an incorrect impression
that they won't affect the invocation of fetch.

While I agree that is a valid issue to address, I am not sure if repeating
these in two sections is a good solution.  A reader will start scanning
the OPTIONS section, finds -q (or -v) and notices it is listed in the
"related to merging" subsection, and would stop there.  This change would
not help such a reader.

It may make more sense to list the options that affects both fetch and
merge at the beginning of the OPTIONS section before the merge/fetch
subsections.  IOW, instead of removing ifndef::git-pull[] like your patch
did, it would add ifndef::git-pull[] in the list of options on the merge
side, and add pull specific description (e.g. "this is passed to both
underlying fetch to squelch progress output of transfer, and underlying
merge to squelch the output during the merging") in git-pull.txt.

>  Documentation/fetch-options.txt |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index fe716b2..83606f4 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -74,7 +74,6 @@ endif::git-pull[]
>  	the command to specify non-default path for the command
>  	run on the other end.
>  
> -ifndef::git-pull[]
>  -q::
>  --quiet::
>  	Pass --quiet to git-fetch-pack and silence any other internally
> @@ -83,4 +82,3 @@ ifndef::git-pull[]
>  -v::
>  --verbose::
>  	Be verbose.
> -endif::git-pull[]
> -- 
> 1.7.0.27.g5d71b

Re: [PATCH 01/10] Documentation/git-pull.txt: mention --quiet and --verbose for fetching

[Tay Ray Chuan]

Hi,

On Fri, Feb 19, 2010 at 5:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
> It may make more sense to list the options that affects both fetch and
> merge at the beginning of the OPTIONS section before the merge/fetch
> subsections.  IOW, instead of removing ifndef::git-pull[] like your patch
> did, it would add ifndef::git-pull[] in the list of options on the merge
> side, and add pull specific description (e.g. "this is passed to both
> underlying fetch to squelch progress output of transfer, and underlying
> merge to squelch the output during the merging") in git-pull.txt.

I considered adding a "common options" section too, but wasn't sure.
I'll take your suggestion on this.

-- 
Cheers,
Ray Chuan

[PATCH 02/10] Documentation/git-push.txt: put --quiet before --verbose

[Tay Ray Chuan]

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 Documentation/git-push.txt |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index bd79119..22cff99 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -145,15 +145,15 @@ useful if you write an alias or script around 'git push'.
 	transfer spends extra cycles to minimize the number of
 	objects to be sent and meant to be used on slower connection.
 
--v::
---verbose::
-	Run verbosely.
-
 -q::
 --quiet::
 	Suppress all output, including the listing of updated refs,
 	unless an error occurs.
 
+-v::
+--verbose::
+	Run verbosely.
+
 include::urls-remotes.txt[]
 
 OUTPUT
-- 
1.7.0.27.g5d71b

[PATCH 03/10] fetch: refactor verbosity option handling into transport.[ch]

[Tay Ray Chuan]

transport_set_verbosity() is now provided to transport users.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin-fetch.c |    5 +----
 transport.c     |    8 ++++++++
 transport.h     |    1 +
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 8654fa7..d23ea2a 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -823,10 +823,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 		die("Where do you want to fetch from today?");
 
 	transport = transport_get(remote, NULL);
-	if (verbosity >= 2)
-		transport->verbose = verbosity <= 3 ? verbosity : 3;
-	if (verbosity < 0)
-		transport->verbose = -1;
+	transport_set_verbosity(transport, verbosity);
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
 	if (keep)
diff --git a/transport.c b/transport.c
index 3846aac..1632c4d 100644
--- a/transport.c
+++ b/transport.c
@@ -1013,6 +1013,14 @@ int transport_set_option(struct transport *transport,
 	return 1;
 }
 
+void transport_set_verbosity(struct transport *transport, int verbosity)
+{
+	if (verbosity >= 2)
+		transport->verbose = verbosity <= 3 ? verbosity : 3;
+	if (verbosity < 0)
+		transport->verbose = -1;
+}
+
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
 		   int *nonfastforward)
diff --git a/transport.h b/transport.h
index 7cea5cc..7d1a0b6 100644
--- a/transport.h
+++ b/transport.h
@@ -122,6 +122,7 @@ struct transport *transport_get(struct remote *, const char *);
  **/
 int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
+void transport_set_verbosity(struct transport *transport, int verbosity);
 
 int transport_push(struct transport *connection,
 		   int refspec_nr, const char **refspec, int flags,
-- 
1.7.0.27.g5d71b

[PATCH 04/10] push: support multiple levels of verbosity

[Tay Ray Chuan]

Remove the flags TRANSPORT_PUSH_QUIET and TRANSPORT_PUSH_VERBOSE; use
transport->verbose instead to determine verbosity for pushing.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin-push.c     |    9 ++++++---
 transport-helper.c |    1 -
 transport.c        |    8 ++++----
 transport.h        |    2 --
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 5633f0a..0082dad 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -17,6 +17,7 @@ static const char * const push_usage[] = {
 static int thin;
 static int deleterefs;
 static const char *receivepack;
+static int verbosity;
 
 static const char **refspec;
 static int refspec_nr;
@@ -105,13 +106,16 @@ static int push_with_options(struct transport *transport, int flags)
 {
 	int err;
 	int nonfastforward;
+
+	transport_set_verbosity(transport, verbosity);
+
 	if (receivepack)
 		transport_set_option(transport,
 				     TRANS_OPT_RECEIVEPACK, receivepack);
 	if (thin)
 		transport_set_option(transport, TRANS_OPT_THIN, "yes");
 
-	if (flags & TRANSPORT_PUSH_VERBOSE)
+	if (verbosity > 0)
 		fprintf(stderr, "Pushing to %s\n", transport->url);
 	err = transport_push(transport, refspec_nr, refspec, flags,
 			     &nonfastforward);
@@ -204,8 +208,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int rc;
 	const char *repo = NULL;	/* default repository */
 	struct option options[] = {
-		OPT_BIT('q', "quiet", &flags, "be quiet", TRANSPORT_PUSH_QUIET),
-		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
+		OPT__VERBOSITY(&verbosity),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
 		OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
diff --git a/transport-helper.c b/transport-helper.c
index 1077428..3d33697 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -576,7 +576,6 @@ static int push_refs(struct transport *transport,
 	if (buf.len == 0)
 		return 0;
 
-	transport->verbose = flags & TRANSPORT_PUSH_VERBOSE ? 1 : 0;
 	standard_options(transport);
 
 	if (flags & TRANSPORT_PUSH_DRY_RUN) {
diff --git a/transport.c b/transport.c
index 1632c4d..0655f65 100644
--- a/transport.c
+++ b/transport.c
@@ -788,8 +788,8 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	args.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);
 	args.force_update = !!(flags & TRANSPORT_PUSH_FORCE);
 	args.use_thin_pack = data->options.thin;
-	args.verbose = !!(flags & TRANSPORT_PUSH_VERBOSE);
-	args.quiet = !!(flags & TRANSPORT_PUSH_QUIET);
+	args.verbose = (transport->verbose > 0);
+	args.quiet = (transport->verbose < 0);
 	args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
 
 	ret = send_pack(&args, data->fd, data->conn, remote_refs,
@@ -1039,8 +1039,8 @@ int transport_push(struct transport *transport,
 			transport->get_refs_list(transport, 1);
 		struct ref *local_refs = get_local_heads();
 		int match_flags = MATCH_REFS_NONE;
-		int verbose = flags & TRANSPORT_PUSH_VERBOSE;
-		int quiet = flags & TRANSPORT_PUSH_QUIET;
+		int verbose = (transport->verbose > 0);
+		int quiet = (transport->verbose < 0);
 		int porcelain = flags & TRANSPORT_PUSH_PORCELAIN;
 		int pretend = flags & TRANSPORT_PUSH_DRY_RUN;
 		int ret, err;
diff --git a/transport.h b/transport.h
index 7d1a0b6..c0743b1 100644
--- a/transport.h
+++ b/transport.h
@@ -88,9 +88,7 @@ struct transport {
 #define TRANSPORT_PUSH_FORCE 2
 #define TRANSPORT_PUSH_DRY_RUN 4
 #define TRANSPORT_PUSH_MIRROR 8
-#define TRANSPORT_PUSH_VERBOSE 16
 #define TRANSPORT_PUSH_PORCELAIN 32
-#define TRANSPORT_PUSH_QUIET 64
 #define TRANSPORT_PUSH_SET_UPSTREAM 128
 
 /* Returns a transport suitable for the url */
-- 
1.7.0.27.g5d71b

[PATCH 05/10] clone: support multiple levels of verbosity

[Tay Ray Chuan]

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin-clone.c |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 58bacbd..959fe4b 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -37,18 +37,17 @@ static const char * const builtin_clone_usage[] = {
 	NULL
 };
 
-static int option_quiet, option_no_checkout, option_bare, option_mirror;
+static int option_no_checkout, option_bare, option_mirror;
 static int option_local, option_no_hardlinks, option_shared, option_recursive;
 static char *option_template, *option_reference, *option_depth;
 static char *option_origin = NULL;
 static char *option_branch = NULL;
 static char *option_upload_pack = "git-upload-pack";
-static int option_verbose;
+static int option_verbosity;
 static int option_progress;
 
 static struct option builtin_clone_options[] = {
-	OPT__QUIET(&option_quiet),
-	OPT__VERBOSE(&option_verbose),
+	OPT__VERBOSITY(&option_verbosity),
 	OPT_BOOLEAN(0, "progress", &option_progress,
 			"force progress reporting"),
 	OPT_BOOLEAN('n', "no-checkout", &option_no_checkout,
@@ -462,7 +461,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		die("could not create leading directories of '%s'", git_dir);
 	set_git_dir(make_absolute_path(git_dir));
 
-	init_db(option_template, option_quiet ? INIT_DB_QUIET : 0);
+	init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
 
 	/*
 	 * At this point, the config exists, so we do not need the
@@ -526,10 +525,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			transport_set_option(transport, TRANS_OPT_DEPTH,
 					     option_depth);
 
-		if (option_quiet)
-			transport->verbose = -1;
-		else if (option_verbose)
-			transport->verbose = 1;
+		transport_set_verbosity(transport, option_verbosity);
 
 		if (option_progress)
 			transport->progress = 1;
@@ -641,7 +637,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		opts.update = 1;
 		opts.merge = 1;
 		opts.fn = oneway_merge;
-		opts.verbose_update = !option_quiet;
+		opts.verbose_update = (option_verbosity > 0);
 		opts.src_index = &the_index;
 		opts.dst_index = &the_index;
 
-- 
1.7.0.27.g5d71b

[PATCH 06/10] transport->progress: use flag authoritatively

[Tay Ray Chuan]

Set transport->progress in transport.c::transport_set_verbosity() after
checking for the appropriate conditions (eg. --progress, isatty(2)),
and thereafter use it without having to check again.

The rules used are as follows (processing aborts when a rule is
satisfied):

  1. Report progress, if force_progress is 1 (ie. --progress).
  2. Don't report progress, if verbosity < 0 (ie. -q/--quiet).
  3. Report progress if isatty(2) is 1.

This changes progress reporting behaviour such that if both --progress
and --quiet are specified, progress is reported.

In two areas, the logic to determine whether to *not* show progress is
changed to simply use the negation of transport->progress. This changes
behaviour in some ways (see previous paragraph for details).

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin-clone.c    |    5 +----
 builtin-fetch.c    |    2 +-
 builtin-push.c     |    2 +-
 transport-helper.c |    3 +--
 transport.c        |   17 +++++++++++++++--
 transport.h        |   10 ++++++++--
 6 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 959fe4b..05f8fb4 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -525,10 +525,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			transport_set_option(transport, TRANS_OPT_DEPTH,
 					     option_depth);
 
-		transport_set_verbosity(transport, option_verbosity);
-
-		if (option_progress)
-			transport->progress = 1;
+		transport_set_verbosity(transport, option_verbosity, option_progress);
 
 		if (option_upload_pack)
 			transport_set_option(transport, TRANS_OPT_UPLOADPACK,
diff --git a/builtin-fetch.c b/builtin-fetch.c
index d23ea2a..6b96b41 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -823,7 +823,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 		die("Where do you want to fetch from today?");
 
 	transport = transport_get(remote, NULL);
-	transport_set_verbosity(transport, verbosity);
+	transport_set_verbosity(transport, verbosity, 0);
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
 	if (keep)
diff --git a/builtin-push.c b/builtin-push.c
index 0082dad..dce3152 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -107,7 +107,7 @@ static int push_with_options(struct transport *transport, int flags)
 	int err;
 	int nonfastforward;
 
-	transport_set_verbosity(transport, verbosity);
+	transport_set_verbosity(transport, verbosity, 0);
 
 	if (receivepack)
 		transport_set_option(transport,
diff --git a/transport-helper.c b/transport-helper.c
index 3d33697..3e69ebd 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -279,9 +279,8 @@ static void standard_options(struct transport *t)
 	char buf[16];
 	int n;
 	int v = t->verbose;
-	int no_progress = v < 0 || (!t->progress && !isatty(2));
 
-	set_helper_option(t, "progress", !no_progress ? "true" : "false");
+	set_helper_option(t, "progress", t->progress ? "true" : "false");
 
 	n = snprintf(buf, sizeof(buf), "%d", v + 1);
 	if (n >= sizeof(buf))
diff --git a/transport.c b/transport.c
index 0655f65..e2cfabd 100644
--- a/transport.c
+++ b/transport.c
@@ -526,7 +526,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 	args.include_tag = data->options.followtags;
 	args.verbose = (transport->verbose > 0);
 	args.quiet = (transport->verbose < 0);
-	args.no_progress = args.quiet || (!transport->progress && !isatty(2));
+	args.no_progress = !transport->progress;
 	args.depth = data->options.depth;
 
 	for (i = 0; i < nr_heads; i++)
@@ -915,6 +915,8 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	const char *helper;
 	struct transport *ret = xcalloc(1, sizeof(*ret));
 
+	ret->progress = isatty(2);
+
 	if (!remote)
 		die("No remote provided to transport_get()");
 
@@ -1013,12 +1015,23 @@ int transport_set_option(struct transport *transport,
 	return 1;
 }
 
-void transport_set_verbosity(struct transport *transport, int verbosity)
+void transport_set_verbosity(struct transport *transport, int verbosity,
+	int force_progress)
 {
 	if (verbosity >= 2)
 		transport->verbose = verbosity <= 3 ? verbosity : 3;
 	if (verbosity < 0)
 		transport->verbose = -1;
+
+	/**
+	 * Rules used to determine whether to report progress (processing aborts
+	 * when a rule is satisfied):
+	 *
+	 *   1. Report progress, if force_progress is 1 (ie. --progress).
+	 *   2. Don't report progress, if verbosity < 0 (ie. -q/--quiet ).
+	 *   3. Report progress if isatty(2) is 1.
+	 **/
+	transport->progress = force_progress || (verbosity >= 0 && isatty(2));
 }
 
 int transport_push(struct transport *transport,
diff --git a/transport.h b/transport.h
index c0743b1..de2745a 100644
--- a/transport.h
+++ b/transport.h
@@ -74,7 +74,12 @@ struct transport {
 	int (*disconnect)(struct transport *connection);
 	char *pack_lockfile;
 	signed verbose : 3;
-	/* Force progress even if stderr is not a tty */
+	/**
+	 * Transports should not set this directly, and should use this
+	 * value without having to check isatty(2), -q/--quiet
+	 * (transport->verbose < 0), etc. - checking has already been done
+	 * in transport_set_verbosity().
+	 **/
 	unsigned progress : 1;
 	/*
 	 * If transport is at least potentially smart, this points to
@@ -120,7 +125,8 @@ struct transport *transport_get(struct remote *, const char *);
  **/
 int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
-void transport_set_verbosity(struct transport *transport, int verbosity);
+void transport_set_verbosity(struct transport *transport, int verbosity,
+	int force_progress);
 
 int transport_push(struct transport *connection,
 		   int refspec_nr, const char **refspec, int flags,
-- 
1.7.0.27.g5d71b

[PATCH 07/10] push: learn --progress

[Tay Ray Chuan]

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 Documentation/git-push.txt |    9 ++++++++-
 builtin-push.c             |    4 +++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 22cff99..8d95724 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -148,12 +148,19 @@ useful if you write an alias or script around 'git push'.
 -q::
 --quiet::
 	Suppress all output, including the listing of updated refs,
-	unless an error occurs.
+	unless an error occurs. Progress is not reported to the standard
+	error stream.
 
 -v::
 --verbose::
 	Run verbosely.
 
+--progress::
+	Progress status is reported on the standard error stream
+	by default when it is attached to a terminal, unless -q
+	is specified. This flag forces progress status even if the
+	standard error stream is not directed to a terminal.
+
 include::urls-remotes.txt[]
 
 OUTPUT
diff --git a/builtin-push.c b/builtin-push.c
index dce3152..ba9fe49 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -18,6 +18,7 @@ static int thin;
 static int deleterefs;
 static const char *receivepack;
 static int verbosity;
+static int progress;
 
 static const char **refspec;
 static int refspec_nr;
@@ -107,7 +108,7 @@ static int push_with_options(struct transport *transport, int flags)
 	int err;
 	int nonfastforward;
 
-	transport_set_verbosity(transport, verbosity, 0);
+	transport_set_verbosity(transport, verbosity, progress);
 
 	if (receivepack)
 		transport_set_option(transport,
@@ -223,6 +224,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
 		OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
 			TRANSPORT_PUSH_SET_UPSTREAM),
+		OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
 		OPT_END()
 	};
 
-- 
1.7.0.27.g5d71b

[PATCH 08/10] fetch: learn --progress

[Tay Ray Chuan]

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 Documentation/fetch-options.txt |    9 ++++++++-
 builtin-fetch.c                 |    4 +++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 83606f4..db1ea25 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -77,8 +77,15 @@ endif::git-pull[]
 -q::
 --quiet::
 	Pass --quiet to git-fetch-pack and silence any other internally
-	used git commands.
+	used git commands. Progress is not reported to the standard error
+	stream.
 
 -v::
 --verbose::
 	Be verbose.
+
+--progress::
+	Progress status is reported on the standard error stream
+	by default when it is attached to a terminal, unless -q
+	is specified. This flag forces progress status even if the
+	standard error stream is not directed to a terminal.
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 6b96b41..7f9f669 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -27,6 +27,7 @@ enum {
 };
 
 static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
+static int progress;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
@@ -56,6 +57,7 @@ static struct option builtin_fetch_options[] = {
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
 	OPT_BOOLEAN('u', "update-head-ok", &update_head_ok,
 		    "allow updating of HEAD ref"),
+	OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
 	OPT_END()
@@ -823,7 +825,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 		die("Where do you want to fetch from today?");
 
 	transport = transport_get(remote, NULL);
-	transport_set_verbosity(transport, verbosity, 0);
+	transport_set_verbosity(transport, verbosity, progress);
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
 	if (keep)
-- 
1.7.0.27.g5d71b

[PATCH 09/10] pull: learn --progress

[Tay Ray Chuan]

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 git-pull.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 38331a8..d45b50c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -38,7 +38,7 @@ test -z "$(git ls-files -u)" || die_conflict
 test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
-log_arg= verbosity=
+log_arg= verbosity= progress=
 merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
@@ -50,6 +50,8 @@ do
 		verbosity="$verbosity -q" ;;
 	-v|--verbose)
 		verbosity="$verbosity -v" ;;
+	--progress)
+		progress=--progress ;;
 	-n|--no-stat|--no-summary)
 		diffstat=--no-stat ;;
 	--stat|--summary)
@@ -214,7 +216,7 @@ test true = "$rebase" && {
 	done
 }
 orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity --update-head-ok "$@" || exit 1
+git fetch $verbosity $progress --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse -q --verify HEAD)
 if test -n "$orig_head" && test "$curr_head" != "$orig_head"
-- 
1.7.0.27.g5d71b

[PATCH 10/10] transport: update flags to be in running order

[Tay Ray Chuan]

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 transport.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/transport.h b/transport.h
index de2745a..d9bbabb 100644
--- a/transport.h
+++ b/transport.h
@@ -93,8 +93,8 @@ struct transport {
 #define TRANSPORT_PUSH_FORCE 2
 #define TRANSPORT_PUSH_DRY_RUN 4
 #define TRANSPORT_PUSH_MIRROR 8
-#define TRANSPORT_PUSH_PORCELAIN 32
-#define TRANSPORT_PUSH_SET_UPSTREAM 128
+#define TRANSPORT_PUSH_PORCELAIN 16
+#define TRANSPORT_PUSH_SET_UPSTREAM 32
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
-- 
1.7.0.27.g5d71b

Re: [PATCH 00/10] teach --progress to transport-related builtins

[Junio C Hamano]

Tay Ray Chuan <rctay89@gmail.com> writes:

> This patch series adds --progress to:
>
>   - git-fetch
>   - git-pull
>   - git-push
>
> I suspect the most contentious issue in this patch series would be the
> logic that determines whether progress reporting is done. This is found
> in patch 6 for transport.c::transport_set_verbosity().
>
> As a guide, I used Jeff's message (gmane#121065). The rules used are as
> follows (processing aborts when a rule is satisfied):
>
>   1. Report progress, if force_progress is 1 (ie. --progress).
>   2. Don't report progress, if verbosity < 0 (ie. -q/--quiet).
>   3. Report progress if isatty(2) is 1.
>
> This changes the current implementation such that if both --progress
> and --quiet are specified, progress is reported. I don't think this is
> a very significant change, but I think it makes sense, since I expect
> --progress to be mostly used by script writers or IDE integrators (to
> force progress reporting even if stderr is not a terminal).

I gave a cursory look and they all looked sensible (except for 1/10
on which I already commented separately).  Thanks.

Re: [PATCH 00/10] teach --progress to transport-related builtins

[Tay Ray Chuan]

Hi,

On Fri, Feb 19, 2010 at 9:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I gave a cursory look and they all looked sensible (except for 1/10
> on which I already commented separately).  Thanks.

ok. I'll sit on this series for a while for more comments to come in
before I send out the next iteration.

-- 
Cheers,
Ray Chuan

Re: [PATCH 00/10] teach --progress to transport-related builtins

[Jeff King]

On Fri, Feb 19, 2010 at 02:31:31PM +0800, Tay Ray Chuan wrote:

> Hi,
> 
> On Fri, Feb 19, 2010 at 9:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > I gave a cursory look and they all looked sensible (except for 1/10
> > on which I already commented separately).  Thanks.
> 
> ok. I'll sit on this series for a while for more comments to come in
> before I send out the next iteration.

I read it over and it looked OK, except for Junio's comments.

-Peff