Re: [PATCH 2/3] silence human readable info messages going to stderr from git push --porcelain

[Junio C Hamano <gitster@pobox.com>]

Larry D'Anna <larry@elder-gods.org> writes:

> These messages are redundant information to a script that's calling git-push.

Redundant is not a reason for a change; unwanted would be.  And some of
the messages you are trying to squelch indeed look like unwanted ones, but
not all of them.

> -	if (flags & TRANSPORT_PUSH_VERBOSE)
> +	if (flags & TRANSPORT_PUSH_VERBOSE && !(flags & TRANSPORT_PUSH_PORCELAIN))
>  		fprintf(stderr, "Pushing to %s\n", transport->url);

Why should you be forbidden to expect "--porcelain -v" to give you this
message?

> @@ -123,7 +123,7 @@ static int push_with_options(struct transport *transport, int flags)
>  	if (!err)
>  		return 0;
>  
> -	if (nonfastforward && advice_push_nonfastforward) {
> +	if (!(flags & TRANSPORT_PUSH_PORCELAIN) && nonfastforward && advice_push_nonfastforward) {
>  		fprintf(stderr, "To prevent you from losing history, non-fast-forward updates were rejected\n"
>  				"Merge the remote changes before pushing again.  See the 'Note about\n"
>  				"fast-forwards' section of 'git push --help' for details.\n");

This probably is a good change; the long lines are unsightly but that is a
separate topic.

> diff --git a/transport.c b/transport.c
> index 3846aac..f707c7b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -674,7 +674,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
>  
>  static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
>  {
> -	if (!count)
> +	if (!count && !porcelain)
>  		fprintf(stderr, "To %s\n", dest);

I don't think this is correct.

If you have more than one remote.there.pushURL, the calling Porcelain
script of "git push --porcelain there" should be able to tell which
destination the following report is about, and without this line you
cannot tell.

I would understand if this change were to make the message go to the
standard output when operating with --porcelain option, though.

> @@ -1067,10 +1067,10 @@ int transport_push(struct transport *transport,
>  		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
>  			struct ref *ref;
>  			for (ref = remote_refs; ref; ref = ref->next)
> -				update_tracking_ref(transport->remote, ref, verbose);
> +				update_tracking_ref(transport->remote, ref, verbose && !porcelain);

Again, why  should you be forbidden to expect "--porcelain -v" to work?

> -		if (!quiet && !ret && !refs_pushed(remote_refs))
> +		if (!quiet && !porcelain && !ret && !refs_pushed(remote_refs))
>  			fprintf(stderr, "Everything up-to-date\n");

This is a borderline.  If you are truly up-to-date, the calling script
won't get anything.  It may be easier for Porcelain scripts to see this
message on the standard output as an explicit succeses report instead.