Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain

[Jeff King <peff@peff.net>]

On Mon, Feb 08, 2010 at 01:13:36PM -0800, Junio C Hamano wrote:

> diff --git a/builtin-push.c b/builtin-push.c
> index 5633f0a..f5082d8 100644
> --- a/builtin-push.c
> +++ b/builtin-push.c
> @@ -226,6 +226,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>  	git_config(git_default_config, NULL);
>  	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
>  
> +	if (flags & TRANSPORT_PUSH_PORCELAIN) {
> +		/* Do not give advice messages to Porcelain scripts */
> +		advice_push_nonfastforward = 0;
> +	}

I think this is sane.

>  {
>  	if (!count)
> -		fprintf(stderr, "To %s\n", dest);
> +		fprintf(porcelain ? stdout : stderr, "To %s\n", dest);

But note here that you are changing the --porcelain format, as callers
which were keeping only the stdout (and letting stderr go to /dev/null,
or spew to the user) saw only the ref lines. So this may be breaking
such callers.

I think you argued elsewhere (and I agree) that with multiple push urls,
this information is useful. Which means that the original porcelain
format was perhaps not very well thought-out. :( So we have to choose
now whether to fix it and break compatibility, or leave it broken. If
the former, then we should make sure there are not other design issues
in need of fixing, so we can just break compatibility _once_.

> @@ -1071,7 +1071,8 @@ int transport_push(struct transport *transport,
>  		}
>  
>  		if (!quiet && !ret && !refs_pushed(remote_refs))
> -			fprintf(stderr, "Everything up-to-date\n");
> +			fprintf(porcelain ? stdout : stderr,
> +				"Everything up-to-date\n");
>  		return ret;
>  	}

This one, on the other hand, seems to me to be just noise. What does a
--porcelain caller learn by seeing "Everything up-to-date" that it did
not already know from seeing the list of refs?

-Peff