Re: [PATCH] status: suppress additional warning output in plumbing modes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 1, 2017, 17:35 UTC
- Message-ID
- <xmqqk23snjpm.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170630162826.27711-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 9 quoted lines
> When status is called with '--porcelain' (as implied by '-z'), we promise > to output only messages as described in the man page. > > Suppress CRLF warnings. > > Signed-off-by: Stefan Beller <sbeller@google.com> > --- > > Maybe something like this?
This looks to me like a stimulus having enough time to go to the spinal cord to induce a knee-jerk reaction, without giving a chance to the brain to think things through.
Surely the reported symptom may have only been about CRLF, but who says that would be the only kind of warning that would be seen during "status --porcelain" codepath?
I tend to agree with Ævar's "output for the script can be read from our standard output" should probably be our first response.
The patch _is_ a good start to document that we may want to do something differently under _PORCELAIN output modes and one location in the code that may be a good place to make that decision, but if we are to squelch the warnings, we should make sure we do not give any warning, not limited to squelching the safe-crlf warning, to the standard error, but still diagnose errors and show error messages, or something like that, I would think.
Show 20 quoted lines
>
> builtin/commit.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 00a01f07c3..3705d5ec6f 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1126,6 +1126,11 @@ static void finalize_deferred_config(struct wt_status *s)
> die(_("--long and -z are incompatible"));
> }
>
> + /* suppress all additional output in porcelain mode */
> + if (status_format == STATUS_FORMAT_PORCELAIN ||
> + status_format == STATUS_FORMAT_PORCELAIN_V2)
> + safe_crlf = SAFE_CRLF_FALSE;
> +
> if (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED)
> status_format = status_deferred_config.status_format;
> if (status_format == STATUS_FORMAT_UNSPECIFIED)