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

From: Jeff King <peff@peff.net>
Date: Feb 8, 2010, 21:32 UTC
Message-ID: <20100208213256.GA470@coredump.intra.peff.net>
In-Reply-To: <7vvde7h1mn.fsf@alter.siamese.dyndns.org>

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

Show 12 quoted lines

> 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_.

Show 9 quoted lines

> @@ -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