threads / patch / 40230

patchpush: don't show Done with --quiet --porcelain

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

## tl;dr

5 messages between Aug 31, 2015 and Sep 10, 2015. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Josh Rabinowitz· Aug 31, 2015, 16:40 UTC · lore

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(-)
Show changes to transport.c +1 −2
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)
Junio C Hamano· Sep 1, 2015, 17:13 UTC · re: Josh Rabinowitz · lore

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

Josh Rabinowitz <joshr@joshr.com> writes:
Show 6 quoted lines
> 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.

Show 18 quoted lines
>  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)
Josh Rabinowitz· Sep 4, 2015, 13:11 UTC · re: Junio C Hamano · lore

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

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:
Show 45 quoted lines
> 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)
Junio C Hamano· Sep 4, 2015, 21:42 UTC · re: Josh Rabinowitz · lore

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

Josh Rabinowitz <joshr@joshr.com> writes:
Show 26 quoted lines
> 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.

Josh Rabinowitz· Sep 10, 2015, 20:23 UTC · re: Junio C Hamano · lore

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

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:
Show 33 quoted lines
> 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.

← back to recent threads