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().
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 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.
Best Wishes
Phillip
if (advice_enabled(ADVICE_AM_WORK_DIR) &&
is_empty_or_missing_file(am_path(state, "patch")) &&
!repo_index_has_changes(the_repository, NULL, NULL))
- printf_ln(_("To record the empty patch as an empty commit, run \"%s --allow-empty\"."), cmdline);
+ strbuf_addf(&sb, _("To record the empty patch as an empty commit, run \"%s --allow-empty\"."), cmdline);
- printf_ln(_("To restore the original branch and stop patching, run \"%s --abort\"."), cmdline);
+ strbuf_addf(&sb, _("To restore the original branch and stop patching, run \"%s --abort\"."), cmdline);
+
+ advise_if_enabled(ADVICE_MERGE_CONFLICT, "%s", sb.buf);
+ strbuf_release(&sb);
}message instructing the user how to continue the operation. This message
exit(128);
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 3b125762694..5e2b6c80eae 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -1224,8 +1224,8 @@ test_expect_success 'record as an empty commit when meeting e-mail message that
test_expect_success 'skip an empty patch in the middle of an am session' '
git checkout empty-commit^ &&
- test_must_fail git am empty-commit.patch >err &&
- grep "Patch is empty." err &&
+ test_must_fail git am empty-commit.patch >out 2>err &&
+ grep "Patch is empty." out &&
grep "To record the empty patch as an empty commit, run \"git am --allow-empty\"." err &&
git am --skip &&
test_path_is_missing .git/rebase-apply &&
@@ -1236,8 +1236,8 @@ test_expect_success 'skip an empty patch in the middle of an am session' '
test_expect_success 'record an empty patch as an empty commit in the middle of an am session' '
git checkout empty-commit^ &&
- test_must_fail git am empty-commit.patch >err &&
- grep "Patch is empty." err &&
+ test_must_fail git am empty-commit.patch >out 2>err &&
+ grep "Patch is empty." out &&
grep "To record the empty patch as an empty commit, run \"git am --allow-empty\"." err &&
git am --allow-empty >output &&
grep "No changes - recorded it as an empty commit." output &&
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 45f1d4f95e5..661feb60709 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -59,7 +59,7 @@ test_expect_success setup '
# Also, it had the unwanted side-effect of deleting f.
test_expect_success 'try to apply corrupted patch' '
test_when_finished "git am --abort" &&
- test_must_fail git -c advice.amWorkDir=false am bad-patch.diff 2>actual &&
+ test_must_fail git -c advice.amWorkDir=false -c advice.mergeConflict=false am bad-patch.diff 2>actual &&
echo "error: git diff header lacks filename information (line 4)" >expected &&
test_path_is_file f &&
test_cmp expected actual
