Re: [PATCH v2 2/2] builtin/am: allow disabling conflict advice

[Junio C Hamano <gitster@pobox.com>]

phillip.wood123@gmail.com writes:

> Hi Philippe
>
> On 10/03/2024 19:51, Philippe Blain via GitGitGadget wrote:
>> diff --git a/builtin/am.c b/builtin/am.c
>> index d1990d7edcb..0e97b827e4b 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -1150,19 +1150,23 @@ static const char *msgnum(const struct am_state *state)
>>   static void NORETURN die_user_resolve(const struct am_state *state)
>>   {
>>   	if (state->resolvemsg) {
>> -		printf_ln("%s", state->resolvemsg);
>> +		advise_if_enabled(ADVICE_MERGE_CONFLICT, "%s", state->resolvemsg);
>>   	} else {
>>   		const char *cmdline = state->interactive ? "git am -i" : "git am";
>> +		struct strbuf sb = STRBUF_INIT;
>>   -		printf_ln(_("When you have resolved this problem, run
>> \"%s --continue\"."), cmdline);
>> -		printf_ln(_("If you prefer to skip this patch, run \"%s --skip\" instead."), cmdline);
>> +		strbuf_addf(&sb, _("When you have resolved this problem, run \"%s --continue\"."), cmdline);
>> +		strbuf_addf(&sb, _("If you prefer to skip this patch, run \"%s --skip\" instead."), cmdline);
>
> I think you need to append "\n" to the message strings here (and
> below) to match the behavior of printf_ln().

Good eyes.  You'll get the final "\n" but the line breaks inside the
paragraph you give to advise*() functions are your responsibility.
Even though advice.c:vadvise() handles multi-line message better
(unlike usage.c:vreportf() that is used for error() and die()) by
giving a line header for each line of the message, we do not wrap
lines at runtime.

> Apart from that both patches look good to me, thanks for
> re-rolling. It is a bit surprising that we don't need to update any

Thanks, both, and indeed it is a bit surprising.

> rebase tests. I haven't checked but I guess either we're not testing
> this advice when rebasing or we're using a grep expression that is
> vague enough not to be affected.