[PATCH] push: don't show Done with --quiet --porcelain

[PATCH] push: don't show Done with --quiet --porcelain

[Josh Rabinowitz]

Change so 'git push --porcelain --quiet' emits no text when there
is no error.  This makes the --quiet option here more consistent with
other git commands.

Signed-off-by: josh rabinowitz <joshr@joshr.com>
---
 transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport.c b/transport.c
index 40692f8..0021b3f 100644
--- a/transport.c
+++ b/transport.c
@@ -1209,7 +1209,7 @@ int transport_push(struct transport *transport,
                transport_update_tracking_ref(transport->remote, ref, verbose);
        }

-       if (porcelain && !push_ret)
+       if (!quiet && porcelain && !push_ret)
            puts("Done");
        else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
            fprintf(stderr, "Everything up-to-date\n");
--
2.3.2 (Apple Git-55)

Re: [PATCH] push: don't show Done with --quiet --porcelain

[Junio C Hamano]

Josh Rabinowitz <joshr@joshr.com> writes:

> Change so 'git push --porcelain --quiet' emits no text when there
> is no error.  This makes the --quiet option here more consistent with
> other git commands.
>
> Signed-off-by: josh rabinowitz <joshr@joshr.com>
> ---

The rationale given in 77555854 (git-push: make git push --porcelain
print "Done", 2010-02-26) does not apply when "--quiet" is in use,
because we do give the rejection notice to the standard output even
under "--quiet", so the calling script can tell between the case
where we couldn't reach the remote side (i.e. no rejection notice)
and the case where we reached them and they rejected (i.e. they will
tell us why the push was rejected) when "git push" reports a failure
with its exit status.

For that matter, I am not sure if this "Done" introduced by 77555854
is really needed even when "--quiet" is not in effect.

In either case, saying "Done" after talking to the remote end
already is an established part of the output meant for Porcelain
when "--porcelain" option is in use.  So I do not think changing it
is a good idea.  Existing scripts that read from "--porcelain" output
would be expecting the line to be there.

>  transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/transport.c b/transport.c
> index 40692f8..0021b3f 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1209,7 +1209,7 @@ int transport_push(struct transport *transport,
>                 transport_update_tracking_ref(transport->remote, ref, verbose);
>         }
>
> -       if (porcelain && !push_ret)
> +       if (!quiet && porcelain && !push_ret)
>             puts("Done");
>         else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
>             fprintf(stderr, "Everything up-to-date\n");
> --
> 2.3.2 (Apple Git-55)

Re: [PATCH] push: don't show Done with --quiet --porcelain

[Josh Rabinowitz]

Hi Junio and other recipients:

Junio, thanks for your response. I think you may have misunderstood my
patch though (or I am misunderstanding your responses), because it
seems we are actually in agreement.

1) My original patch is to make 'git push --porcelain --quiet' not
emit 'Done' when there is no error. It would continue to emit "Done"
when using 'git push --porcelain' without an error.

2) In your first paragraph, you seem to state that while printing
"Done" is advantageous when using 'git push --porcelain' without
--quiet, the "Done" output isn't needed when --quiet is used. This
appears to agree with my patch's intent.

3) in your second paragraph, you seem to agree with me again, that
"Done" is not needed when "git push --porcelain --quiet" is use

4) Then in your third paragraph, you say that you don't want to remove
the "Done" output when using "git push --porcelain" without --quiet --
which my patch preserves (again, it would only remove the "Done" text
when 'git push --porcelain' is used with --quiet and there is no
error.)

In summary, I think we are in agreement that this patch is probably
acceptable. Look forward to reading comments.

Best,
 Josh Rabinowitz



On Tue, Sep 1, 2015 at 1:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Josh Rabinowitz <joshr@joshr.com> writes:
>
>> Change so 'git push --porcelain --quiet' emits no text when there
>> is no error.  This makes the --quiet option here more consistent with
>> other git commands.
>>
>> Signed-off-by: josh rabinowitz <joshr@joshr.com>
>> ---
>
> The rationale given in 77555854 (git-push: make git push --porcelain
> print "Done", 2010-02-26) does not apply when "--quiet" is in use,
> because we do give the rejection notice to the standard output even
> under "--quiet", so the calling script can tell between the case
> where we couldn't reach the remote side (i.e. no rejection notice)
> and the case where we reached them and they rejected (i.e. they will
> tell us why the push was rejected) when "git push" reports a failure
> with its exit status.
>
> For that matter, I am not sure if this "Done" introduced by 77555854
> is really needed even when "--quiet" is not in effect.
>
> In either case, saying "Done" after talking to the remote end
> already is an established part of the output meant for Porcelain
> when "--porcelain" option is in use.  So I do not think changing it
> is a good idea.  Existing scripts that read from "--porcelain" output
> would be expecting the line to be there.
>
>>  transport.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/transport.c b/transport.c
>> index 40692f8..0021b3f 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -1209,7 +1209,7 @@ int transport_push(struct transport *transport,
>>                 transport_update_tracking_ref(transport->remote, ref, verbose);
>>         }
>>
>> -       if (porcelain && !push_ret)
>> +       if (!quiet && porcelain && !push_ret)
>>             puts("Done");
>>         else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
>>             fprintf(stderr, "Everything up-to-date\n");
>> --
>> 2.3.2 (Apple Git-55)

Re: [PATCH] push: don't show Done with --quiet --porcelain

[Junio C Hamano]

Josh Rabinowitz <joshr@joshr.com> writes:

> Hi Junio and other recipients:
>
> Junio, thanks for your response. I think you may have misunderstood my
> patch though (or I am misunderstanding your responses), because it
> seems we are actually in agreement.
>
> 1) My original patch is to make 'git push --porcelain --quiet' not
> emit 'Done' when there is no error. It would continue to emit "Done"
> when using 'git push --porcelain' without an error.
>
> 2) In your first paragraph, you seem to state that while printing
> "Done" is advantageous when using 'git push --porcelain' without
> --quiet, the "Done" output isn't needed when --quiet is used. This
> appears to agree with my patch's intent.
>
> 3) in your second paragraph, you seem to agree with me again, that
> "Done" is not needed when "git push --porcelain --quiet" is use
>
> 4) Then in your third paragraph, you say that you don't want to remove
> the "Done" output when using "git push --porcelain" without --quiet --
> which my patch preserves (again, it would only remove the "Done" text
> when 'git push --porcelain' is used with --quiet and there is no
> error.)
>
> In summary, I think we are in agreement that this patch is probably
> acceptable. Look forward to reading comments.

I think your 4. misinterprets what I meant to say.

Even if we agree 1 thru 3, changing the output, with or without
"--quiet", is an unwelcome thing to do to existing scripts.

Re: [PATCH] push: don't show Done with --quiet --porcelain

[Josh Rabinowitz]

Hello, Junio and other recipients:

Thanks for your response.

It just seems very very unlikely that anyone would be depending on a
non-error from git pull --porcelain --quiet' to producing  the "Done"
string. In my case, it's something I didn't expect and wanted to
suppress. (I've automated the use of that command and wished that the
only output would be errors - which is what it's documented to do --
see below)

>From a consistency standpoint, it doesn't make sense to have a --quiet
option output "Done" when there is no error.

If decisions are made that almost no output can ever be changed (which
is what your opinion seems to be leaning towards) then the code is
largely stuck in the present.

It just seems insane to be stuck with the current behavior of 'git
push --porcelain --quiet' printing out "Done" -- especially since the
current behavior is in conflict with the docs (at least from 2.3.2):

  from 'man git-push':
       -q, --quiet
           Suppress all output, including the listing of updated refs, unless
           an error occurs. Progress is not reported to the standard error
           stream.

In any case, thanks again for the response and for your additional
consideration.

Best,
 Josh

On Fri, Sep 4, 2015 at 5:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Josh Rabinowitz <joshr@joshr.com> writes:
>
>> Hi Junio and other recipients:
>>
>> Junio, thanks for your response. I think you may have misunderstood my
>> patch though (or I am misunderstanding your responses), because it
>> seems we are actually in agreement.
>>
>> 1) My original patch is to make 'git push --porcelain --quiet' not
>> emit 'Done' when there is no error. It would continue to emit "Done"
>> when using 'git push --porcelain' without an error.
>>
>> 2) In your first paragraph, you seem to state that while printing
>> "Done" is advantageous when using 'git push --porcelain' without
>> --quiet, the "Done" output isn't needed when --quiet is used. This
>> appears to agree with my patch's intent.
>>
>> 3) in your second paragraph, you seem to agree with me again, that
>> "Done" is not needed when "git push --porcelain --quiet" is use
>>
>> 4) Then in your third paragraph, you say that you don't want to remove
>> the "Done" output when using "git push --porcelain" without --quiet --
>> which my patch preserves (again, it would only remove the "Done" text
>> when 'git push --porcelain' is used with --quiet and there is no
>> error.)
>>
>> In summary, I think we are in agreement that this patch is probably
>> acceptable. Look forward to reading comments.
>
> I think your 4. misinterprets what I meant to say.
>
> Even if we agree 1 thru 3, changing the output, with or without
> "--quiet", is an unwelcome thing to do to existing scripts.