* [PATCH] push: don't show Done with --quiet --porcelain @ 2015-08-31 16:40 Josh Rabinowitz 2015-09-01 17:13 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Josh Rabinowitz @ 2015-08-31 16:40 UTC (permalink / raw) To: git 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) ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] push: don't show Done with --quiet --porcelain 2015-08-31 16:40 [PATCH] push: don't show Done with --quiet --porcelain Josh Rabinowitz @ 2015-09-01 17:13 ` Junio C Hamano 2015-09-04 13:11 ` Josh Rabinowitz 0 siblings, 1 reply; 5+ messages in thread From: Junio C Hamano @ 2015-09-01 17:13 UTC (permalink / raw) To: Josh Rabinowitz; +Cc: git, Larry D'Anna, Tay Ray Chuan 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) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] push: don't show Done with --quiet --porcelain 2015-09-01 17:13 ` Junio C Hamano @ 2015-09-04 13:11 ` Josh Rabinowitz 2015-09-04 21:42 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Josh Rabinowitz @ 2015-09-04 13:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Larry D'Anna, Tay Ray Chuan 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) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] push: don't show Done with --quiet --porcelain 2015-09-04 13:11 ` Josh Rabinowitz @ 2015-09-04 21:42 ` Junio C Hamano 2015-09-10 20:23 ` Josh Rabinowitz 0 siblings, 1 reply; 5+ messages in thread From: Junio C Hamano @ 2015-09-04 21:42 UTC (permalink / raw) To: Josh Rabinowitz; +Cc: git, Larry D'Anna, Tay Ray Chuan 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] push: don't show Done with --quiet --porcelain 2015-09-04 21:42 ` Junio C Hamano @ 2015-09-10 20:23 ` Josh Rabinowitz 0 siblings, 0 replies; 5+ messages in thread From: Josh Rabinowitz @ 2015-09-10 20:23 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Larry D'Anna, Tay Ray Chuan 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-10 20:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-31 16:40 [PATCH] push: don't show Done with --quiet --porcelain Josh Rabinowitz 2015-09-01 17:13 ` Junio C Hamano 2015-09-04 13:11 ` Josh Rabinowitz 2015-09-04 21:42 ` Junio C Hamano 2015-09-10 20:23 ` Josh Rabinowitz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.