Re: [PATCH v3 1/7] merge-ort-wrappers: make printed message match the one from recursive

[Elijah Newren <newren@gmail.com>]

On Thu, Jul 21, 2022 at 1:05 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Thu, Jul 21, 2022 at 8:47 AM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >>
> >> >       if (head && repo_index_has_changes(opt->repo, head, &sb)) {
> >> > -             fprintf(stderr, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
> >> > +             struct strbuf err = STRBUF_INIT;
> >> > +             strbuf_addstr(&err, "error: ");
> >> > +             strbuf_addf(&err, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
> >> >                   sb.buf);
> >> > +             strbuf_addch(&err, '\n');
> >> > +             fputs(err.buf, stderr);
> >> > +             strbuf_release(&err);
> >>
> >> Makes me wonder why this is not a mere
> >>
> >>         error(_("Your local chagnes ... by merge:\n  %s"), sb.buf);
> >>
> >> that reuses the exact string.  The err() function in merge-recursive.c
> >> is strangely complex (and probably buggy---if it is not buffering
> >> output, it adds "error: " prefix to opt->obuf before calling vaddf
> >> to add the message, and then sends that to error() to give it
> >> another "error: " prefix), but all the above does is to send a
> >> message to standard error stream.
> >
> > Ah, that would be nicer; thanks for the pointer.  I would still need
> > to prefix it with an
> >     strbuf_addch(&sb, '\n');
> > but two lines certainly beats six.
>
> Your "strbuf" version uses the same format string as my error()
> thing and then manually add one LF at the end, before sending it to
> fputs(), which, unlike puts() does not add any extra LF at the end.
>
> error() gives a terminating newline at the end.
>
> Do you still need to add one more?

Ah, sorry, my mistake.  I somehow thought error() just added the
"error: " prefix.  So, indeed, this is just a change from
fprintf(stderr, ...) to error(...).  No second newline needed.

Thanks!