{"thread":{"id":"40230","subject":"[PATCH] push: don't show Done with --quiet --porcelain","startedAt":"2015-08-31T16:40:45Z","lastAt":"2015-09-10T20:23:07Z","messageCount":5,"participants":["Josh Rabinowitz","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"269002","messageId":"CA+WR+g7frN4uV12dxahxZbr-5az7sj3zx2oN-Z3rKH0_Y8jqqg@mail.gmail.com","threadId":"40230","inReplyTo":null,"subject":"[PATCH] push: don't show Done with --quiet --porcelain","fromName":"Josh Rabinowitz","fromEmail":"joshr@joshr.com","sentAt":"2015-08-31T16:40:45Z","receivedAt":"2015-08-31T16:40:45Z","isPatch":true,"sender":{"key":"joshr@joshr.com","avatar":null},"body":"Change so 'git push --porcelain --quiet' emits no text when there\nis no error. This makes the --quiet option here more consistent with\nother git commands.\n\nSigned-off-by: josh rabinowitz \n---\n transport.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/transport.c b/transport.c\nindex 40692f8..0021b3f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1209,7 +1209,7 @@ int transport_push(struct transport *transport,\n transport_update_tracking_ref(transport->remote, ref, verbose);\n }\n\n- if (porcelain && !push_ret)\n+ if (!quiet && porcelain && !push_ret)\n puts(\"Done\");\n else if (!quiet && !ret && !transport_refs_pushed(remote_refs))\n fprintf(stderr, \"Everything up-to-date\\n\");\n--\n2.3.2 (Apple Git-55)\n"},{"id":"269106","messageId":"xmqqvbburrkf.fsf@gitster.mtv.corp.google.com","threadId":"40230","inReplyTo":"CA+WR+g7frN4uV12dxahxZbr-5az7sj3zx2oN-Z3rKH0_Y8jqqg@mail.gmail.com","subject":"Re: [PATCH] push: don't show Done with --quiet --porcelain","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-01T17:13:20Z","receivedAt":"2015-09-01T17:13:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Rabinowitz writes:\n\n> Change so 'git push --porcelain --quiet' emits no text when there\n> is no error. This makes the --quiet option here more consistent with\n> other git commands.\n>\n> Signed-off-by: josh rabinowitz \n> ---\n\nThe rationale given in 77555854 (git-push: make git push --porcelain\nprint \"Done\", 2010-02-26) does not apply when \"--quiet\" is in use,\nbecause we do give the rejection notice to the standard output even\nunder \"--quiet\", so the calling script can tell between the case\nwhere we couldn't reach the remote side (i.e. no rejection notice)\nand the case where we reached them and they rejected (i.e. they will\ntell us why the push was rejected) when \"git push\" reports a failure\nwith its exit status.\n\nFor that matter, I am not sure if this \"Done\" introduced by 77555854\nis really needed even when \"--quiet\" is not in effect.\n\nIn either case, saying \"Done\" after talking to the remote end\nalready is an established part of the output meant for Porcelain\nwhen \"--porcelain\" option is in use. So I do not think changing it\nis a good idea. Existing scripts that read from \"--porcelain\" output\nwould be expecting the line to be there.\n\n> transport.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/transport.c b/transport.c\n> index 40692f8..0021b3f 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -1209,7 +1209,7 @@ int transport_push(struct transport *transport,\n> transport_update_tracking_ref(transport->remote, ref, verbose);\n> }\n>\n> - if (porcelain && !push_ret)\n> + if (!quiet && porcelain && !push_ret)\n> puts(\"Done\");\n> else if (!quiet && !ret && !transport_refs_pushed(remote_refs))\n> fprintf(stderr, \"Everything up-to-date\\n\");\n> --\n> 2.3.2 (Apple Git-55)\n"},{"id":"269417","messageId":"CA+WR+g7V0Rez_oug5P_LDmafQKXqnDHDNN_vk=-1Sbj3-754DQ@mail.gmail.com","threadId":"40230","inReplyTo":"xmqqvbburrkf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] push: don't show Done with --quiet --porcelain","fromName":"Josh Rabinowitz","fromEmail":"joshr@joshr.com","sentAt":"2015-09-04T13:11:01Z","receivedAt":"2015-09-04T13:11:01Z","isPatch":true,"sender":{"key":"joshr@joshr.com","avatar":null},"body":"Hi Junio and other recipients:\n\nJunio, thanks for your response. I think you may have misunderstood my\npatch though (or I am misunderstanding your responses), because it\nseems we are actually in agreement.\n\n1) My original patch is to make 'git push --porcelain --quiet' not\nemit 'Done' when there is no error. It would continue to emit \"Done\"\nwhen using 'git push --porcelain' without an error.\n\n2) In your first paragraph, you seem to state that while printing\n\"Done\" is advantageous when using 'git push --porcelain' without\n--quiet, the \"Done\" output isn't needed when --quiet is used. This\nappears to agree with my patch's intent.\n\n3) in your second paragraph, you seem to agree with me again, that\n\"Done\" is not needed when \"git push --porcelain --quiet\" is use\n\n4) Then in your third paragraph, you say that you don't want to remove\nthe \"Done\" output when using \"git push --porcelain\" without --quiet --\nwhich my patch preserves (again, it would only remove the \"Done\" text\nwhen 'git push --porcelain' is used with --quiet and there is no\nerror.)\n\nIn summary, I think we are in agreement that this patch is probably\nacceptable. Look forward to reading comments.\n\nBest,\n Josh Rabinowitz\n\n\n\nOn Tue, Sep 1, 2015 at 1:13 PM, Junio C Hamano wrote:\n> Josh Rabinowitz writes:\n>\n>> Change so 'git push --porcelain --quiet' emits no text when there\n>> is no error. This makes the --quiet option here more consistent with\n>> other git commands.\n>>\n>> Signed-off-by: josh rabinowitz \n>> ---\n>\n> The rationale given in 77555854 (git-push: make git push --porcelain\n> print \"Done\", 2010-02-26) does not apply when \"--quiet\" is in use,\n> because we do give the rejection notice to the standard output even\n> under \"--quiet\", so the calling script can tell between the case\n> where we couldn't reach the remote side (i.e. no rejection notice)\n> and the case where we reached them and they rejected (i.e. they will\n> tell us why the push was rejected) when \"git push\" reports a failure\n> with its exit status.\n>\n> For that matter, I am not sure if this \"Done\" introduced by 77555854\n> is really needed even when \"--quiet\" is not in effect.\n>\n> In either case, saying \"Done\" after talking to the remote end\n> already is an established part of the output meant for Porcelain\n> when \"--porcelain\" option is in use. So I do not think changing it\n> is a good idea. Existing scripts that read from \"--porcelain\" output\n> would be expecting the line to be there.\n>\n>> transport.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/transport.c b/transport.c\n>> index 40692f8..0021b3f 100644\n>> --- a/transport.c\n>> +++ b/transport.c\n>> @@ -1209,7 +1209,7 @@ int transport_push(struct transport *transport,\n>> transport_update_tracking_ref(transport->remote, ref, verbose);\n>> }\n>>\n>> - if (porcelain && !push_ret)\n>> + if (!quiet && porcelain && !push_ret)\n>> puts(\"Done\");\n>> else if (!quiet && !ret && !transport_refs_pushed(remote_refs))\n>> fprintf(stderr, \"Everything up-to-date\\n\");\n>> --\n>> 2.3.2 (Apple Git-55)\n"},{"id":"269448","messageId":"xmqqbndhg8un.fsf@gitster.mtv.corp.google.com","threadId":"40230","inReplyTo":"CA+WR+g7V0Rez_oug5P_LDmafQKXqnDHDNN_vk=-1Sbj3-754DQ@mail.gmail.com","subject":"Re: [PATCH] push: don't show Done with --quiet --porcelain","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-04T21:42:08Z","receivedAt":"2015-09-04T21:42:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Rabinowitz writes:\n\n> Hi Junio and other recipients:\n>\n> Junio, thanks for your response. I think you may have misunderstood my\n> patch though (or I am misunderstanding your responses), because it\n> seems we are actually in agreement.\n>\n> 1) My original patch is to make 'git push --porcelain --quiet' not\n> emit 'Done' when there is no error. It would continue to emit \"Done\"\n> when using 'git push --porcelain' without an error.\n>\n> 2) In your first paragraph, you seem to state that while printing\n> \"Done\" is advantageous when using 'git push --porcelain' without\n> --quiet, the \"Done\" output isn't needed when --quiet is used. This\n> appears to agree with my patch's intent.\n>\n> 3) in your second paragraph, you seem to agree with me again, that\n> \"Done\" is not needed when \"git push --porcelain --quiet\" is use\n>\n> 4) Then in your third paragraph, you say that you don't want to remove\n> the \"Done\" output when using \"git push --porcelain\" without --quiet --\n> which my patch preserves (again, it would only remove the \"Done\" text\n> when 'git push --porcelain' is used with --quiet and there is no\n> error.)\n>\n> In summary, I think we are in agreement that this patch is probably\n> acceptable. Look forward to reading comments.\n\nI think your 4. misinterprets what I meant to say.\n\nEven if we agree 1 thru 3, changing the output, with or without\n\"--quiet\", is an unwelcome thing to do to existing scripts.\n"},{"id":"269735","messageId":"CA+WR+g6mU9=izqskNzMYEQc=OJN2rvz9AjGpJ4XB-sMZXoVxwA@mail.gmail.com","threadId":"40230","inReplyTo":"xmqqbndhg8un.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] push: don't show Done with --quiet --porcelain","fromName":"Josh Rabinowitz","fromEmail":"joshr@joshr.com","sentAt":"2015-09-10T20:23:07Z","receivedAt":"2015-09-10T20:23:07Z","isPatch":true,"sender":{"key":"joshr@joshr.com","avatar":null},"body":"Hello, Junio and other recipients:\n\nThanks for your response.\n\nIt just seems very very unlikely that anyone would be depending on a\nnon-error from git pull --porcelain --quiet' to producing the \"Done\"\nstring. In my case, it's something I didn't expect and wanted to\nsuppress. (I've automated the use of that command and wished that the\nonly output would be errors - which is what it's documented to do --\nsee below)\n\n>From a consistency standpoint, it doesn't make sense to have a --quiet\noption output \"Done\" when there is no error.\n\nIf decisions are made that almost no output can ever be changed (which\nis what your opinion seems to be leaning towards) then the code is\nlargely stuck in the present.\n\nIt just seems insane to be stuck with the current behavior of 'git\npush --porcelain --quiet' printing out \"Done\" -- especially since the\ncurrent behavior is in conflict with the docs (at least from 2.3.2):\n\n from 'man git-push':\n -q, --quiet\n Suppress all output, including the listing of updated refs, unless\n an error occurs. Progress is not reported to the standard error\n stream.\n\nIn any case, thanks again for the response and for your additional\nconsideration.\n\nBest,\n Josh\n\nOn Fri, Sep 4, 2015 at 5:42 PM, Junio C Hamano wrote:\n> Josh Rabinowitz writes:\n>\n>> Hi Junio and other recipients:\n>>\n>> Junio, thanks for your response. I think you may have misunderstood my\n>> patch though (or I am misunderstanding your responses), because it\n>> seems we are actually in agreement.\n>>\n>> 1) My original patch is to make 'git push --porcelain --quiet' not\n>> emit 'Done' when there is no error. It would continue to emit \"Done\"\n>> when using 'git push --porcelain' without an error.\n>>\n>> 2) In your first paragraph, you seem to state that while printing\n>> \"Done\" is advantageous when using 'git push --porcelain' without\n>> --quiet, the \"Done\" output isn't needed when --quiet is used. This\n>> appears to agree with my patch's intent.\n>>\n>> 3) in your second paragraph, you seem to agree with me again, that\n>> \"Done\" is not needed when \"git push --porcelain --quiet\" is use\n>>\n>> 4) Then in your third paragraph, you say that you don't want to remove\n>> the \"Done\" output when using \"git push --porcelain\" without --quiet --\n>> which my patch preserves (again, it would only remove the \"Done\" text\n>> when 'git push --porcelain' is used with --quiet and there is no\n>> error.)\n>>\n>> In summary, I think we are in agreement that this patch is probably\n>> acceptable. Look forward to reading comments.\n>\n> I think your 4. misinterprets what I meant to say.\n>\n> Even if we agree 1 thru 3, changing the output, with or without\n> \"--quiet\", is an unwelcome thing to do to existing scripts.\n"}]}