Re: [PATCH v5 1/2] add-patch: do not show UI messages on stderr

[Jeff King <peff@peff.net>]

On Mon, Apr 29, 2024 at 12:24:46PM -0700, Junio C Hamano wrote:

> Rubén Justo <rjusto@gmail.com> writes:
> 
> > diff --git a/add-patch.c b/add-patch.c
> > index 0997d4af73..fc0eed4fd4 100644
> > --- a/add-patch.c
> > +++ b/add-patch.c
> > @@ -293,10 +293,9 @@ static void err(struct add_p_state *s, const char *fmt, ...)
> >  	va_list args;
> >  
> >  	va_start(args, fmt);
> > -	fputs(s->s.error_color, stderr);
> > -	vfprintf(stderr, fmt, args);
> > -	fputs(s->s.reset_color, stderr);
> > -	fputc('\n', stderr);
> > +	fputs(s->s.error_color, stdout);
> > +	vprintf(fmt, args);
> > +	puts(s->s.reset_color);
> 
> I like the attention of the detail here ;-).

Indeed. I had to read this several times to wonder why it was not a
mistake to leave the first fputs() but use vprintf() and puts() for the
other two (for those just reading, the answer is that puts() prints an
extra newline, so we can only use it at the end). Which IMHO really just
points to how inconsistent the stdio interfaces are, but there is
nothing we can do about that. ;)

I am tempted to suggest that it borders on too-clever, and writing out
"stdout" everywhere with vfprintf() and fputs() would be more obvious.
But in a little self-contained function like this I don't know that it
matters much.

-Peff