[PATCH v2 0/2] Allow disabling advice shown after merge conflicts

From: Philippe Blain via GitGitGadget <gitgitgadget@gmail.com>
Date: Mar 10, 2024, 19:50 UTC
Message-ID: <pull.1682.v2.git.1710100261.gitgitgadget@gmail.com>
In-Reply-To: <pull.1682.git.1709396291693.gitgitgadget@gmail.com>

This series introduces a new config 'advice.mergeConflict' and uses it to allow disabling the advice shown when 'git rebase', 'git cherry-pick', 'git revert', 'git rebase --apply' and 'git am' stop because of conflicts.

Changes since v1:

 * renamed the new advice to 'advice.mergeConflict' to make it non-sequencer
   specific
 * added 2/2 which uses the advice in builtin/am, which covers 'git rebase
   --apply' and 'git am'

Note that the code path where 'git rebase --apply' stops because of conflicts is not covered by the tests but I tested it manually using this diff:

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 47534f1062..34eac2e6f4 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -374,7 +374,7 @@ test_pull_autostash_fail ()
     echo conflicting >>seq.txt &&
     test_tick &&
     git commit -m "Create conflict" seq.txt &&
-	test_must_fail git pull --rebase . seq 2>err >out &&
+	test_must_fail git -c rebase.backend=apply pull --rebase . seq 2>err >out &&
     test_grep "Resolve all conflicts manually" err
 '


Philippe Blain (2):
  sequencer: allow disabling conflict advice
  builtin/am: allow disabling conflict advice

 Documentation/config/advice.txt |  2 ++
 advice.c                        |  1 +
 advice.h                        |  1 +
 builtin/am.c                    | 14 +++++++++-----
 sequencer.c                     | 33 ++++++++++++++++++---------------
 t/t3501-revert-cherry-pick.sh   |  1 +
 t/t3507-cherry-pick-conflict.sh |  2 ++
 t/t4150-am.sh                   |  8 ++++----
 t/t4254-am-corrupt.sh           |  2 +-
 9 files changed, 39 insertions(+), 25 deletions(-)


base-commit: 0f9d4d28b7e6021b7e6db192b7bf47bd3a0d0d1d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1682%2Fphil-blain%2Fsequencer-conflict-advice-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1682/phil-blain/sequencer-conflict-advice-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1682

Range-diff vs v1:

 1:  e929d3381cf ! 1:  a2ce6fd24c2 sequencer: allow disabling conflict advice
     @@ Commit message
          sequencer: allow disabling conflict advice
      
          Allow disabling the advice shown when a squencer operation results in a
     -    merge conflict through a new config 'advice.sequencerConflict'.
     +    merge conflict through a new config 'advice.mergeConflict', which is
     +    named generically such that it can be used by other commands eventually.
     +
     +    Note that we use 'advise_if_enabled' for each message in the second hunk
     +    in sequencer.c, instead of using 'if (show_hints &&
     +    advice_enabled(...)', because the former instructs the user how to
     +    disable the advice, which is more user-friendly.
      
          Update the tests accordingly. Note that the body of the second test in
          t3507-cherry-pick-conflict.sh is enclosed in double quotes, so we must
     @@ Commit message
      
       ## Documentation/config/advice.txt ##
      @@ Documentation/config/advice.txt: advice.*::
     - 	rmHints::
     - 		In case of failure in the output of linkgit:git-rm[1],
     - 		show directions on how to proceed from the current state.
     -+	sequencerConflict::
     -+		Advice shown when a sequencer operation stops because
     -+		of conflicts.
     - 	sequencerInUse::
     - 		Advice shown when a sequencer command is already in progress.
     - 	skippedCherryPicks::
     + 		Advice on how to set your identity configuration when
     + 		your information is guessed from the system username and
     + 		domain name.
     ++	mergeConflict::
     ++		Advice shown when various commands stop because of conflicts.
     + 	nestedTag::
     + 		Advice shown if a user attempts to recursively tag a tag object.
     + 	pushAlreadyExists::
      
       ## advice.c ##
      @@ advice.c: static struct {
     - 	[ADVICE_RESET_NO_REFRESH_WARNING]		= { "resetNoRefresh" },
     - 	[ADVICE_RESOLVE_CONFLICT]			= { "resolveConflict" },
     - 	[ADVICE_RM_HINTS]				= { "rmHints" },
     -+	[ADVICE_SEQUENCER_CONFLICT]                     = { "sequencerConflict" },
     - 	[ADVICE_SEQUENCER_IN_USE]			= { "sequencerInUse" },
     - 	[ADVICE_SET_UPSTREAM_FAILURE]			= { "setUpstreamFailure" },
     - 	[ADVICE_SKIPPED_CHERRY_PICKS]			= { "skippedCherryPicks" },
     + 	[ADVICE_GRAFT_FILE_DEPRECATED]			= { "graftFileDeprecated" },
     + 	[ADVICE_IGNORED_HOOK]				= { "ignoredHook" },
     + 	[ADVICE_IMPLICIT_IDENTITY]			= { "implicitIdentity" },
     ++	[ADVICE_MERGE_CONFLICT]				= { "mergeConflict" },
     + 	[ADVICE_NESTED_TAG]				= { "nestedTag" },
     + 	[ADVICE_OBJECT_NAME_WARNING]			= { "objectNameWarning" },
     + 	[ADVICE_PUSH_ALREADY_EXISTS]			= { "pushAlreadyExists" },
      
       ## advice.h ##
      @@ advice.h: enum advice_type {
     - 	ADVICE_RESOLVE_CONFLICT,
     - 	ADVICE_RM_HINTS,
     - 	ADVICE_SEQUENCER_IN_USE,
     -+	ADVICE_SEQUENCER_CONFLICT,
     - 	ADVICE_SET_UPSTREAM_FAILURE,
     - 	ADVICE_SKIPPED_CHERRY_PICKS,
     - 	ADVICE_STATUS_AHEAD_BEHIND_WARNING,
     + 	ADVICE_IGNORED_HOOK,
     + 	ADVICE_IMPLICIT_IDENTITY,
     + 	ADVICE_NESTED_TAG,
     ++	ADVICE_MERGE_CONFLICT,
     + 	ADVICE_OBJECT_NAME_WARNING,
     + 	ADVICE_PUSH_ALREADY_EXISTS,
     + 	ADVICE_PUSH_FETCH_FIRST,
      
       ## sequencer.c ##
      @@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
     @@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
       
       	if (msg) {
      -		advise("%s\n", msg);
     -+		advise_if_enabled(ADVICE_SEQUENCER_CONFLICT, "%s\n", msg);
     ++		advise_if_enabled(ADVICE_MERGE_CONFLICT, "%s\n", msg);
       		/*
       		 * A conflict has occurred but the porcelain
       		 * (typically rebase --interactive) wants to take care
     @@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
       		if (opts->no_commit)
      -			advise(_("after resolving the conflicts, mark the corrected paths\n"
      -				 "with 'git add <paths>' or 'git rm <paths>'"));
     -+			advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
     ++			advise_if_enabled(ADVICE_MERGE_CONFLICT,
      +					  _("after resolving the conflicts, mark the corrected paths\n"
      +					    "with 'git add <paths>' or 'git rm <paths>'"));
       		else if (opts->action == REPLAY_PICK)
     @@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
      -				 "You can instead skip this commit with \"git cherry-pick --skip\".\n"
      -				 "To abort and get back to the state before \"git cherry-pick\",\n"
      -				 "run \"git cherry-pick --abort\"."));
     -+			advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
     ++			advise_if_enabled(ADVICE_MERGE_CONFLICT,
      +					  _("After resolving the conflicts, mark them with\n"
      +					    "\"git add/rm <pathspec>\", then run\n"
      +					    "\"git cherry-pick --continue\".\n"
     @@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
      -				 "You can instead skip this commit with \"git revert --skip\".\n"
      -				 "To abort and get back to the state before \"git revert\",\n"
      -				 "run \"git revert --abort\"."));
     -+			advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
     ++			advise_if_enabled(ADVICE_MERGE_CONFLICT,
      +					  _("After resolving the conflicts, mark them with\n"
      +					    "\"git add/rm <pathspec>\", then run\n"
      +					    "\"git revert --continue\".\n"
     @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'advice from failed revert' '
       	hint: You can instead skip this commit with "git revert --skip".
       	hint: To abort and get back to the state before "git revert",
       	hint: run "git revert --abort".
     -+	hint: Disable this message with "git config advice.sequencerConflict false"
     ++	hint: Disable this message with "git config advice.mergeConflict false"
       	EOF
       	test_commit --append --no-tag "double-add dream" dream dream &&
       	test_must_fail git revert HEAD^ 2>actual &&
     @@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-
       	hint: You can instead skip this commit with "git cherry-pick --skip".
       	hint: To abort and get back to the state before "git cherry-pick",
       	hint: run "git cherry-pick --abort".
     -+	hint: Disable this message with "git config advice.sequencerConflict false"
     ++	hint: Disable this message with "git config advice.mergeConflict false"
       	EOF
       	test_must_fail git cherry-pick picked 2>actual &&
       
     @@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-
       	error: could not apply \$picked... picked
       	hint: after resolving the conflicts, mark the corrected paths
       	hint: with 'git add <paths>' or 'git rm <paths>'
     -+	hint: Disable this message with \"git config advice.sequencerConflict false\"
     ++	hint: Disable this message with \"git config advice.mergeConflict false\"
       	EOF
       	test_must_fail git cherry-pick --no-commit picked 2>actual &&
       
 -:  ----------- > 2:  3235542cc6f builtin/am: allow disabling conflict advice

-- 
gitgitgadget