Re: [PATCH v2 2/2] builtin/am: allow disabling conflict advice
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 11, 2024, 17:12 UTC
- Message-ID
- <xmqq5xxsu1z5.fsf@gitster.g>
- In-Reply-To
- <f06dcfad-e4b8-4cb7-8728-f5fb018f7be0@gmail.com>
phillip.wood123@gmail.com writes:
Show 24 quoted lines
> 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.