From: ZheNing Hu <adlternative@xxxxxxxxx> In the past, git cherry-pick would print such advice when there was a conflict: hint: after resolving the conflicts, mark the corrected paths hint: with 'git add <paths>' or 'git rm <paths>' hint: and commit the result with 'git commit' But in fact, when we want to cherry-pick multiple commits, we should not use "git commit" after resolving conflicts, which will make Git generate some errors. We should recommend users to use `git cherry-pick --continue`, `git cherry-pick --abort`, just like git rebase does. This is the improved advice: hint: Resolve all conflicts manually, mark them as resolved with hint: "git add/rm <conflicted_files>", then run hint: "git cherry-pick --continue". hint: You can instead skip this commit: run "git cherry-pick --skip". hint: To abort and get back to the state before "git cherry-pick", hint: run "git cherry-pick --abort". Similarly, this optimization can be applied to git revert: hint: Resolve all conflicts manually, mark them as resolved with hint: "git add/rm <conflicted_files>", then run hint: "git revert --continue". hint: You can instead skip this commit: run "git revert --skip". hint: To abort and get back to the state before "git revert", hint: run "git revert --abort". It is worth mentioning that now we use advice() to print the content of GIT_CHERRY_PICK_HELP in print_advice(), each line of output will start with "hint: ". Mentored-by: Christian Couder <christian.couder@xxxxxxxxx> Mentored-by Hariom Verma <hariom18599@xxxxxxxxx>: Helped-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> Hepled-by: Junio C Hamano <gitster@xxxxxxxxx> Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx> --- [GSOC] cherry-pick: use better advice message Because git cherry-pick's past advice message is not good enough, it often misleads new users of Git, so this patch makes git chery-pick advice message better. v6: https://lore.kernel.org/git/pull.1010.v3.git.1628142482640.gitgitgadget@xxxxxxxxx/ v6-->v7: 1. Modify the advice order, respect git cherry-pick --no-commit advice. 2. Let git revert also use better message. 3. Use double quotes instead of single quotes for the test body. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1010%2Fadlternative%2Fcherry-pick-help-fix-3-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1010/adlternative/cherry-pick-help-fix-3-v4 Pull-Request: https://github.com/gitgitgadget/git/pull/1010 Range-diff vs v3: 1: 701645dde17 ! 1: dc51c0b8c2b [GSOC] cherry-pick: use better advice message @@ Commit message hint: To abort and get back to the state before "git cherry-pick", hint: run "git cherry-pick --abort". + Similarly, this optimization can be applied to git revert: + + hint: Resolve all conflicts manually, mark them as resolved with + hint: "git add/rm <conflicted_files>", then run + hint: "git revert --continue". + hint: You can instead skip this commit: run "git revert --skip". + hint: To abort and get back to the state before "git revert", + hint: run "git revert --abort". + It is worth mentioning that now we use advice() to print the content of GIT_CHERRY_PICK_HELP in print_advice(), each line of output will start with "hint: ". @@ sequencer.c: static void print_advice(struct repository *r, int show_hint, * 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 (show_hint) { -- if (opts->no_commit) -+ if (opts->action == REPLAY_PICK) { + if (opts->no_commit) + advise(_("after resolving the conflicts, mark the corrected paths\n" + "with 'git add <paths>' or 'git rm <paths>'")); ++ else if (opts->action == REPLAY_PICK) + advise(_("Resolve all conflicts manually, mark them as resolved with\n" + "\"git add/rm <conflicted_files>\", then run\n" + "\"git cherry-pick --continue\".\n" + "You can instead skip this commit: run \"git cherry-pick --skip\".\n" + "To abort and get back to the state before \"git cherry-pick\",\n" + "run \"git cherry-pick --abort\".")); -+ } else if (opts->no_commit) - advise(_("after resolving the conflicts, mark the corrected paths\n" - "with 'git add <paths>' or 'git rm <paths>'")); ++ else if (opts->action == REPLAY_REVERT) ++ advise(_("Resolve all conflicts manually, mark them as resolved with\n" ++ "\"git add/rm <conflicted_files>\", then run\n" ++ "\"git revert --continue\".\n" ++ "You can instead skip this commit: run \"git revert --skip\".\n" ++ "To abort and get back to the state before \"git revert\",\n" ++ "run \"git revert --abort\".")); else + advise(_("after resolving the conflicts, mark the corrected paths\n" + "with 'git add <paths>' or 'git rm <paths>'\n" + + ## t/t3501-revert-cherry-pick.sh ## +@@ t/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick works with dirty renamed file' ' + grep -q "^modified$" renamed + ' + ++test_expect_success 'advice from failed revert' ' ++ echo dream >dream && ++ git add dream && ++ git commit -m "add dream" && ++ dream_oid=$(git rev-parse --short HEAD) && ++ cat <<-EOF >expected && ++ error: could not revert $dream_oid... add dream ++ hint: Resolve all conflicts manually, mark them as resolved with ++ hint: "git add/rm <conflicted_files>", then run ++ hint: "git revert --continue". ++ hint: You can instead skip this commit: run "git revert --skip". ++ hint: To abort and get back to the state before "git revert", ++ hint: run "git revert --abort". ++ EOF ++ echo dream >>dream && ++ git add dream && ++ git commit -m "double-add dream" && ++ test_must_fail git revert HEAD^ 2>actual && ++ test_cmp expected actual ++' + test_done ## t/t3507-cherry-pick-conflict.sh ## -@@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-pick' " - picked=\$(git rev-parse --short picked) && +@@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'failed cherry-pick does not advance HEAD' ' + test "$head" = "$newhead" + ' + +-test_expect_success 'advice from failed cherry-pick' " ++test_expect_success 'advice from failed cherry-pick' ' + pristine_detach initial && + +- picked=\$(git rev-parse --short picked) && ++ picked=$(git rev-parse --short picked) && cat <<-EOF >expected && - error: could not apply \$picked... picked +- 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: and commit the result with 'git commit' ++ error: could not apply $picked... picked + hint: Resolve all conflicts manually, mark them as resolved with -+ hint: \"git add/rm <conflicted_files>\", then run -+ hint: \"git cherry-pick --continue\". -+ hint: You can instead skip this commit: run \"git cherry-pick --skip\". -+ hint: To abort and get back to the state before \"git cherry-pick\", -+ hint: run \"git cherry-pick --abort\". ++ hint: "git add/rm <conflicted_files>", then run ++ hint: "git cherry-pick --continue". ++ hint: You can instead skip this commit: run "git cherry-pick --skip". ++ hint: To abort and get back to the state before "git cherry-pick", ++ hint: run "git cherry-pick --abort". EOF test_must_fail git cherry-pick picked 2>actual && -@@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-pick --no-commit' " - picked=\$(git rev-parse --short picked) && - cat <<-EOF >expected && - 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: Resolve all conflicts manually, mark them as resolved with -+ hint: \"git add/rm <conflicted_files>\", then run -+ hint: \"git cherry-pick --continue\". -+ hint: You can instead skip this commit: run \"git cherry-pick --skip\". -+ hint: To abort and get back to the state before \"git cherry-pick\", -+ hint: run \"git cherry-pick --abort\". - EOF - test_must_fail git cherry-pick --no-commit picked 2>actual && + test_cmp expected actual +-" ++' + test_expect_success 'advice from failed cherry-pick --no-commit' " + pristine_detach initial && sequencer.c | 16 +++++++++++++++- t/t3501-revert-cherry-pick.sh | 20 ++++++++++++++++++++ t/t3507-cherry-pick-conflict.sh | 17 ++++++++++------- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/sequencer.c b/sequencer.c index 0bec01cf38e..2dd73d24a87 100644 --- a/sequencer.c +++ b/sequencer.c @@ -403,7 +403,7 @@ static void print_advice(struct repository *r, int show_hint, char *msg = getenv("GIT_CHERRY_PICK_HELP"); if (msg) { - fprintf(stderr, "%s\n", msg); + advise("%s\n", msg); /* * A conflict has occurred but the porcelain * (typically rebase --interactive) wants to take care @@ -418,6 +418,20 @@ 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>'")); + else if (opts->action == REPLAY_PICK) + advise(_("Resolve all conflicts manually, mark them as resolved with\n" + "\"git add/rm <conflicted_files>\", then run\n" + "\"git cherry-pick --continue\".\n" + "You can instead skip this commit: run \"git cherry-pick --skip\".\n" + "To abort and get back to the state before \"git cherry-pick\",\n" + "run \"git cherry-pick --abort\".")); + else if (opts->action == REPLAY_REVERT) + advise(_("Resolve all conflicts manually, mark them as resolved with\n" + "\"git add/rm <conflicted_files>\", then run\n" + "\"git revert --continue\".\n" + "You can instead skip this commit: run \"git revert --skip\".\n" + "To abort and get back to the state before \"git revert\",\n" + "run \"git revert --abort\".")); else advise(_("after resolving the conflicts, mark the corrected paths\n" "with 'git add <paths>' or 'git rm <paths>'\n" diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh index 9d100cd1884..6766aed7282 100755 --- a/t/t3501-revert-cherry-pick.sh +++ b/t/t3501-revert-cherry-pick.sh @@ -158,4 +158,24 @@ test_expect_success 'cherry-pick works with dirty renamed file' ' grep -q "^modified$" renamed ' +test_expect_success 'advice from failed revert' ' + echo dream >dream && + git add dream && + git commit -m "add dream" && + dream_oid=$(git rev-parse --short HEAD) && + cat <<-EOF >expected && + error: could not revert $dream_oid... add dream + hint: Resolve all conflicts manually, mark them as resolved with + hint: "git add/rm <conflicted_files>", then run + hint: "git revert --continue". + hint: You can instead skip this commit: run "git revert --skip". + hint: To abort and get back to the state before "git revert", + hint: run "git revert --abort". + EOF + echo dream >>dream && + git add dream && + git commit -m "double-add dream" && + test_must_fail git revert HEAD^ 2>actual && + test_cmp expected actual +' test_done diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index 014001b8f32..cb2ebea9ad3 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -47,20 +47,23 @@ test_expect_success 'failed cherry-pick does not advance HEAD' ' test "$head" = "$newhead" ' -test_expect_success 'advice from failed cherry-pick' " +test_expect_success 'advice from failed cherry-pick' ' pristine_detach initial && - picked=\$(git rev-parse --short picked) && + picked=$(git rev-parse --short picked) && cat <<-EOF >expected && - 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: and commit the result with 'git commit' + error: could not apply $picked... picked + hint: Resolve all conflicts manually, mark them as resolved with + hint: "git add/rm <conflicted_files>", then run + hint: "git cherry-pick --continue". + hint: You can instead skip this commit: run "git cherry-pick --skip". + hint: To abort and get back to the state before "git cherry-pick", + hint: run "git cherry-pick --abort". EOF test_must_fail git cherry-pick picked 2>actual && test_cmp expected actual -" +' test_expect_success 'advice from failed cherry-pick --no-commit' " pristine_detach initial && base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006 -- gitgitgadget