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

From: Junio C Hamano <[email-masked]>
Date: Thu, 21 Jul 2022 13:05:50 -0700

Elijah Newren <[email-masked]> writes:

> On Thu, Jul 21, 2022 at 8:47 AM Junio C Hamano <[email-masked]> wrote:
>>
>> "Elijah Newren via GitGitGadget" <[email-masked]> 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?