Re: [PATCH] progress: don't print if !isatty(2).

From: Jeff King <peff@peff.net>
Date: May 24, 2012, 05:45 UTC
Message-ID: <20120524054506.GA3440@sigill.intra.peff.net>
In-Reply-To: <1337836732-26778-1-git-send-email-apenwarr@gmail.com>

On Thu, May 24, 2012 at 01:18:52AM -0400, Avery Pennarun wrote:

> If stderr isn't a tty, we shouldn't be printing incremental progress
> messages.  In particular, this affected 'git checkout -f . >&logfile' unless
> you provided -q.  And git-new-workdir has no way to provide -q.

Makes sense to fix checkout, but...

Show 12 quoted lines

> diff --git a/progress.c b/progress.c
> index 3971f49..4d9f416 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -211,9 +211,11 @@ int display_progress(struct progress *progress, unsigned n)
>  struct progress *start_progress_delay(const char *title, unsigned total,
>  				       unsigned percent_treshold, unsigned delay)
>  {
> -	struct progress *progress = malloc(sizeof(*progress));
> +	struct progress *progress = NULL;
> +	if (isatty(2))
> +		progress = malloc(sizeof(*progress));

This is the wrong place to put the fix. The user might have asked git to override the isatty(2) check and show progress anyway (e.g., "git push --progress"), and this would break that case.

The fix has to go in builtin/checkout.c, and probably looks like this:

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3ddda34..e8c1b1f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -343,7 +343,7 @@ static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree)
 	opts.reset = 1;
 	opts.merge = 1;
 	opts.fn = oneway_merge;
-	opts.verbose_update = !o->quiet;
+	opts.verbose_update = !o->quiet && isatty(2);
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 	parse_tree(tree);
@@ -420,7 +420,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		topts.update = 1;
 		topts.merge = 1;
 		topts.gently = opts->merge && old->commit;
-		topts.verbose_update = !opts->quiet;
+		topts.verbose_update = !opts->quiet && isatty(2);
 		topts.fn = twoway_merge;
 		if (opts->overwrite_ignore) {
 			topts.dir = xcalloc(1, sizeof(*topts.dir));

but I did not test it.

-Peff