{"thread":{"id":"50667","subject":"[PATCH 0/2] cherry-pick/revert cleanup scissors fix","startedAt":"2019-03-06T10:30:31Z","lastAt":"2019-03-08T03:40:11Z","messageCount":25,"participants":["Denton Liu","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"370801","messageId":"cover.1551867827.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":null,"subject":"[PATCH 0/2] cherry-pick/revert cleanup scissors fix","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-06T10:30:25Z","receivedAt":"2019-03-06T10:30:31Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"As I stated earlier[1], I discovered today that git revert (and thus,\ngit cherry-pick as well) do not place the scissors line properly as\nwell. This patchset adds a scissors line when conflicts arise in both\ncherry-pick and revert, which fixes the same bug present in git-merge\nearlier.\n\nThis patchset should apply on top of dl/merge-cleanup-scissors-fix.\n\n[1]: https://public-inbox.org/git/20190306014143.GA2580@dev-l/\n\nDenton Liu (2):\n t3507: cleanup space after redirection operators\n cherry-pick/revert: add scissors line on merge conflict\n\n Documentation/git-cherry-pick.txt | 7 ++\n Documentation/git-revert.txt | 7 ++\n builtin/merge.c | 13 +---\n builtin/revert.c | 5 ++\n sequencer.c | 22 +++---\n sequencer.h | 3 +-\n t/t3507-cherry-pick-conflict.sh | 122 +++++++++++++++++++++++++-----\n 7 files changed, 138 insertions(+), 41 deletions(-)\n\n-- \n2.19.1.6.g4de041ebd8.dirty\n\n"},{"id":"370802","messageId":"4eee16cc5a841ac20c9ffcd010c0e1feb88059bf.1551867827.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551867827.git.liu.denton@gmail.com","subject":"[PATCH 1/2] t3507: cleanup space after redirection operators","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-06T10:30:28Z","receivedAt":"2019-03-06T10:30:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n t/t3507-cherry-pick-conflict.sh | 34 ++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 0db166152a..74ff925526 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -88,7 +88,7 @@ test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' '\n \n test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git cherry-pick base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n@@ -96,7 +96,7 @@ test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '\n test_expect_success \\\n \t'cherry-pick --strategy=resolve w/dirty tree does not set CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git cherry-pick --strategy=resolve base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n@@ -175,23 +175,23 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n \t\tgit ls-files --stage foo &&\n \t\tgit checkout picked -- foo &&\n \t\tgit ls-files --stage foo\n-\t} > stages &&\n+\t} >stages &&\n \tsed \"\n \t\t1 s/ 0\t/ 1\t/\n \t\t2 s/ 0\t/ 2\t/\n \t\t3 s/ 0\t/ 3\t/\n-\t\" < stages > expected &&\n+\t\" expected &&\n \tgit read-tree -u --reset HEAD &&\n \n \ttest_must_fail git cherry-pick picked &&\n-\tgit ls-files --stage --unmerged > actual &&\n+\tgit ls-files --stage --unmerged >actual &&\n \n \ttest_cmp expected actual\n '\n \n test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \tpristine_detach initial &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t=======\n@@ -201,14 +201,14 @@ test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \n \ttest_must_fail git cherry-pick picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'diff3 -m style' '\n \tpristine_detach initial &&\n \tgit config merge.conflictstyle diff3 &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t||||||| parent of objid picked\n@@ -220,14 +220,14 @@ test_expect_success 'diff3 -m style' '\n \n \ttest_must_fail git cherry-pick picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'revert also handles conflicts sanely' '\n \tgit config --unset merge.conflictstyle &&\n \tpristine_detach initial &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t=======\n@@ -241,24 +241,24 @@ test_expect_success 'revert also handles conflicts sanely' '\n \t\tgit ls-files --stage foo &&\n \t\tgit checkout base -- foo &&\n \t\tgit ls-files --stage foo\n-\t} > stages &&\n+\t} >stages &&\n \tsed \"\n \t\t1 s/ 0\t/ 1\t/\n \t\t2 s/ 0\t/ 2\t/\n \t\t3 s/ 0\t/ 3\t/\n-\t\" < stages > expected-stages &&\n+\t\" expected-stages &&\n \tgit read-tree -u --reset HEAD &&\n \n \thead=$(git rev-parse HEAD) &&\n \ttest_must_fail git revert picked &&\n \tnewhead=$(git rev-parse HEAD) &&\n-\tgit ls-files --stage --unmerged > actual-stages &&\n+\tgit ls-files --stage --unmerged >actual-stages &&\n \n \ttest \"$head\" = \"$newhead\" &&\n \ttest_must_fail git update-index --refresh -q &&\n \ttest_must_fail git diff-index --exit-code HEAD &&\n \ttest_cmp expected-stages actual-stages &&\n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n@@ -284,7 +284,7 @@ test_expect_success 'revert --no-commit sets REVERT_HEAD' '\n \n test_expect_success 'revert w/dirty tree does not set REVERT_HEAD' '\n \tpristine_detach base &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git revert base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n \ttest_must_fail git rev-parse --verify REVERT_HEAD\n@@ -319,7 +319,7 @@ test_expect_success 'failed commit does not clear REVERT_HEAD' '\n test_expect_success 'revert conflict, diff3 -m style' '\n \tpristine_detach initial &&\n \tgit config merge.conflictstyle diff3 &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t||||||| objid picked\n@@ -331,7 +331,7 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \n \ttest_must_fail git revert picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n-- \n2.19.1.6.g4de041ebd8.dirty\n\n"},{"id":"370803","messageId":"70a508ca0b2d837b311afefcc2b0ffb6cfbd34fb.1551867827.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551867827.git.liu.denton@gmail.com","subject":"[PATCH 2/2] cherry-pick/revert: add scissors line on merge conflict","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-06T10:30:31Z","receivedAt":"2019-03-06T10:30:36Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Fix a bug where the scissors line is placed after the Conflicts:\nsection, in the case where a merge conflict occurs and\ncommit.cleanup = scissors.\n\nNote that the removal of the if-else tower in git_sequencer_config may\nappear to be a no-op refactor but it actually isn't. First of all, we\nnow accept \"default\" as a configuration option and also we die on an\ninvalid cleanup mode. Most importantly, though, if\ncommit.cleanup = scissors, the cleanup enum will be set to\nCOMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE. This\nallows us to append scissors to MERGE_MSG in the case of a conflict.\n\nSigned-off-by: Denton Liu \n---\n Documentation/git-cherry-pick.txt | 7 +++\n Documentation/git-revert.txt | 7 +++\n builtin/merge.c | 13 ++---\n builtin/revert.c | 5 ++\n sequencer.c | 22 ++++----\n sequencer.h | 3 +-\n t/t3507-cherry-pick-conflict.sh | 88 +++++++++++++++++++++++++++++++\n 7 files changed, 121 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex d35d771fc8..5c086d78c8 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -57,6 +57,13 @@ OPTIONS\n \tWith this option, 'git cherry-pick' will let you edit the commit\n \tmessage prior to committing.\n \n+--cleanup=::\n+\tThis option determines how the commit message will be cleaned up before\n+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n+\taddition, if the '' is given a value of `scissors`, scissors will\n+\tbe prepended to MERGE_MSG before being passed on in the case of a\n+\tconflict.\n+\n -x::\n \tWhen recording the commit, append a line that says\n \t\"(cherry picked from commit ...)\" to the original commit\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex 837707a8fd..1894010e60 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -66,6 +66,13 @@ more details.\n \tWith this option, 'git revert' will not start the commit\n \tmessage editor.\n \n+--cleanup=::\n+\tThis option determines how the commit message will be cleaned up before\n+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n+\taddition, if the '' is given a value of `scissors`, scissors will\n+\tbe prepended to MERGE_MSG before being passed on in the case of a\n+\tconflict.\n+\n -n::\n --no-commit::\n \tUsually the command automatically creates some commits with\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 92efc3d8fa..d4217ebcf5 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -913,17 +913,10 @@ static int suggest_conflicts(void)\n \t * We can't use cleanup_mode because if we're not using the editor,\n \t * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even\n \t * though the message is meant to be processed later by git-commit.\n-\t * Thus, we will get the cleanup mode is returned we _are_ using an\n-\t * editor.\n+\t * Thus, we will get the cleanup mode which is returned when we _are_ using\n+\t * an editor.\n \t */\n-\tif (get_cleanup_mode(cleanup_arg, 1) == COMMIT_MSG_CLEANUP_SCISSORS) {\n-\t fputc('\\n', fp);\n-\t wt_status_add_cut_line(fp);\n-\t /* comments out the newline from append_conflicts_hint */\n-\t fputc(comment_line_char, fp);\n-\t}\n-\n-\tappend_conflicts_hint(&msgbuf);\n+\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n \tfputs(msgbuf.buf, fp);\n \tstrbuf_release(&msgbuf);\n \tfclose(fp);\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 9a66720cfc..fe18036be7 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n {\n \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n \tconst char *me = action_name(opts);\n+\tconst char *cleanup_arg = NULL;\n \tint cmd = 0;\n \tstruct option base_options[] = {\n \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n+\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \tif (opts->keep_redundant_commits)\n \t\topts->allow_empty = 1;\n \n+\tif (cleanup_arg)\n+\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n+\n \t/* Check for incompatible command line arguments */\n \tif (cmd) {\n \t\tchar *this_operation;\ndiff --git a/sequencer.c b/sequencer.c\nindex 707e72fb39..85ad58555d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -165,17 +165,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n \t\tif (status)\n \t\t\treturn status;\n \n-\t\tif (!strcmp(s, \"verbatim\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n-\t\telse if (!strcmp(s, \"whitespace\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n-\t\telse if (!strcmp(s, \"strip\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n-\t\telse if (!strcmp(s, \"scissors\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n-\t\telse\n-\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n-\t\t\t\t s);\n+\t\topts->default_msg_cleanup = get_cleanup_mode(s, 1);\n \n \t\tfree((char *)s);\n \t\treturn status;\n@@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n }\n \n-void append_conflicts_hint(struct strbuf *msgbuf)\n+void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n {\n \tint i;\n \n+\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n+\t\tstrbuf_addch(msgbuf, '\\n');\n+\t\twt_status_append_cut_line(msgbuf);\n+\t\tstrbuf_addch(msgbuf, comment_line_char);\n+\t}\n+\n \tstrbuf_addch(msgbuf, '\\n');\n \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n \tfor (i = 0; i < active_nr;) {\n@@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \t\t\t_(action_name(opts)));\n \n \tif (!clean)\n-\t\tappend_conflicts_hint(msgbuf);\n+\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n \n \treturn !clean;\n }\ndiff --git a/sequencer.h b/sequencer.h\nindex 5690e0c27e..aa99503dd7 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -91,7 +91,8 @@ int rearrange_squash(void);\n extern const char sign_off_header[];\n \n void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n-void append_conflicts_hint(struct strbuf *msgbuf);\n+void append_conflicts_hint(struct strbuf *msgbuf,\n+\t\tenum commit_msg_cleanup_mode cleanup_mode);\n enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \tint use_editor);\n \ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 74ff925526..c94e44dad0 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -189,6 +189,48 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success \\\n+\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config commit.cleanup scissors &&\n+\tcat <<-EOF >expected &&\n+\t\tpicked\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git cherry-pick picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n+test_expect_success \\\n+\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config --unset commit.cleanup &&\n+\tcat <<-EOF >expected &&\n+\t\tpicked\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \tpristine_detach initial &&\n \tcat <<-EOF >expected &&\n@@ -335,6 +377,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success \\\n+\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config commit.cleanup scissors &&\n+\tcat >expected <<-EOF &&\n+\t\tRevert \"picked\"\n+\n+\t\tThis reverts commit objid.\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git revert picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n+test_expect_success \\\n+\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config --unset commit.cleanup &&\n+\tcat >expected <<-EOF &&\n+\t\tRevert \"picked\"\n+\n+\t\tThis reverts commit objid.\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git revert --cleanup=scissors picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'failed cherry-pick does not forget -s' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked &&\n-- \n2.19.1.6.g4de041ebd8.dirty\n\n"},{"id":"370817","messageId":"1fa469d7-76ef-36b5-9688-43853fa2b2ee@gmail.com","threadId":"50667","inReplyTo":"70a508ca0b2d837b311afefcc2b0ffb6cfbd34fb.1551867827.git.liu.denton@gmail.com","subject":"Re: [PATCH 2/2] cherry-pick/revert: add scissors line on merge conflict","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-03-06T16:29:20Z","receivedAt":"2019-03-06T16:29:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Denton\n\nOn 06/03/2019 10:30, Denton Liu wrote:\n> Fix a bug where the scissors line is placed after the Conflicts:\n> section, in the case where a merge conflict occurs and\n> commit.cleanup = scissors.\n\nWas that the case with cherry-pick and revert as well as merge, or were \nthey missing the scissors line entirely as the subject line suggests?\n\n> Note that the removal of the if-else tower in git_sequencer_config may\n> appear to be a no-op refactor but it actually isn't. First of all, we\n> now accept \"default\" as a configuration option and also we die on an\n> invalid cleanup mode. Most importantly, though, if\n> commit.cleanup = scissors, the cleanup enum will be set to\n> COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE. This\n> allows us to append scissors to MERGE_MSG in the case of a conflict.\n\nI've got some comments about this change below\n\n> Signed-off-by: Denton Liu \n> ---\n> Documentation/git-cherry-pick.txt | 7 +++\n> Documentation/git-revert.txt | 7 +++\n> builtin/merge.c | 13 ++---\n> builtin/revert.c | 5 ++\n> sequencer.c | 22 ++++----\n> sequencer.h | 3 +-\n> t/t3507-cherry-pick-conflict.sh | 88 +++++++++++++++++++++++++++++++\n> 7 files changed, 121 insertions(+), 24 deletions(-)\n> \n> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> index d35d771fc8..5c086d78c8 100644\n> --- a/Documentation/git-cherry-pick.txt\n> +++ b/Documentation/git-cherry-pick.txt\n> @@ -57,6 +57,13 @@ OPTIONS\n> \tWith this option, 'git cherry-pick' will let you edit the commit\n> \tmessage prior to committing.\n> \n> +--cleanup=::\n> +\tThis option determines how the commit message will be cleaned up before\n> +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> +\taddition, if the '' is given a value of `scissors`, scissors will\n> +\tbe prepended to MERGE_MSG before being passed on in the case of a\n> +\tconflict.\n> +\n> -x::\n> \tWhen recording the commit, append a line that says\n> \t\"(cherry picked from commit ...)\" to the original commit\n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index 837707a8fd..1894010e60 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -66,6 +66,13 @@ more details.\n> \tWith this option, 'git revert' will not start the commit\n> \tmessage editor.\n> \n> +--cleanup=::\n> +\tThis option determines how the commit message will be cleaned up before\n> +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> +\taddition, if the '' is given a value of `scissors`, scissors will\n> +\tbe prepended to MERGE_MSG before being passed on in the case of a\n> +\tconflict.\n> +\n> -n::\n> --no-commit::\n> \tUsually the command automatically creates some commits with\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 92efc3d8fa..d4217ebcf5 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -913,17 +913,10 @@ static int suggest_conflicts(void)\n> \t * We can't use cleanup_mode because if we're not using the editor,\n> \t * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even\n> \t * though the message is meant to be processed later by git-commit.\n> -\t * Thus, we will get the cleanup mode is returned we _are_ using an\n> -\t * editor.\n> +\t * Thus, we will get the cleanup mode which is returned when we _are_ using\n> +\t * an editor.\n> \t */\n> -\tif (get_cleanup_mode(cleanup_arg, 1) == COMMIT_MSG_CLEANUP_SCISSORS) {\n> -\t fputc('\\n', fp);\n> -\t wt_status_add_cut_line(fp);\n> -\t /* comments out the newline from append_conflicts_hint */\n> -\t fputc(comment_line_char, fp);\n> -\t}\n> -\n> -\tappend_conflicts_hint(&msgbuf);\n> +\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n> \tfputs(msgbuf.buf, fp);\n> \tstrbuf_release(&msgbuf);\n> \tfclose(fp);\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 9a66720cfc..fe18036be7 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> {\n> \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n> \tconst char *me = action_name(opts);\n> +\tconst char *cleanup_arg = NULL;\n> \tint cmd = 0;\n> \tstruct option base_options[] = {\n> \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n> \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n> \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n> +\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n> \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n> \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n> \t\tOPT_NOOP_NOARG('r', NULL),\n> @@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \tif (opts->keep_redundant_commits)\n> \t\topts->allow_empty = 1;\n> \n> +\tif (cleanup_arg)\n> +\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n> +\n> \t/* Check for incompatible command line arguments */\n> \tif (cmd) {\n> \t\tchar *this_operation;\n> diff --git a/sequencer.c b/sequencer.c\n> index 707e72fb39..85ad58555d 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -165,17 +165,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n> \t\tif (status)\n> \t\t\treturn status;\n> \n> -\t\tif (!strcmp(s, \"verbatim\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n> -\t\telse if (!strcmp(s, \"whitespace\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> -\t\telse if (!strcmp(s, \"strip\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n> -\t\telse if (!strcmp(s, \"scissors\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> -\t\telse\n> -\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n> -\t\t\t\t s);\n> +\t\topts->default_msg_cleanup = get_cleanup_mode(s, 1);\n\nIf we're rebasing then I'm not sure we want to pass 1 here, we won't be \nusing an editor unless the user is rewording a commit message. Also I'm \nnot clear about the implications of changing the cleanup mode for \nscissors - will whitespace and comments still be cleaned up?\n\nI'm not terribly happy with the idea that this will now die while \nreading the config file if there's on invalid cleanup option. If you \nreally want to enforce a valid value then it would be better to return \nan error from get_cleanup_mode().\n\nBest Wishes\n\nPhillip\n\n> \n> \t\tfree((char *)s);\n> \t\treturn status;\n> @@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> }\n> \n> -void append_conflicts_hint(struct strbuf *msgbuf)\n> +void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n> {\n> \tint i;\n> \n> +\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n> +\t\tstrbuf_addch(msgbuf, '\\n');\n> +\t\twt_status_append_cut_line(msgbuf);\n> +\t\tstrbuf_addch(msgbuf, comment_line_char);\n> +\t}\n> +\n> \tstrbuf_addch(msgbuf, '\\n');\n> \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n> \tfor (i = 0; i < active_nr;) {\n> @@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n> \t\t\t_(action_name(opts)));\n> \n> \tif (!clean)\n> -\t\tappend_conflicts_hint(msgbuf);\n> +\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n> \n> \treturn !clean;\n> }\n> diff --git a/sequencer.h b/sequencer.h\n> index 5690e0c27e..aa99503dd7 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -91,7 +91,8 @@ int rearrange_squash(void);\n> extern const char sign_off_header[];\n> \n> void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n> -void append_conflicts_hint(struct strbuf *msgbuf);\n> +void append_conflicts_hint(struct strbuf *msgbuf,\n> +\t\tenum commit_msg_cleanup_mode cleanup_mode);\n> enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> \tint use_editor);\n> \n> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n> index 74ff925526..c94e44dad0 100755\n> --- a/t/t3507-cherry-pick-conflict.sh\n> +++ b/t/t3507-cherry-pick-conflict.sh\n> @@ -189,6 +189,48 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success \\\n> +\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config commit.cleanup scissors &&\n> +\tcat <<-EOF >expected &&\n> +\t\tpicked\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git cherry-pick picked &&\n> +\n> +\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> +\ttest_i18ncmp expected actual\n> +'\n> +\n> +test_expect_success \\\n> +\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config --unset commit.cleanup &&\n> +\tcat <<-EOF >expected &&\n> +\t\tpicked\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n> +\n> +\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> +\ttest_i18ncmp expected actual\n> +'\n> +\n> test_expect_success 'failed cherry-pick describes conflict in work tree' '\n> \tpristine_detach initial &&\n> \tcat <<-EOF >expected &&\n> @@ -335,6 +377,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success \\\n> +\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config commit.cleanup scissors &&\n> +\tcat >expected <<-EOF &&\n> +\t\tRevert \"picked\"\n> +\n> +\t\tThis reverts commit objid.\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git revert picked &&\n> +\n> +\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> +\ttest_i18ncmp expected actual\n> +'\n> +\n> +test_expect_success \\\n> +\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config --unset commit.cleanup &&\n> +\tcat >expected <<-EOF &&\n> +\t\tRevert \"picked\"\n> +\n> +\t\tThis reverts commit objid.\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git revert --cleanup=scissors picked &&\n> +\n> +\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> +\ttest_i18ncmp expected actual\n> +'\n> +\n> test_expect_success 'failed cherry-pick does not forget -s' '\n> \tpristine_detach initial &&\n> \ttest_must_fail git cherry-pick -s picked &&\n> \n"},{"id":"370847","messageId":"20190307000457.GA24933@dev-l","threadId":"50667","inReplyTo":"1fa469d7-76ef-36b5-9688-43853fa2b2ee@gmail.com","subject":"Re: [PATCH 2/2] cherry-pick/revert: add scissors line on merge conflict","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T00:04:57Z","receivedAt":"2019-03-07T00:05:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Phillip,\n\nOn Wed, Mar 06, 2019 at 04:29:20PM +0000, Phillip Wood wrote:\n> Hi Denton\n> \n> On 06/03/2019 10:30, Denton Liu wrote:\n> >Fix a bug where the scissors line is placed after the Conflicts:\n> >section, in the case where a merge conflict occurs and\n> >commit.cleanup = scissors.\n> \n> Was that the case with cherry-pick and revert as well as merge, or were they\n> missing the scissors line entirely as the subject line suggests?\n> \n> >Note that the removal of the if-else tower in git_sequencer_config may\n> >appear to be a no-op refactor but it actually isn't. First of all, we\n> >now accept \"default\" as a configuration option and also we die on an\n> >invalid cleanup mode. Most importantly, though, if\n> >commit.cleanup = scissors, the cleanup enum will be set to\n> >COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE. This\n> >allows us to append scissors to MERGE_MSG in the case of a conflict.\n> \n> I've got some comments about this change below\n> \n> >Signed-off-by: Denton Liu \n> >---\n> > Documentation/git-cherry-pick.txt | 7 +++\n> > Documentation/git-revert.txt | 7 +++\n> > builtin/merge.c | 13 ++---\n> > builtin/revert.c | 5 ++\n> > sequencer.c | 22 ++++----\n> > sequencer.h | 3 +-\n> > t/t3507-cherry-pick-conflict.sh | 88 +++++++++++++++++++++++++++++++\n> > 7 files changed, 121 insertions(+), 24 deletions(-)\n> >\n> >diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> >index d35d771fc8..5c086d78c8 100644\n> >--- a/Documentation/git-cherry-pick.txt\n> >+++ b/Documentation/git-cherry-pick.txt\n> >@@ -57,6 +57,13 @@ OPTIONS\n> > \tWith this option, 'git cherry-pick' will let you edit the commit\n> > \tmessage prior to committing.\n> >+--cleanup=::\n> >+\tThis option determines how the commit message will be cleaned up before\n> >+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> >+\taddition, if the '' is given a value of `scissors`, scissors will\n> >+\tbe prepended to MERGE_MSG before being passed on in the case of a\n> >+\tconflict.\n> >+\n> > -x::\n> > \tWhen recording the commit, append a line that says\n> > \t\"(cherry picked from commit ...)\" to the original commit\n> >diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> >index 837707a8fd..1894010e60 100644\n> >--- a/Documentation/git-revert.txt\n> >+++ b/Documentation/git-revert.txt\n> >@@ -66,6 +66,13 @@ more details.\n> > \tWith this option, 'git revert' will not start the commit\n> > \tmessage editor.\n> >+--cleanup=::\n> >+\tThis option determines how the commit message will be cleaned up before\n> >+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> >+\taddition, if the '' is given a value of `scissors`, scissors will\n> >+\tbe prepended to MERGE_MSG before being passed on in the case of a\n> >+\tconflict.\n> >+\n> > -n::\n> > --no-commit::\n> > \tUsually the command automatically creates some commits with\n> >diff --git a/builtin/merge.c b/builtin/merge.c\n> >index 92efc3d8fa..d4217ebcf5 100644\n> >--- a/builtin/merge.c\n> >+++ b/builtin/merge.c\n> >@@ -913,17 +913,10 @@ static int suggest_conflicts(void)\n> > \t * We can't use cleanup_mode because if we're not using the editor,\n> > \t * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even\n> > \t * though the message is meant to be processed later by git-commit.\n> >-\t * Thus, we will get the cleanup mode is returned we _are_ using an\n> >-\t * editor.\n> >+\t * Thus, we will get the cleanup mode which is returned when we _are_ using\n> >+\t * an editor.\n> > \t */\n> >-\tif (get_cleanup_mode(cleanup_arg, 1) == COMMIT_MSG_CLEANUP_SCISSORS) {\n> >-\t fputc('\\n', fp);\n> >-\t wt_status_add_cut_line(fp);\n> >-\t /* comments out the newline from append_conflicts_hint */\n> >-\t fputc(comment_line_char, fp);\n> >-\t}\n> >-\n> >-\tappend_conflicts_hint(&msgbuf);\n> >+\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n> > \tfputs(msgbuf.buf, fp);\n> > \tstrbuf_release(&msgbuf);\n> > \tfclose(fp);\n> >diff --git a/builtin/revert.c b/builtin/revert.c\n> >index 9a66720cfc..fe18036be7 100644\n> >--- a/builtin/revert.c\n> >+++ b/builtin/revert.c\n> >@@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> > {\n> > \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n> > \tconst char *me = action_name(opts);\n> >+\tconst char *cleanup_arg = NULL;\n> > \tint cmd = 0;\n> > \tstruct option base_options[] = {\n> > \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n> > \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n> > \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n> >+\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n> > \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n> > \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n> > \t\tOPT_NOOP_NOARG('r', NULL),\n> >@@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> > \tif (opts->keep_redundant_commits)\n> > \t\topts->allow_empty = 1;\n> >+\tif (cleanup_arg)\n> >+\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n> >+\n> > \t/* Check for incompatible command line arguments */\n> > \tif (cmd) {\n> > \t\tchar *this_operation;\n> >diff --git a/sequencer.c b/sequencer.c\n> >index 707e72fb39..85ad58555d 100644\n> >--- a/sequencer.c\n> >+++ b/sequencer.c\n> >@@ -165,17 +165,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n> > \t\tif (status)\n> > \t\t\treturn status;\n> >-\t\tif (!strcmp(s, \"verbatim\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n> >-\t\telse if (!strcmp(s, \"whitespace\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> >-\t\telse if (!strcmp(s, \"strip\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n> >-\t\telse if (!strcmp(s, \"scissors\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> >-\t\telse\n> >-\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n> >-\t\t\t\t s);\n> >+\t\topts->default_msg_cleanup = get_cleanup_mode(s, 1);\n> \n> If we're rebasing then I'm not sure we want to pass 1 here, we won't be\n> using an editor unless the user is rewording a commit message. Also I'm not\n> clear about the implications of changing the cleanup mode for scissors -\n> will whitespace and comments still be cleaned up?\n\nI believe that rebase -i should be safe with the change but to be sure,\nI'll change the line to read:\n\n\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n\n(Note to self, this will require a change in rebase--helper.c too.)\n\n> I'm not terribly happy with the idea that this will now die while reading\n> the config file if there's on invalid cleanup option. If you really want to\n> enforce a valid value then it would be better to return an error from\n> get_cleanup_mode().\n\nThe reason why I made it die was because I wanted to reuse that function\nas much as possible and it already happened to be implemented that way.\nHowever, there is prescedent in the code for dieing on invalid\nconfiguration options, namely in date.c:\n\n\tif (mode->type == DATE_STRFTIME) {\n\t\tif (!skip_prefix(p, \":\", &p))\n\t\t\tdie(\"date format missing colon separator: %s\", format);\n\t\tmode->strftime_fmt = xstrdup(p);\n\t} else if (*p)\n\t\tdie(\"unknown date format %s\", format);\n\nThat being said, I'm pretty ambivalent between making it a warning and a\ndie. If you have any strong opinions on this, I can change it to a\nwarning.\n\nThanks,\n\nDenton\n\n> \n> Best Wishes\n> \n> Phillip\n> \n> > \t\tfree((char *)s);\n> > \t\treturn status;\n> >@@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> > \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> > }\n> >-void append_conflicts_hint(struct strbuf *msgbuf)\n> >+void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n> > {\n> > \tint i;\n> >+\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n> >+\t\tstrbuf_addch(msgbuf, '\\n');\n> >+\t\twt_status_append_cut_line(msgbuf);\n> >+\t\tstrbuf_addch(msgbuf, comment_line_char);\n> >+\t}\n> >+\n> > \tstrbuf_addch(msgbuf, '\\n');\n> > \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n> > \tfor (i = 0; i < active_nr;) {\n> >@@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n> > \t\t\t_(action_name(opts)));\n> > \tif (!clean)\n> >-\t\tappend_conflicts_hint(msgbuf);\n> >+\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n> > \treturn !clean;\n> > }\n> >diff --git a/sequencer.h b/sequencer.h\n> >index 5690e0c27e..aa99503dd7 100644\n> >--- a/sequencer.h\n> >+++ b/sequencer.h\n> >@@ -91,7 +91,8 @@ int rearrange_squash(void);\n> > extern const char sign_off_header[];\n> > void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n> >-void append_conflicts_hint(struct strbuf *msgbuf);\n> >+void append_conflicts_hint(struct strbuf *msgbuf,\n> >+\t\tenum commit_msg_cleanup_mode cleanup_mode);\n> > enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> > \tint use_editor);\n> >diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n> >index 74ff925526..c94e44dad0 100755\n> >--- a/t/t3507-cherry-pick-conflict.sh\n> >+++ b/t/t3507-cherry-pick-conflict.sh\n> >@@ -189,6 +189,48 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n> > \ttest_cmp expected actual\n> > '\n> >+test_expect_success \\\n> >+\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config commit.cleanup scissors &&\n> >+\tcat <<-EOF >expected &&\n> >+\t\tpicked\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git cherry-pick picked &&\n> >+\n> >+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> >+\ttest_i18ncmp expected actual\n> >+'\n> >+\n> >+test_expect_success \\\n> >+\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config --unset commit.cleanup &&\n> >+\tcat <<-EOF >expected &&\n> >+\t\tpicked\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n> >+\n> >+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> >+\ttest_i18ncmp expected actual\n> >+'\n> >+\n> > test_expect_success 'failed cherry-pick describes conflict in work tree' '\n> > \tpristine_detach initial &&\n> > \tcat <<-EOF >expected &&\n> >@@ -335,6 +377,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n> > \ttest_cmp expected actual\n> > '\n> >+test_expect_success \\\n> >+\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config commit.cleanup scissors &&\n> >+\tcat >expected <<-EOF &&\n> >+\t\tRevert \"picked\"\n> >+\n> >+\t\tThis reverts commit objid.\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git revert picked &&\n> >+\n> >+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> >+\ttest_i18ncmp expected actual\n> >+'\n> >+\n> >+test_expect_success \\\n> >+\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config --unset commit.cleanup &&\n> >+\tcat >expected <<-EOF &&\n> >+\t\tRevert \"picked\"\n> >+\n> >+\t\tThis reverts commit objid.\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git revert --cleanup=scissors picked &&\n> >+\n> >+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n> >+\ttest_i18ncmp expected actual\n> >+'\n> >+\n> > test_expect_success 'failed cherry-pick does not forget -s' '\n> > \tpristine_detach initial &&\n> > \ttest_must_fail git cherry-pick -s picked &&\n> >\n"},{"id":"370848","messageId":"20190307001607.GA25324@dev-l","threadId":"50667","inReplyTo":"1fa469d7-76ef-36b5-9688-43853fa2b2ee@gmail.com","subject":"Re: [PATCH 2/2] cherry-pick/revert: add scissors line on merge conflict","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T00:16:07Z","receivedAt":"2019-03-07T00:16:11Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Phillip, sorry I forgot to address one of your replies.\n\nOn Wed, Mar 06, 2019 at 04:29:20PM +0000, Phillip Wood wrote:\n> Hi Denton\n> \n> On 06/03/2019 10:30, Denton Liu wrote:\n> >Fix a bug where the scissors line is placed after the Conflicts:\n> >section, in the case where a merge conflict occurs and\n> >commit.cleanup = scissors.\n> \n> Was that the case with cherry-pick and revert as well as merge, or were they\n> missing the scissors line entirely as the subject line suggests?\n\nThe bug is that git-{cherry-pick,revert,merge} all place the conflicts\nhint into MERGE_MSG, which then gets read by git-commit which will place\na scissors line under all that stuff.\n\nThe fix is to make git-{cherry-pick,revert,merge} put the scissors line\nbefore the conflicts hint, so the scissors are actually placed in that\ncommand instead of in git-commit. Is the commit message still unclear or\ndoes it make sense now?\n\nThanks,\n\nDenton\n"},{"id":"370867","messageId":"cover.1551940635.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551867827.git.liu.denton@gmail.com","subject":"[PATCH v2 0/3] cherry-pick/revert cleanup scissors fix","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T06:44:21Z","receivedAt":"2019-03-07T06:44:26Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"As I stated earlier[1], I discovered today that git revert (and thus,\ngit cherry-pick as well) do not place the scissors line properly as\nwell. This patchset adds a scissors line when conflicts arise in both\ncherry-pick and revert, which fixes the same bug present in git-merge\nearlier.\n\nThis patchset should apply on top of dl/merge-cleanup-scissors-fix.\n\nChanges since v1:\n\n* Address Phillip's concern of calling get_cleanup_mode with use_editor = 1;\n only set to 1 if we are calling for revert or cherry-pick, else 0 to maintain\n original behaviour (for rebasing).\n\n* Do not die if provided an invalid cleanup option, instead just warn and\n fallback to default option.\n\n[1]: https://public-inbox.org/git/20190306014143.GA2580@dev-l/\n\n\nDenton Liu (3):\n t3507: cleanup space after redirection operators\n cherry-pick/revert: add scissors line on merge conflict\n sequencer.c: don't die on invalid cleanup_arg\n\n Documentation/git-cherry-pick.txt | 7 ++\n Documentation/git-revert.txt | 7 ++\n builtin/merge.c | 13 +---\n builtin/rebase--helper.c | 2 +-\n builtin/revert.c | 5 ++\n sequencer.c | 39 +++++-----\n sequencer.h | 3 +-\n t/t3507-cherry-pick-conflict.sh | 122 +++++++++++++++++++++++++-----\n 8 files changed, 149 insertions(+), 49 deletions(-)\n\nRange-diff against v1:\n1: 70a508ca0b ! 1: 8fdc5bfb15 cherry-pick/revert: add scissors line on merge conflict\n @@ -8,11 +8,11 @@\n \n Note that the removal of the if-else tower in git_sequencer_config may\n appear to be a no-op refactor but it actually isn't. First of all, we\n - now accept \"default\" as a configuration option and also we die on an\n - invalid cleanup mode. Most importantly, though, if\n + now accept \"default\". More importantly, though, if\n commit.cleanup = scissors, the cleanup enum will be set to\n - COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE. This\n - allows us to append scissors to MERGE_MSG in the case of a conflict.\n + COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE when we\n + are reverting or cherry-picking. This allows us to append scissors to\n + MERGE_MSG in the case of a conflict.\n \n Signed-off-by: Denton Liu \n \n @@ -77,6 +77,22 @@\n \tstrbuf_release(&msgbuf);\n \tfclose(fp);\n \n + diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n + --- a/builtin/rebase--helper.c\n + +++ b/builtin/rebase--helper.c\n +@@\n + \t\tOPT_END()\n + \t};\n + \n ++\topts.action = REPLAY_INTERACTIVE_REBASE;\n + \tsequencer_init_config(&opts);\n + \tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n + \n +-\topts.action = REPLAY_INTERACTIVE_REBASE;\n + \topts.allow_ff = 1;\n + \topts.allow_empty = 1;\n + \n +\n diff --git a/builtin/revert.c b/builtin/revert.c\n --- a/builtin/revert.c\n +++ b/builtin/revert.c\n @@ -108,6 +124,18 @@\n diff --git a/sequencer.c b/sequencer.c\n --- a/sequencer.c\n +++ b/sequencer.c\n +@@\n + static GIT_PATH_FUNC(rebase_path_strategy_opts, \"rebase-merge/strategy_opts\")\n + static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, \"rebase-merge/allow_rerere_autoupdate\")\n + \n ++static inline int is_rebase_i(const struct replay_opts *opts)\n ++{\n ++\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n ++}\n ++\n + static int git_sequencer_config(const char *k, const char *v, void *cb)\n + {\n + \tstruct replay_opts *opts = cb;\n @@\n \t\tif (status)\n \t\t\treturn status;\n @@ -123,10 +151,22 @@\n -\t\telse\n -\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n -\t\t\t\t s);\n -+\t\topts->default_msg_cleanup = get_cleanup_mode(s, 1);\n ++\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n \n \t\tfree((char *)s);\n \t\treturn status;\n +@@\n + \tgit_config(git_sequencer_config, opts);\n + }\n + \n +-static inline int is_rebase_i(const struct replay_opts *opts)\n +-{\n +-\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n +-}\n +-\n + static const char *get_dir(const struct replay_opts *opts)\n + {\n + \tif (is_rebase_i(opts))\n @@\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n }\n-: ---------- > 2: f3af8000ae sequencer.c: don't die on invalid cleanup_arg\n-- \n2.21.0.368.gbf248417d7\n\n"},{"id":"370868","messageId":"4eee16cc5a841ac20c9ffcd010c0e1feb88059bf.1551940635.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551940635.git.liu.denton@gmail.com","subject":"[PATCH v2 1/3] t3507: cleanup space after redirection operators","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T06:44:23Z","receivedAt":"2019-03-07T06:44:28Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n t/t3507-cherry-pick-conflict.sh | 34 ++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 0db166152a..74ff925526 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -88,7 +88,7 @@ test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' '\n \n test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git cherry-pick base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n@@ -96,7 +96,7 @@ test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '\n test_expect_success \\\n \t'cherry-pick --strategy=resolve w/dirty tree does not set CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git cherry-pick --strategy=resolve base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n@@ -175,23 +175,23 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n \t\tgit ls-files --stage foo &&\n \t\tgit checkout picked -- foo &&\n \t\tgit ls-files --stage foo\n-\t} > stages &&\n+\t} >stages &&\n \tsed \"\n \t\t1 s/ 0\t/ 1\t/\n \t\t2 s/ 0\t/ 2\t/\n \t\t3 s/ 0\t/ 3\t/\n-\t\" < stages > expected &&\n+\t\" expected &&\n \tgit read-tree -u --reset HEAD &&\n \n \ttest_must_fail git cherry-pick picked &&\n-\tgit ls-files --stage --unmerged > actual &&\n+\tgit ls-files --stage --unmerged >actual &&\n \n \ttest_cmp expected actual\n '\n \n test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \tpristine_detach initial &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t=======\n@@ -201,14 +201,14 @@ test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \n \ttest_must_fail git cherry-pick picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'diff3 -m style' '\n \tpristine_detach initial &&\n \tgit config merge.conflictstyle diff3 &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t||||||| parent of objid picked\n@@ -220,14 +220,14 @@ test_expect_success 'diff3 -m style' '\n \n \ttest_must_fail git cherry-pick picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'revert also handles conflicts sanely' '\n \tgit config --unset merge.conflictstyle &&\n \tpristine_detach initial &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t=======\n@@ -241,24 +241,24 @@ test_expect_success 'revert also handles conflicts sanely' '\n \t\tgit ls-files --stage foo &&\n \t\tgit checkout base -- foo &&\n \t\tgit ls-files --stage foo\n-\t} > stages &&\n+\t} >stages &&\n \tsed \"\n \t\t1 s/ 0\t/ 1\t/\n \t\t2 s/ 0\t/ 2\t/\n \t\t3 s/ 0\t/ 3\t/\n-\t\" < stages > expected-stages &&\n+\t\" expected-stages &&\n \tgit read-tree -u --reset HEAD &&\n \n \thead=$(git rev-parse HEAD) &&\n \ttest_must_fail git revert picked &&\n \tnewhead=$(git rev-parse HEAD) &&\n-\tgit ls-files --stage --unmerged > actual-stages &&\n+\tgit ls-files --stage --unmerged >actual-stages &&\n \n \ttest \"$head\" = \"$newhead\" &&\n \ttest_must_fail git update-index --refresh -q &&\n \ttest_must_fail git diff-index --exit-code HEAD &&\n \ttest_cmp expected-stages actual-stages &&\n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n@@ -284,7 +284,7 @@ test_expect_success 'revert --no-commit sets REVERT_HEAD' '\n \n test_expect_success 'revert w/dirty tree does not set REVERT_HEAD' '\n \tpristine_detach base &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git revert base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n \ttest_must_fail git rev-parse --verify REVERT_HEAD\n@@ -319,7 +319,7 @@ test_expect_success 'failed commit does not clear REVERT_HEAD' '\n test_expect_success 'revert conflict, diff3 -m style' '\n \tpristine_detach initial &&\n \tgit config merge.conflictstyle diff3 &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t||||||| objid picked\n@@ -331,7 +331,7 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \n \ttest_must_fail git revert picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n-- \n2.21.0.368.gbf248417d7\n\n"},{"id":"370869","messageId":"8fdc5bfb151e4224eba01021c6f0be3e61e1f535.1551940635.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551940635.git.liu.denton@gmail.com","subject":"[PATCH v2 2/3] cherry-pick/revert: add scissors line on merge conflict","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T06:44:26Z","receivedAt":"2019-03-07T06:44:31Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Fix a bug where the scissors line is placed after the Conflicts:\nsection, in the case where a merge conflict occurs and\ncommit.cleanup = scissors.\n\nNote that the removal of the if-else tower in git_sequencer_config may\nappear to be a no-op refactor but it actually isn't. First of all, we\nnow accept \"default\". More importantly, though, if\ncommit.cleanup = scissors, the cleanup enum will be set to\nCOMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE when we\nare reverting or cherry-picking. This allows us to append scissors to\nMERGE_MSG in the case of a conflict.\n\nSigned-off-by: Denton Liu \n---\n Documentation/git-cherry-pick.txt | 7 +++\n Documentation/git-revert.txt | 7 +++\n builtin/merge.c | 13 ++---\n builtin/rebase--helper.c | 2 +-\n builtin/revert.c | 5 ++\n sequencer.c | 32 +++++------\n sequencer.h | 3 +-\n t/t3507-cherry-pick-conflict.sh | 88 +++++++++++++++++++++++++++++++\n 8 files changed, 127 insertions(+), 30 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex d35d771fc8..5c086d78c8 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -57,6 +57,13 @@ OPTIONS\n \tWith this option, 'git cherry-pick' will let you edit the commit\n \tmessage prior to committing.\n \n+--cleanup=::\n+\tThis option determines how the commit message will be cleaned up before\n+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n+\taddition, if the '' is given a value of `scissors`, scissors will\n+\tbe prepended to MERGE_MSG before being passed on in the case of a\n+\tconflict.\n+\n -x::\n \tWhen recording the commit, append a line that says\n \t\"(cherry picked from commit ...)\" to the original commit\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex 837707a8fd..1894010e60 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -66,6 +66,13 @@ more details.\n \tWith this option, 'git revert' will not start the commit\n \tmessage editor.\n \n+--cleanup=::\n+\tThis option determines how the commit message will be cleaned up before\n+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n+\taddition, if the '' is given a value of `scissors`, scissors will\n+\tbe prepended to MERGE_MSG before being passed on in the case of a\n+\tconflict.\n+\n -n::\n --no-commit::\n \tUsually the command automatically creates some commits with\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 92efc3d8fa..d4217ebcf5 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -913,17 +913,10 @@ static int suggest_conflicts(void)\n \t * We can't use cleanup_mode because if we're not using the editor,\n \t * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even\n \t * though the message is meant to be processed later by git-commit.\n-\t * Thus, we will get the cleanup mode is returned we _are_ using an\n-\t * editor.\n+\t * Thus, we will get the cleanup mode which is returned when we _are_ using\n+\t * an editor.\n \t */\n-\tif (get_cleanup_mode(cleanup_arg, 1) == COMMIT_MSG_CLEANUP_SCISSORS) {\n-\t fputc('\\n', fp);\n-\t wt_status_add_cut_line(fp);\n-\t /* comments out the newline from append_conflicts_hint */\n-\t fputc(comment_line_char, fp);\n-\t}\n-\n-\tappend_conflicts_hint(&msgbuf);\n+\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n \tfputs(msgbuf.buf, fp);\n \tstrbuf_release(&msgbuf);\n \tfclose(fp);\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex f7c2a5fdc8..5f1bc9d383 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -48,10 +48,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n+\topts.action = REPLAY_INTERACTIVE_REBASE;\n \tsequencer_init_config(&opts);\n \tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n \n-\topts.action = REPLAY_INTERACTIVE_REBASE;\n \topts.allow_ff = 1;\n \topts.allow_empty = 1;\n \ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 9a66720cfc..fe18036be7 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n {\n \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n \tconst char *me = action_name(opts);\n+\tconst char *cleanup_arg = NULL;\n \tint cmd = 0;\n \tstruct option base_options[] = {\n \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n+\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \tif (opts->keep_redundant_commits)\n \t\topts->allow_empty = 1;\n \n+\tif (cleanup_arg)\n+\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n+\n \t/* Check for incompatible command line arguments */\n \tif (cmd) {\n \t\tchar *this_operation;\ndiff --git a/sequencer.c b/sequencer.c\nindex 707e72fb39..5c04bae7ac 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -153,6 +153,11 @@ static GIT_PATH_FUNC(rebase_path_strategy, \"rebase-merge/strategy\")\n static GIT_PATH_FUNC(rebase_path_strategy_opts, \"rebase-merge/strategy_opts\")\n static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, \"rebase-merge/allow_rerere_autoupdate\")\n \n+static inline int is_rebase_i(const struct replay_opts *opts)\n+{\n+\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n+}\n+\n static int git_sequencer_config(const char *k, const char *v, void *cb)\n {\n \tstruct replay_opts *opts = cb;\n@@ -165,17 +170,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n \t\tif (status)\n \t\t\treturn status;\n \n-\t\tif (!strcmp(s, \"verbatim\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n-\t\telse if (!strcmp(s, \"whitespace\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n-\t\telse if (!strcmp(s, \"strip\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n-\t\telse if (!strcmp(s, \"scissors\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n-\t\telse\n-\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n-\t\t\t\t s);\n+\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n \n \t\tfree((char *)s);\n \t\treturn status;\n@@ -199,11 +194,6 @@ void sequencer_init_config(struct replay_opts *opts)\n \tgit_config(git_sequencer_config, opts);\n }\n \n-static inline int is_rebase_i(const struct replay_opts *opts)\n-{\n-\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n-}\n-\n static const char *get_dir(const struct replay_opts *opts)\n {\n \tif (is_rebase_i(opts))\n@@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n }\n \n-void append_conflicts_hint(struct strbuf *msgbuf)\n+void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n {\n \tint i;\n \n+\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n+\t\tstrbuf_addch(msgbuf, '\\n');\n+\t\twt_status_append_cut_line(msgbuf);\n+\t\tstrbuf_addch(msgbuf, comment_line_char);\n+\t}\n+\n \tstrbuf_addch(msgbuf, '\\n');\n \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n \tfor (i = 0; i < active_nr;) {\n@@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \t\t\t_(action_name(opts)));\n \n \tif (!clean)\n-\t\tappend_conflicts_hint(msgbuf);\n+\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n \n \treturn !clean;\n }\ndiff --git a/sequencer.h b/sequencer.h\nindex 5690e0c27e..aa99503dd7 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -91,7 +91,8 @@ int rearrange_squash(void);\n extern const char sign_off_header[];\n \n void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n-void append_conflicts_hint(struct strbuf *msgbuf);\n+void append_conflicts_hint(struct strbuf *msgbuf,\n+\t\tenum commit_msg_cleanup_mode cleanup_mode);\n enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \tint use_editor);\n \ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 74ff925526..c94e44dad0 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -189,6 +189,48 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success \\\n+\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config commit.cleanup scissors &&\n+\tcat <<-EOF >expected &&\n+\t\tpicked\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git cherry-pick picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n+test_expect_success \\\n+\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config --unset commit.cleanup &&\n+\tcat <<-EOF >expected &&\n+\t\tpicked\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \tpristine_detach initial &&\n \tcat <<-EOF >expected &&\n@@ -335,6 +377,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success \\\n+\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config commit.cleanup scissors &&\n+\tcat >expected <<-EOF &&\n+\t\tRevert \"picked\"\n+\n+\t\tThis reverts commit objid.\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git revert picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n+test_expect_success \\\n+\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config --unset commit.cleanup &&\n+\tcat >expected <<-EOF &&\n+\t\tRevert \"picked\"\n+\n+\t\tThis reverts commit objid.\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git revert --cleanup=scissors picked &&\n+\n+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'failed cherry-pick does not forget -s' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked &&\n-- \n2.21.0.368.gbf248417d7\n\n"},{"id":"370870","messageId":"f3af8000aec51e49f3fe8f90dbfec61606596b2e.1551940635.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551940635.git.liu.denton@gmail.com","subject":"[PATCH v2 3/3] sequencer.c: don't die on invalid cleanup_arg","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T06:44:28Z","receivedAt":"2019-03-07T06:44:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When get_cleanup_mode was provided with an invalid cleanup_arg, it used\nto die. Warn user and fallback to default behaviour if an invalid\ncleanup_arg is given.\n\nSigned-off-by: Denton Liu \n---\n sequencer.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5c04bae7ac..f9bdfa90ad 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -502,8 +502,11 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \telse if (!strcmp(cleanup_arg, \"scissors\"))\n \t\treturn use_editor ? COMMIT_MSG_CLEANUP_SCISSORS :\n \t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n-\telse\n-\t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n+\telse {\n+\t\twarning(_(\"Invalid cleanup mode %s, falling back to default\"), cleanup_arg);\n+\t\treturn use_editor ? COMMIT_MSG_CLEANUP_ALL :\n+\t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n+\t}\n }\n \n void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n-- \n2.21.0.368.gbf248417d7\n\n"},{"id":"370872","messageId":"xmqqh8cfm92k.fsf@gitster-ct.c.googlers.com","threadId":"50667","inReplyTo":"4eee16cc5a841ac20c9ffcd010c0e1feb88059bf.1551940635.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 1/3] t3507: cleanup space after redirection operators","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-07T07:34:43Z","receivedAt":"2019-03-07T07:34:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> Signed-off-by: Denton Liu \n> ---\n> t/t3507-cherry-pick-conflict.sh | 34 ++++++++++++++++-----------------\n> 1 file changed, 17 insertions(+), 17 deletions(-)\n\nA good preliminary clean-up. Looks good.\n"},{"id":"370875","messageId":"xmqqa7i7m886.fsf@gitster-ct.c.googlers.com","threadId":"50667","inReplyTo":"8fdc5bfb151e4224eba01021c6f0be3e61e1f535.1551940635.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 2/3] cherry-pick/revert: add scissors line on merge conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-07T07:52:57Z","receivedAt":"2019-03-07T07:53:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> Fix a bug where the scissors line is placed after the Conflicts:\n> section, in the case where a merge conflict occurs and\n> commit.cleanup = scissors.\n>\n> Note that the removal of the if-else tower in git_sequencer_config may\n> appear to be a no-op refactor but it actually isn't. First of all, we\n> now accept \"default\". More importantly, though, if\n> commit.cleanup = scissors, the cleanup enum will be set to\n> COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE when we\n> are reverting or cherry-picking. This allows us to append scissors to\n> MERGE_MSG in the case of a conflict.\n\nGood thing to point out in the log message.\n\n> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> index d35d771fc8..5c086d78c8 100644\n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index 837707a8fd..1894010e60 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -66,6 +66,13 @@ more details.\n> \tWith this option, 'git revert' will not start the commit\n> \tmessage editor.\n> \n> +--cleanup=::\n> +\tThis option determines how the commit message will be cleaned up before\n> +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> +\taddition, if the '' is given a value of `scissors`, scissors will\n> +\tbe prepended to MERGE_MSG before being passed on in the case of a\n> +\tconflict.\n\nThese both say \"prepended\", but shouldn't the code\n\n - add the merge message proper, with the expectation that it would\n be used more or less intact in the final commit message; then\n\n - add the scissors; then\n\n - append informative cruft that would be removed, only to help\n human users.\n\nin this order? I'd expect that most people would consider that the\nprimary payload of MERGE_MSG file is the part that would become the\ncommit message, so I would have expected the second step would be\ndescribed as \"appended to\".\n\nAnother thing I notice is that this singles out \"scissors\" mode;\ndoesn't the code do anything worth describing with other clean-up\nmodes?\n\n> +\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n\nUse $_x40 (or $OID_REGEX) to make it more readable and also\nfuture-proof?\n\n"},{"id":"370876","messageId":"xmqq5zsvm7uc.fsf@gitster-ct.c.googlers.com","threadId":"50667","inReplyTo":"f3af8000aec51e49f3fe8f90dbfec61606596b2e.1551940635.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 3/3] sequencer.c: don't die on invalid cleanup_arg","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-07T08:01:15Z","receivedAt":"2019-03-07T08:01:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> When get_cleanup_mode was provided with an invalid cleanup_arg, it used\n> to die. Warn user and fallback to default behaviour if an invalid\n> cleanup_arg is given.\n>\n> Signed-off-by: Denton Liu \n> ---\n> sequencer.c | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 5c04bae7ac..f9bdfa90ad 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -502,8 +502,11 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> \telse if (!strcmp(cleanup_arg, \"scissors\"))\n> \t\treturn use_editor ? COMMIT_MSG_CLEANUP_SCISSORS :\n> \t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n> -\telse\n> -\t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> +\telse {\n> +\t\twarning(_(\"Invalid cleanup mode %s, falling back to default\"), cleanup_arg);\n> +\t\treturn use_editor ? COMMIT_MSG_CLEANUP_ALL :\n> +\t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n> +\t}\n> }\n\nIn what different contexts does this get called, I wonder?\n\nI think\n\n\t$ git cherry-pick --cleanup=bogus ...\n\nshould error out, instead of falling back to anything else, while\nhaving\n\n\t[commit] cleanup = bogus\n\nin .git/config should *not* say anything if you are not running a\ncommand that does not get affected by the commit.cleanup variable,\nand with such a bogus configuration, a command that does use the\nvariable should either also die, or fallback with a warning.\n\nI have a suspicion that the change is breaking the error detection\ndone for the command line argument parsing, and if that is the case,\nthen it is a bad idea. But I may have misread the code.\n\nThanks.\n\n\n\n\n\n\n"},{"id":"370883","messageId":"20190307091903.GA7970@archbookpro.localdomain","threadId":"50667","inReplyTo":"xmqqa7i7m886.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/3] cherry-pick/revert: add scissors line on merge conflict","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T09:19:03Z","receivedAt":"2019-03-07T09:19:10Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Junio,\n\nOn Thu, Mar 07, 2019 at 04:52:57PM +0900, Junio C Hamano wrote:\n> Denton Liu writes:\n> > diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> > index d35d771fc8..5c086d78c8 100644\n> > diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> > index 837707a8fd..1894010e60 100644\n> > --- a/Documentation/git-revert.txt\n> > +++ b/Documentation/git-revert.txt\n> > @@ -66,6 +66,13 @@ more details.\n> > \tWith this option, 'git revert' will not start the commit\n> > \tmessage editor.\n> > \n> > +--cleanup=::\n> > +\tThis option determines how the commit message will be cleaned up before\n> > +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> > +\taddition, if the '' is given a value of `scissors`, scissors will\n> > +\tbe prepended to MERGE_MSG before being passed on in the case of a\n> > +\tconflict.\n> \n> These both say \"prepended\", but shouldn't the code\n> \n> - add the merge message proper, with the expectation that it would\n> be used more or less intact in the final commit message; then\n> \n> - add the scissors; then\n> \n> - append informative cruft that would be removed, only to help\n> human users.\n> \n> in this order? I'd expect that most people would consider that the\n> primary payload of MERGE_MSG file is the part that would become the\n> commit message, so I would have expected the second step would be\n> described as \"appended to\".\n\nI copied this over from 6f06b6aeef (merge: add scissors line on merge\nconflict, 2019-01-22). I guess I'll have to fix it up in git-merge too.\n\n> \n> Another thing I notice is that this singles out \"scissors\" mode;\n> doesn't the code do anything worth describing with other clean-up\n> modes?\n\nThe revert/cherry-pick code doesn't really do anything with the cleanup\nmode. It's only used to determine whether to generate the scissors line\nin the case of a conflict.\n"},{"id":"370888","messageId":"cover.1551951770.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551940635.git.liu.denton@gmail.com","subject":"[PATCH v3 0/4] cherry-pick/revert cleanup scissors fix","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T09:58:24Z","receivedAt":"2019-03-07T09:58:31Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"As I stated earlier[1], I discovered today that git revert (and thus,\ngit cherry-pick as well) do not place the scissors line properly as\nwell. This patchset adds a scissors line when conflicts arise in both\ncherry-pick and revert, which fixes the same bug present in git-merge\nearlier.\n\nThis patchset should apply on top of dl/merge-cleanup-scissors-fix.\n\nChanges since v1:\n\n* Address Phillip's concern of calling get_cleanup_mode with use_editor = 1;\n only set to 1 if we are calling for revert or cherry-pick, else 0 to maintain\n original behaviour (for rebasing).\n\n* Do not die if provided an invalid cleanup option, instead just warn and\n fallback to default option.\n\nChanges since v2:\n\n* s/prepended/appended/\n\n* Use $OID_REGEX instead of self-made regex\n\n* Don't die if bad config option is given, otherwise die if a bad\n command-line argument is given\n\n[1]: https://public-inbox.org/git/20190306014143.GA2580@dev-l/\n\n\nDenton Liu (4):\n merge-options.txt: correct typo\n t3507: cleanup space after redirection operators\n cherry-pick/revert: add scissors line on merge conflict\n sequencer.c: don't die on invalid cleanup_arg\n\n Documentation/git-cherry-pick.txt | 7 ++\n Documentation/git-revert.txt | 7 ++\n Documentation/merge-options.txt | 4 +-\n builtin/commit.c | 2 +-\n builtin/merge.c | 15 +---\n builtin/rebase--helper.c | 2 +-\n builtin/revert.c | 5 ++\n sequencer.c | 40 +++++-----\n sequencer.h | 5 +-\n t/t3507-cherry-pick-conflict.sh | 120 +++++++++++++++++++++++++-----\n 10 files changed, 153 insertions(+), 54 deletions(-)\n\nRange-diff against v2:\n-: ---------- > 1: 1076d4c451 merge-options.txt: correct typo\n1: 4eee16cc5a = 2: 6bce4d4722 t3507: cleanup space after redirection operators\n2: 8fdc5bfb15 ! 3: 14672ce10c cherry-pick/revert: add scissors line on merge conflict\n @@ -27,7 +27,7 @@\n +\tThis option determines how the commit message will be cleaned up before\n +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n +\taddition, if the '' is given a value of `scissors`, scissors will\n -+\tbe prepended to MERGE_MSG before being passed on in the case of a\n ++\tbe appended to MERGE_MSG before being passed on in the case of a\n +\tconflict.\n +\n -x::\n @@ -45,7 +45,7 @@\n +\tThis option determines how the commit message will be cleaned up before\n +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n +\taddition, if the '' is given a value of `scissors`, scissors will\n -+\tbe prepended to MERGE_MSG before being passed on in the case of a\n ++\tbe appended to MERGE_MSG before being passed on in the case of a\n +\tconflict.\n +\n -n::\n @@ -233,8 +233,7 @@\n +\n +\ttest_must_fail git cherry-pick picked &&\n +\n -+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n -+\ttest_i18ncmp expected actual\n ++\ttest_i18ncmp expected .git/MERGE_MSG\n +'\n +\n +test_expect_success \\\n @@ -254,8 +253,7 @@\n +\n +\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n +\n -+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n -+\ttest_i18ncmp expected actual\n ++\ttest_i18ncmp expected .git/MERGE_MSG\n +'\n +\n test_expect_success 'failed cherry-pick describes conflict in work tree' '\n @@ -272,7 +270,7 @@\n +\tcat >expected <<-EOF &&\n +\t\tRevert \"picked\"\n +\n -+\t\tThis reverts commit objid.\n ++\t\tThis reverts commit OBJID.\n +\n +\t\t# ------------------------ >8 ------------------------\n +\t\t# Do not modify or remove the line above.\n @@ -284,7 +282,7 @@\n +\n +\ttest_must_fail git revert picked &&\n +\n -+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n ++\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n +\ttest_i18ncmp expected actual\n +'\n +\n @@ -295,7 +293,7 @@\n +\tcat >expected <<-EOF &&\n +\t\tRevert \"picked\"\n +\n -+\t\tThis reverts commit objid.\n ++\t\tThis reverts commit OBJID.\n +\n +\t\t# ------------------------ >8 ------------------------\n +\t\t# Do not modify or remove the line above.\n @@ -307,7 +305,7 @@\n +\n +\ttest_must_fail git revert --cleanup=scissors picked &&\n +\n -+\tsed \"s/[a-f0-9]\\{40\\}/objid/\" .git/MERGE_MSG >actual &&\n ++\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n +\ttest_i18ncmp expected actual\n +'\n +\n3: f3af8000ae < -: ---------- sequencer.c: don't die on invalid cleanup_arg\n-: ---------- > 4: 68ec2b7cd7 sequencer.c: don't die on invalid cleanup_arg\n-- \n2.21.0.370.g4fdb13b891\n\n"},{"id":"370889","messageId":"1076d4c45108b1edad48fcac0f203c8c4e521b79.1551951770.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551951770.git.liu.denton@gmail.com","subject":"[PATCH v3 1/4] merge-options.txt: correct typo","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T09:58:27Z","receivedAt":"2019-03-07T09:58:34Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The scissors line is included after the human commit message. Therefore,\nsaying \"scissors will be prepended\" was incorrect. Update to say\n\"appended\" which is more correct.\n\nSigned-off-by: Denton Liu \n---\n\nNotes:\n Note that this can be squashed into 6f06b6aeef (merge: add scissors line on\n merge conflict, 2019-01-22) if it makes life easier.\n\n Documentation/merge-options.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex c2a263ba74..6fff647409 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -31,8 +31,8 @@ set to `no` at the beginning of them.\n \tThis option determines how the merge message will be cleaned up\n \tbefore commiting or being passed on. See linkgit:git-commit[1] for more\n \tdetails. In addition, if the '' is given a value of `scissors`,\n-\tscissors will be prepended to MERGE_MSG before being passed on in the case\n-\tof a merge conflict.\n+\tscissors will be appended to MERGE_MSG before being passed on in the\n+\tcase of a merge conflict.\n \n --ff::\n \tWhen the merge resolves as a fast-forward, only update the branch\n-- \n2.21.0.370.g4fdb13b891\n\n"},{"id":"370890","messageId":"6bce4d4722dd9408c4146daefc21c7e3443748c9.1551951770.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551951770.git.liu.denton@gmail.com","subject":"[PATCH v3 2/4] t3507: cleanup space after redirection operators","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T09:58:30Z","receivedAt":"2019-03-07T09:58:35Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n t/t3507-cherry-pick-conflict.sh | 34 ++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 0db166152a..74ff925526 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -88,7 +88,7 @@ test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' '\n \n test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git cherry-pick base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n@@ -96,7 +96,7 @@ test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '\n test_expect_success \\\n \t'cherry-pick --strategy=resolve w/dirty tree does not set CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git cherry-pick --strategy=resolve base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n@@ -175,23 +175,23 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n \t\tgit ls-files --stage foo &&\n \t\tgit checkout picked -- foo &&\n \t\tgit ls-files --stage foo\n-\t} > stages &&\n+\t} >stages &&\n \tsed \"\n \t\t1 s/ 0\t/ 1\t/\n \t\t2 s/ 0\t/ 2\t/\n \t\t3 s/ 0\t/ 3\t/\n-\t\" < stages > expected &&\n+\t\" expected &&\n \tgit read-tree -u --reset HEAD &&\n \n \ttest_must_fail git cherry-pick picked &&\n-\tgit ls-files --stage --unmerged > actual &&\n+\tgit ls-files --stage --unmerged >actual &&\n \n \ttest_cmp expected actual\n '\n \n test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \tpristine_detach initial &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t=======\n@@ -201,14 +201,14 @@ test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \n \ttest_must_fail git cherry-pick picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'diff3 -m style' '\n \tpristine_detach initial &&\n \tgit config merge.conflictstyle diff3 &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t||||||| parent of objid picked\n@@ -220,14 +220,14 @@ test_expect_success 'diff3 -m style' '\n \n \ttest_must_fail git cherry-pick picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'revert also handles conflicts sanely' '\n \tgit config --unset merge.conflictstyle &&\n \tpristine_detach initial &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t=======\n@@ -241,24 +241,24 @@ test_expect_success 'revert also handles conflicts sanely' '\n \t\tgit ls-files --stage foo &&\n \t\tgit checkout base -- foo &&\n \t\tgit ls-files --stage foo\n-\t} > stages &&\n+\t} >stages &&\n \tsed \"\n \t\t1 s/ 0\t/ 1\t/\n \t\t2 s/ 0\t/ 2\t/\n \t\t3 s/ 0\t/ 3\t/\n-\t\" < stages > expected-stages &&\n+\t\" expected-stages &&\n \tgit read-tree -u --reset HEAD &&\n \n \thead=$(git rev-parse HEAD) &&\n \ttest_must_fail git revert picked &&\n \tnewhead=$(git rev-parse HEAD) &&\n-\tgit ls-files --stage --unmerged > actual-stages &&\n+\tgit ls-files --stage --unmerged >actual-stages &&\n \n \ttest \"$head\" = \"$newhead\" &&\n \ttest_must_fail git update-index --refresh -q &&\n \ttest_must_fail git diff-index --exit-code HEAD &&\n \ttest_cmp expected-stages actual-stages &&\n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n@@ -284,7 +284,7 @@ test_expect_success 'revert --no-commit sets REVERT_HEAD' '\n \n test_expect_success 'revert w/dirty tree does not set REVERT_HEAD' '\n \tpristine_detach base &&\n-\techo foo > foo &&\n+\techo foo >foo &&\n \ttest_must_fail git revert base &&\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n \ttest_must_fail git rev-parse --verify REVERT_HEAD\n@@ -319,7 +319,7 @@ test_expect_success 'failed commit does not clear REVERT_HEAD' '\n test_expect_success 'revert conflict, diff3 -m style' '\n \tpristine_detach initial &&\n \tgit config merge.conflictstyle diff3 &&\n-\tcat <<-EOF > expected &&\n+\tcat <<-EOF >expected &&\n \t<<<<<<< HEAD\n \ta\n \t||||||| objid picked\n@@ -331,7 +331,7 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \n \ttest_must_fail git revert picked &&\n \n-\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo > actual &&\n+\tsed \"s/[a-f0-9]*\\.\\.\\./objid/\" foo >actual &&\n \ttest_cmp expected actual\n '\n \n-- \n2.21.0.370.g4fdb13b891\n\n"},{"id":"370891","messageId":"14672ce10c90c46b15b62c48d2eea6fe8d85ed86.1551951770.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551951770.git.liu.denton@gmail.com","subject":"[PATCH v3 3/4] cherry-pick/revert: add scissors line on merge conflict","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T09:58:32Z","receivedAt":"2019-03-07T09:58:38Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Fix a bug where the scissors line is placed after the Conflicts:\nsection, in the case where a merge conflict occurs and\ncommit.cleanup = scissors.\n\nNote that the removal of the if-else tower in git_sequencer_config may\nappear to be a no-op refactor but it actually isn't. First of all, we\nnow accept \"default\". More importantly, though, if\ncommit.cleanup = scissors, the cleanup enum will be set to\nCOMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE when we\nare reverting or cherry-picking. This allows us to append scissors to\nMERGE_MSG in the case of a conflict.\n\nSigned-off-by: Denton Liu \n---\n Documentation/git-cherry-pick.txt | 7 +++\n Documentation/git-revert.txt | 7 +++\n builtin/merge.c | 13 ++---\n builtin/rebase--helper.c | 2 +-\n builtin/revert.c | 5 ++\n sequencer.c | 32 +++++-------\n sequencer.h | 3 +-\n t/t3507-cherry-pick-conflict.sh | 86 +++++++++++++++++++++++++++++++\n 8 files changed, 125 insertions(+), 30 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex d35d771fc8..92235bd1c4 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -57,6 +57,13 @@ OPTIONS\n \tWith this option, 'git cherry-pick' will let you edit the commit\n \tmessage prior to committing.\n \n+--cleanup=::\n+\tThis option determines how the commit message will be cleaned up before\n+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n+\taddition, if the '' is given a value of `scissors`, scissors will\n+\tbe appended to MERGE_MSG before being passed on in the case of a\n+\tconflict.\n+\n -x::\n \tWhen recording the commit, append a line that says\n \t\"(cherry picked from commit ...)\" to the original commit\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex 837707a8fd..bd4ad395a9 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -66,6 +66,13 @@ more details.\n \tWith this option, 'git revert' will not start the commit\n \tmessage editor.\n \n+--cleanup=::\n+\tThis option determines how the commit message will be cleaned up before\n+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n+\taddition, if the '' is given a value of `scissors`, scissors will\n+\tbe appended to MERGE_MSG before being passed on in the case of a\n+\tconflict.\n+\n -n::\n --no-commit::\n \tUsually the command automatically creates some commits with\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 92efc3d8fa..d4217ebcf5 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -913,17 +913,10 @@ static int suggest_conflicts(void)\n \t * We can't use cleanup_mode because if we're not using the editor,\n \t * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even\n \t * though the message is meant to be processed later by git-commit.\n-\t * Thus, we will get the cleanup mode is returned we _are_ using an\n-\t * editor.\n+\t * Thus, we will get the cleanup mode which is returned when we _are_ using\n+\t * an editor.\n \t */\n-\tif (get_cleanup_mode(cleanup_arg, 1) == COMMIT_MSG_CLEANUP_SCISSORS) {\n-\t fputc('\\n', fp);\n-\t wt_status_add_cut_line(fp);\n-\t /* comments out the newline from append_conflicts_hint */\n-\t fputc(comment_line_char, fp);\n-\t}\n-\n-\tappend_conflicts_hint(&msgbuf);\n+\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n \tfputs(msgbuf.buf, fp);\n \tstrbuf_release(&msgbuf);\n \tfclose(fp);\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex f7c2a5fdc8..5f1bc9d383 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -48,10 +48,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n+\topts.action = REPLAY_INTERACTIVE_REBASE;\n \tsequencer_init_config(&opts);\n \tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n \n-\topts.action = REPLAY_INTERACTIVE_REBASE;\n \topts.allow_ff = 1;\n \topts.allow_empty = 1;\n \ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 9a66720cfc..fe18036be7 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n {\n \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n \tconst char *me = action_name(opts);\n+\tconst char *cleanup_arg = NULL;\n \tint cmd = 0;\n \tstruct option base_options[] = {\n \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n+\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \tif (opts->keep_redundant_commits)\n \t\topts->allow_empty = 1;\n \n+\tif (cleanup_arg)\n+\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n+\n \t/* Check for incompatible command line arguments */\n \tif (cmd) {\n \t\tchar *this_operation;\ndiff --git a/sequencer.c b/sequencer.c\nindex 707e72fb39..5c04bae7ac 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -153,6 +153,11 @@ static GIT_PATH_FUNC(rebase_path_strategy, \"rebase-merge/strategy\")\n static GIT_PATH_FUNC(rebase_path_strategy_opts, \"rebase-merge/strategy_opts\")\n static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, \"rebase-merge/allow_rerere_autoupdate\")\n \n+static inline int is_rebase_i(const struct replay_opts *opts)\n+{\n+\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n+}\n+\n static int git_sequencer_config(const char *k, const char *v, void *cb)\n {\n \tstruct replay_opts *opts = cb;\n@@ -165,17 +170,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n \t\tif (status)\n \t\t\treturn status;\n \n-\t\tif (!strcmp(s, \"verbatim\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n-\t\telse if (!strcmp(s, \"whitespace\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n-\t\telse if (!strcmp(s, \"strip\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n-\t\telse if (!strcmp(s, \"scissors\"))\n-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n-\t\telse\n-\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n-\t\t\t\t s);\n+\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n \n \t\tfree((char *)s);\n \t\treturn status;\n@@ -199,11 +194,6 @@ void sequencer_init_config(struct replay_opts *opts)\n \tgit_config(git_sequencer_config, opts);\n }\n \n-static inline int is_rebase_i(const struct replay_opts *opts)\n-{\n-\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n-}\n-\n static const char *get_dir(const struct replay_opts *opts)\n {\n \tif (is_rebase_i(opts))\n@@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n }\n \n-void append_conflicts_hint(struct strbuf *msgbuf)\n+void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n {\n \tint i;\n \n+\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n+\t\tstrbuf_addch(msgbuf, '\\n');\n+\t\twt_status_append_cut_line(msgbuf);\n+\t\tstrbuf_addch(msgbuf, comment_line_char);\n+\t}\n+\n \tstrbuf_addch(msgbuf, '\\n');\n \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n \tfor (i = 0; i < active_nr;) {\n@@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \t\t\t_(action_name(opts)));\n \n \tif (!clean)\n-\t\tappend_conflicts_hint(msgbuf);\n+\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n \n \treturn !clean;\n }\ndiff --git a/sequencer.h b/sequencer.h\nindex 5690e0c27e..aa99503dd7 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -91,7 +91,8 @@ int rearrange_squash(void);\n extern const char sign_off_header[];\n \n void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n-void append_conflicts_hint(struct strbuf *msgbuf);\n+void append_conflicts_hint(struct strbuf *msgbuf,\n+\t\tenum commit_msg_cleanup_mode cleanup_mode);\n enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \tint use_editor);\n \ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 74ff925526..c3894ca9d6 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -189,6 +189,46 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success \\\n+\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config commit.cleanup scissors &&\n+\tcat <<-EOF >expected &&\n+\t\tpicked\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git cherry-pick picked &&\n+\n+\ttest_i18ncmp expected .git/MERGE_MSG\n+'\n+\n+test_expect_success \\\n+\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config --unset commit.cleanup &&\n+\tcat <<-EOF >expected &&\n+\t\tpicked\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n+\n+\ttest_i18ncmp expected .git/MERGE_MSG\n+'\n+\n test_expect_success 'failed cherry-pick describes conflict in work tree' '\n \tpristine_detach initial &&\n \tcat <<-EOF >expected &&\n@@ -335,6 +375,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success \\\n+\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config commit.cleanup scissors &&\n+\tcat >expected <<-EOF &&\n+\t\tRevert \"picked\"\n+\n+\t\tThis reverts commit OBJID.\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git revert picked &&\n+\n+\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n+test_expect_success \\\n+\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n+\tpristine_detach initial &&\n+\tgit config --unset commit.cleanup &&\n+\tcat >expected <<-EOF &&\n+\t\tRevert \"picked\"\n+\n+\t\tThis reverts commit OBJID.\n+\n+\t\t# ------------------------ >8 ------------------------\n+\t\t# Do not modify or remove the line above.\n+\t\t# Everything below it will be ignored.\n+\t\t#\n+\t\t# Conflicts:\n+\t\t#\tfoo\n+\t\tEOF\n+\n+\ttest_must_fail git revert --cleanup=scissors picked &&\n+\n+\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'failed cherry-pick does not forget -s' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked &&\n-- \n2.21.0.370.g4fdb13b891\n\n"},{"id":"370892","messageId":"68ec2b7cd7457e9bcd3f6f211774457c73ef8646.1551951770.git.liu.denton@gmail.com","threadId":"50667","inReplyTo":"cover.1551951770.git.liu.denton@gmail.com","subject":"[PATCH v3 4/4] sequencer.c: don't die on invalid cleanup_arg","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T09:58:35Z","receivedAt":"2019-03-07T09:58:40Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When get_cleanup_mode was provided with an invalid cleanup_arg, it used\nto die. Warn user and fallback to default behaviour if an invalid\ncleanup_arg is given.\n\nSigned-off-by: Denton Liu \n---\n builtin/commit.c | 2 +-\n builtin/merge.c | 4 ++--\n builtin/revert.c | 2 +-\n sequencer.c | 10 +++++++---\n sequencer.h | 2 +-\n 5 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 43291d79bd..0072a5817a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1167,7 +1167,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n-\tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n+\tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor, 1);\n \n \thandle_untracked_files_arg(s);\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex d4217ebcf5..3b597ec540 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -916,7 +916,7 @@ static int suggest_conflicts(void)\n \t * Thus, we will get the cleanup mode which is returned when we _are_ using\n \t * an editor.\n \t */\n-\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n+\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1, 1));\n \tfputs(msgbuf.buf, fp);\n \tstrbuf_release(&msgbuf);\n \tfclose(fp);\n@@ -1424,7 +1424,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tif (option_edit < 0)\n \t\toption_edit = default_edit_option();\n \n-\tcleanup_mode = get_cleanup_mode(cleanup_arg, 0 < option_edit);\n+\tcleanup_mode = get_cleanup_mode(cleanup_arg, 0 < option_edit, 1);\n \n \tif (!use_strategies) {\n \t\tif (!remoteheads)\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex fe18036be7..a96f2ecd8a 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -139,7 +139,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \t\topts->allow_empty = 1;\n \n \tif (cleanup_arg)\n-\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n+\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1, 1);\n \n \t/* Check for incompatible command line arguments */\n \tif (cmd) {\ndiff --git a/sequencer.c b/sequencer.c\nindex 5c04bae7ac..7d18c55223 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -170,7 +170,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n \t\tif (status)\n \t\t\treturn status;\n \n-\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n+\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts), 0);\n \n \t\tfree((char *)s);\n \t\treturn status;\n@@ -488,7 +488,7 @@ static int fast_forward_to(const struct object_id *to, const struct object_id *f\n }\n \n enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n-\tint use_editor)\n+\tint use_editor, int die_on_error)\n {\n \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n \t\treturn use_editor ? COMMIT_MSG_CLEANUP_ALL :\n@@ -502,7 +502,11 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n \telse if (!strcmp(cleanup_arg, \"scissors\"))\n \t\treturn use_editor ? COMMIT_MSG_CLEANUP_SCISSORS :\n \t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n-\telse\n+\telse if (!die_on_error) {\n+\t\twarning(_(\"Invalid cleanup mode %s, falling back to default\"), cleanup_arg);\n+\t\treturn use_editor ? COMMIT_MSG_CLEANUP_ALL :\n+\t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n+\t} else\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n }\n \ndiff --git a/sequencer.h b/sequencer.h\nindex aa99503dd7..c4c80051ea 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -94,7 +94,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n void append_conflicts_hint(struct strbuf *msgbuf,\n \t\tenum commit_msg_cleanup_mode cleanup_mode);\n enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n-\tint use_editor);\n+\tint use_editor, int die_on_error);\n \n void cleanup_message(struct strbuf *msgbuf,\n \tenum commit_msg_cleanup_mode cleanup_mode, int verbose);\n-- \n2.21.0.370.g4fdb13b891\n\n"},{"id":"370908","messageId":"556113c0-b952-522b-d7b4-233c313d158f@gmail.com","threadId":"50667","inReplyTo":"68ec2b7cd7457e9bcd3f6f211774457c73ef8646.1551951770.git.liu.denton@gmail.com","subject":"Re: [PATCH v3 4/4] sequencer.c: don't die on invalid cleanup_arg","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-03-07T15:21:00Z","receivedAt":"2019-03-07T15:21:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Denton\n\nThanks for doing this, I think it looks ok but it would be better to \nmake these changes earlier in the series so they come before what is now \npatch 3\n\nBest Wishes\n\nPhillip\n\nOn 07/03/2019 09:58, Denton Liu wrote:\n> When get_cleanup_mode was provided with an invalid cleanup_arg, it used\n> to die. Warn user and fallback to default behaviour if an invalid\n> cleanup_arg is given.\n> \n> Signed-off-by: Denton Liu \n> ---\n> builtin/commit.c | 2 +-\n> builtin/merge.c | 4 ++--\n> builtin/revert.c | 2 +-\n> sequencer.c | 10 +++++++---\n> sequencer.h | 2 +-\n> 5 files changed, 12 insertions(+), 8 deletions(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 43291d79bd..0072a5817a 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1167,7 +1167,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n> \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n> \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n> \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n> -\tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n> +\tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor, 1);\n> \n> \thandle_untracked_files_arg(s);\n> \n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index d4217ebcf5..3b597ec540 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -916,7 +916,7 @@ static int suggest_conflicts(void)\n> \t * Thus, we will get the cleanup mode which is returned when we _are_ using\n> \t * an editor.\n> \t */\n> -\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n> +\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1, 1));\n> \tfputs(msgbuf.buf, fp);\n> \tstrbuf_release(&msgbuf);\n> \tfclose(fp);\n> @@ -1424,7 +1424,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n> \tif (option_edit < 0)\n> \t\toption_edit = default_edit_option();\n> \n> -\tcleanup_mode = get_cleanup_mode(cleanup_arg, 0 < option_edit);\n> +\tcleanup_mode = get_cleanup_mode(cleanup_arg, 0 < option_edit, 1);\n> \n> \tif (!use_strategies) {\n> \t\tif (!remoteheads)\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index fe18036be7..a96f2ecd8a 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -139,7 +139,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \t\topts->allow_empty = 1;\n> \n> \tif (cleanup_arg)\n> -\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n> +\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1, 1);\n> \n> \t/* Check for incompatible command line arguments */\n> \tif (cmd) {\n> diff --git a/sequencer.c b/sequencer.c\n> index 5c04bae7ac..7d18c55223 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -170,7 +170,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n> \t\tif (status)\n> \t\t\treturn status;\n> \n> -\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n> +\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts), 0);\n> \n> \t\tfree((char *)s);\n> \t\treturn status;\n> @@ -488,7 +488,7 @@ static int fast_forward_to(const struct object_id *to, const struct object_id *f\n> }\n> \n> enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> -\tint use_editor)\n> +\tint use_editor, int die_on_error)\n> {\n> \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n> \t\treturn use_editor ? COMMIT_MSG_CLEANUP_ALL :\n> @@ -502,7 +502,11 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> \telse if (!strcmp(cleanup_arg, \"scissors\"))\n> \t\treturn use_editor ? COMMIT_MSG_CLEANUP_SCISSORS :\n> \t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n> -\telse\n> +\telse if (!die_on_error) {\n> +\t\twarning(_(\"Invalid cleanup mode %s, falling back to default\"), cleanup_arg);\n> +\t\treturn use_editor ? COMMIT_MSG_CLEANUP_ALL :\n> +\t\t\t\t COMMIT_MSG_CLEANUP_SPACE;\n> +\t} else\n> \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> }\n> \n> diff --git a/sequencer.h b/sequencer.h\n> index aa99503dd7..c4c80051ea 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -94,7 +94,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n> void append_conflicts_hint(struct strbuf *msgbuf,\n> \t\tenum commit_msg_cleanup_mode cleanup_mode);\n> enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> -\tint use_editor);\n> +\tint use_editor, int die_on_error);\n> \n> void cleanup_message(struct strbuf *msgbuf,\n> \tenum commit_msg_cleanup_mode cleanup_mode, int verbose);\n> \n"},{"id":"370909","messageId":"b568185b-2411-7c43-b1bb-c22594904fa0@gmail.com","threadId":"50667","inReplyTo":"14672ce10c90c46b15b62c48d2eea6fe8d85ed86.1551951770.git.liu.denton@gmail.com","subject":"Re: [PATCH v3 3/4] cherry-pick/revert: add scissors line on merge conflict","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-03-07T15:24:16Z","receivedAt":"2019-03-07T15:24:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Denton\n\nOn 07/03/2019 09:58, Denton Liu wrote:\n> Fix a bug where the scissors line is placed after the Conflicts:\n> section, in the case where a merge conflict occurs and\n> commit.cleanup = scissors.\n> \n> Note that the removal of the if-else tower in git_sequencer_config may\n> appear to be a no-op refactor but it actually isn't. First of all, we\n> now accept \"default\". More importantly, though, if\n> commit.cleanup = scissors, the cleanup enum will be set to\n> COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE when we\n> are reverting or cherry-picking. This allows us to append scissors to\n> MERGE_MSG in the case of a conflict.\n> \n> Signed-off-by: Denton Liu \n> ---\n> Documentation/git-cherry-pick.txt | 7 +++\n> Documentation/git-revert.txt | 7 +++\n> builtin/merge.c | 13 ++---\n> builtin/rebase--helper.c | 2 +-\n> builtin/revert.c | 5 ++\n> sequencer.c | 32 +++++-------\n> sequencer.h | 3 +-\n> t/t3507-cherry-pick-conflict.sh | 86 +++++++++++++++++++++++++++++++\n> 8 files changed, 125 insertions(+), 30 deletions(-)\n> \n> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> index d35d771fc8..92235bd1c4 100644\n> --- a/Documentation/git-cherry-pick.txt\n> +++ b/Documentation/git-cherry-pick.txt\n> @@ -57,6 +57,13 @@ OPTIONS\n> \tWith this option, 'git cherry-pick' will let you edit the commit\n> \tmessage prior to committing.\n> \n> +--cleanup=::\n> +\tThis option determines how the commit message will be cleaned up before\n> +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> +\taddition, if the '' is given a value of `scissors`, scissors will\n> +\tbe appended to MERGE_MSG before being passed on in the case of a\n> +\tconflict.\n> +\n> -x::\n> \tWhen recording the commit, append a line that says\n> \t\"(cherry picked from commit ...)\" to the original commit\n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index 837707a8fd..bd4ad395a9 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -66,6 +66,13 @@ more details.\n> \tWith this option, 'git revert' will not start the commit\n> \tmessage editor.\n> \n> +--cleanup=::\n> +\tThis option determines how the commit message will be cleaned up before\n> +\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> +\taddition, if the '' is given a value of `scissors`, scissors will\n> +\tbe appended to MERGE_MSG before being passed on in the case of a\n> +\tconflict.\n> +\n> -n::\n> --no-commit::\n> \tUsually the command automatically creates some commits with\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 92efc3d8fa..d4217ebcf5 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -913,17 +913,10 @@ static int suggest_conflicts(void)\n> \t * We can't use cleanup_mode because if we're not using the editor,\n> \t * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even\n> \t * though the message is meant to be processed later by git-commit.\n> -\t * Thus, we will get the cleanup mode is returned we _are_ using an\n> -\t * editor.\n> +\t * Thus, we will get the cleanup mode which is returned when we _are_ using\n> +\t * an editor.\n> \t */\n> -\tif (get_cleanup_mode(cleanup_arg, 1) == COMMIT_MSG_CLEANUP_SCISSORS) {\n> -\t fputc('\\n', fp);\n> -\t wt_status_add_cut_line(fp);\n> -\t /* comments out the newline from append_conflicts_hint */\n> -\t fputc(comment_line_char, fp);\n> -\t}\n> -\n> -\tappend_conflicts_hint(&msgbuf);\n> +\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n> \tfputs(msgbuf.buf, fp);\n> \tstrbuf_release(&msgbuf);\n> \tfclose(fp);\n> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n> index f7c2a5fdc8..5f1bc9d383 100644\n> --- a/builtin/rebase--helper.c\n> +++ b/builtin/rebase--helper.c\n> @@ -48,10 +48,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n> \t\tOPT_END()\n> \t};\n> \n> +\topts.action = REPLAY_INTERACTIVE_REBASE;\n> \tsequencer_init_config(&opts);\n> \tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n> \n> -\topts.action = REPLAY_INTERACTIVE_REBASE;\n> \topts.allow_ff = 1;\n> \topts.allow_empty = 1;\n\n\nWhat are you basing this series on? builtin/rebase--helper.c was removed \nlast September in 34b47315d9 (\"rebase -i: move rebase--helper modes to \nrebase--interactive\", 2018-09-27)\n\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 9a66720cfc..fe18036be7 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> {\n> \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n> \tconst char *me = action_name(opts);\n> +\tconst char *cleanup_arg = NULL;\n> \tint cmd = 0;\n> \tstruct option base_options[] = {\n> \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n> \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n> \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n> +\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n> \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n> \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n> \t\tOPT_NOOP_NOARG('r', NULL),\n> @@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \tif (opts->keep_redundant_commits)\n> \t\topts->allow_empty = 1;\n> \n> +\tif (cleanup_arg)\n> +\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n> +\n> \t/* Check for incompatible command line arguments */\n> \tif (cmd) {\n> \t\tchar *this_operation;\n> diff --git a/sequencer.c b/sequencer.c\n> index 707e72fb39..5c04bae7ac 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -153,6 +153,11 @@ static GIT_PATH_FUNC(rebase_path_strategy, \"rebase-merge/strategy\")\n> static GIT_PATH_FUNC(rebase_path_strategy_opts, \"rebase-merge/strategy_opts\")\n> static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, \"rebase-merge/allow_rerere_autoupdate\")\n> \n> +static inline int is_rebase_i(const struct replay_opts *opts)\n> +{\n> +\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n> +}\n> +\n\nThis is already in sequencer.c in a different place in recent versions\n\nBest Wishes\n\nPhillip\n\n> static int git_sequencer_config(const char *k, const char *v, void *cb)\n> {\n> \tstruct replay_opts *opts = cb;\n> @@ -165,17 +170,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n> \t\tif (status)\n> \t\t\treturn status;\n> \n> -\t\tif (!strcmp(s, \"verbatim\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n> -\t\telse if (!strcmp(s, \"whitespace\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> -\t\telse if (!strcmp(s, \"strip\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n> -\t\telse if (!strcmp(s, \"scissors\"))\n> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> -\t\telse\n> -\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n> -\t\t\t\t s);\n> +\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n> \n> \t\tfree((char *)s);\n> \t\treturn status;\n> @@ -199,11 +194,6 @@ void sequencer_init_config(struct replay_opts *opts)\n> \tgit_config(git_sequencer_config, opts);\n> }\n> \n> -static inline int is_rebase_i(const struct replay_opts *opts)\n> -{\n> -\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n> -}\n> -\n> static const char *get_dir(const struct replay_opts *opts)\n> {\n> \tif (is_rebase_i(opts))\n> @@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> }\n> \n> -void append_conflicts_hint(struct strbuf *msgbuf)\n> +void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n> {\n> \tint i;\n> \n> +\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n> +\t\tstrbuf_addch(msgbuf, '\\n');\n> +\t\twt_status_append_cut_line(msgbuf);\n> +\t\tstrbuf_addch(msgbuf, comment_line_char);\n> +\t}\n> +\n> \tstrbuf_addch(msgbuf, '\\n');\n> \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n> \tfor (i = 0; i < active_nr;) {\n> @@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n> \t\t\t_(action_name(opts)));\n> \n> \tif (!clean)\n> -\t\tappend_conflicts_hint(msgbuf);\n> +\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n> \n> \treturn !clean;\n> }\n> diff --git a/sequencer.h b/sequencer.h\n> index 5690e0c27e..aa99503dd7 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -91,7 +91,8 @@ int rearrange_squash(void);\n> extern const char sign_off_header[];\n> \n> void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n> -void append_conflicts_hint(struct strbuf *msgbuf);\n> +void append_conflicts_hint(struct strbuf *msgbuf,\n> +\t\tenum commit_msg_cleanup_mode cleanup_mode);\n> enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> \tint use_editor);\n> \n> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n> index 74ff925526..c3894ca9d6 100755\n> --- a/t/t3507-cherry-pick-conflict.sh\n> +++ b/t/t3507-cherry-pick-conflict.sh\n> @@ -189,6 +189,46 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success \\\n> +\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config commit.cleanup scissors &&\n> +\tcat <<-EOF >expected &&\n> +\t\tpicked\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git cherry-pick picked &&\n> +\n> +\ttest_i18ncmp expected .git/MERGE_MSG\n> +'\n> +\n> +test_expect_success \\\n> +\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config --unset commit.cleanup &&\n> +\tcat <<-EOF >expected &&\n> +\t\tpicked\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n> +\n> +\ttest_i18ncmp expected .git/MERGE_MSG\n> +'\n> +\n> test_expect_success 'failed cherry-pick describes conflict in work tree' '\n> \tpristine_detach initial &&\n> \tcat <<-EOF >expected &&\n> @@ -335,6 +375,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success \\\n> +\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config commit.cleanup scissors &&\n> +\tcat >expected <<-EOF &&\n> +\t\tRevert \"picked\"\n> +\n> +\t\tThis reverts commit OBJID.\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git revert picked &&\n> +\n> +\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n> +\ttest_i18ncmp expected actual\n> +'\n> +\n> +test_expect_success \\\n> +\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n> +\tpristine_detach initial &&\n> +\tgit config --unset commit.cleanup &&\n> +\tcat >expected <<-EOF &&\n> +\t\tRevert \"picked\"\n> +\n> +\t\tThis reverts commit OBJID.\n> +\n> +\t\t# ------------------------ >8 ------------------------\n> +\t\t# Do not modify or remove the line above.\n> +\t\t# Everything below it will be ignored.\n> +\t\t#\n> +\t\t# Conflicts:\n> +\t\t#\tfoo\n> +\t\tEOF\n> +\n> +\ttest_must_fail git revert --cleanup=scissors picked &&\n> +\n> +\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n> +\ttest_i18ncmp expected actual\n> +'\n> +\n> test_expect_success 'failed cherry-pick does not forget -s' '\n> \tpristine_detach initial &&\n> \ttest_must_fail git cherry-pick -s picked &&\n> \n"},{"id":"370917","messageId":"20190307175628.GA14130@dev-l","threadId":"50667","inReplyTo":"b568185b-2411-7c43-b1bb-c22594904fa0@gmail.com","subject":"Re: [PATCH v3 3/4] cherry-pick/revert: add scissors line on merge conflict","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-07T17:56:28Z","receivedAt":"2019-03-07T17:56:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Phillip,\n\nOn Thu, Mar 07, 2019 at 03:24:16PM +0000, Phillip Wood wrote:\n> Hi Denton\n> \n> On 07/03/2019 09:58, Denton Liu wrote:\n> >Fix a bug where the scissors line is placed after the Conflicts:\n> >section, in the case where a merge conflict occurs and\n> >commit.cleanup = scissors.\n> >\n> >Note that the removal of the if-else tower in git_sequencer_config may\n> >appear to be a no-op refactor but it actually isn't. First of all, we\n> >now accept \"default\". More importantly, though, if\n> >commit.cleanup = scissors, the cleanup enum will be set to\n> >COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE when we\n> >are reverting or cherry-picking. This allows us to append scissors to\n> >MERGE_MSG in the case of a conflict.\n> >\n> >Signed-off-by: Denton Liu \n> >---\n> > Documentation/git-cherry-pick.txt | 7 +++\n> > Documentation/git-revert.txt | 7 +++\n> > builtin/merge.c | 13 ++---\n> > builtin/rebase--helper.c | 2 +-\n> > builtin/revert.c | 5 ++\n> > sequencer.c | 32 +++++-------\n> > sequencer.h | 3 +-\n> > t/t3507-cherry-pick-conflict.sh | 86 +++++++++++++++++++++++++++++++\n> > 8 files changed, 125 insertions(+), 30 deletions(-)\n> >\n> >diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> >index d35d771fc8..92235bd1c4 100644\n> >--- a/Documentation/git-cherry-pick.txt\n> >+++ b/Documentation/git-cherry-pick.txt\n> >@@ -57,6 +57,13 @@ OPTIONS\n> > \tWith this option, 'git cherry-pick' will let you edit the commit\n> > \tmessage prior to committing.\n> >+--cleanup=::\n> >+\tThis option determines how the commit message will be cleaned up before\n> >+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> >+\taddition, if the '' is given a value of `scissors`, scissors will\n> >+\tbe appended to MERGE_MSG before being passed on in the case of a\n> >+\tconflict.\n> >+\n> > -x::\n> > \tWhen recording the commit, append a line that says\n> > \t\"(cherry picked from commit ...)\" to the original commit\n> >diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> >index 837707a8fd..bd4ad395a9 100644\n> >--- a/Documentation/git-revert.txt\n> >+++ b/Documentation/git-revert.txt\n> >@@ -66,6 +66,13 @@ more details.\n> > \tWith this option, 'git revert' will not start the commit\n> > \tmessage editor.\n> >+--cleanup=::\n> >+\tThis option determines how the commit message will be cleaned up before\n> >+\tbeing passed on. See linkgit:git-commit[1] for more details. In\n> >+\taddition, if the '' is given a value of `scissors`, scissors will\n> >+\tbe appended to MERGE_MSG before being passed on in the case of a\n> >+\tconflict.\n> >+\n> > -n::\n> > --no-commit::\n> > \tUsually the command automatically creates some commits with\n> >diff --git a/builtin/merge.c b/builtin/merge.c\n> >index 92efc3d8fa..d4217ebcf5 100644\n> >--- a/builtin/merge.c\n> >+++ b/builtin/merge.c\n> >@@ -913,17 +913,10 @@ static int suggest_conflicts(void)\n> > \t * We can't use cleanup_mode because if we're not using the editor,\n> > \t * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even\n> > \t * though the message is meant to be processed later by git-commit.\n> >-\t * Thus, we will get the cleanup mode is returned we _are_ using an\n> >-\t * editor.\n> >+\t * Thus, we will get the cleanup mode which is returned when we _are_ using\n> >+\t * an editor.\n> > \t */\n> >-\tif (get_cleanup_mode(cleanup_arg, 1) == COMMIT_MSG_CLEANUP_SCISSORS) {\n> >-\t fputc('\\n', fp);\n> >-\t wt_status_add_cut_line(fp);\n> >-\t /* comments out the newline from append_conflicts_hint */\n> >-\t fputc(comment_line_char, fp);\n> >-\t}\n> >-\n> >-\tappend_conflicts_hint(&msgbuf);\n> >+\tappend_conflicts_hint(&msgbuf, get_cleanup_mode(cleanup_arg, 1));\n> > \tfputs(msgbuf.buf, fp);\n> > \tstrbuf_release(&msgbuf);\n> > \tfclose(fp);\n> >diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n> >index f7c2a5fdc8..5f1bc9d383 100644\n> >--- a/builtin/rebase--helper.c\n> >+++ b/builtin/rebase--helper.c\n> >@@ -48,10 +48,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n> > \t\tOPT_END()\n> > \t};\n> >+\topts.action = REPLAY_INTERACTIVE_REBASE;\n> > \tsequencer_init_config(&opts);\n> > \tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n> >-\topts.action = REPLAY_INTERACTIVE_REBASE;\n> > \topts.allow_ff = 1;\n> > \topts.allow_empty = 1;\n> \n> \n> What are you basing this series on? builtin/rebase--helper.c was removed\n> last September in 34b47315d9 (\"rebase -i: move rebase--helper modes to\n> rebase--interactive\", 2018-09-27)\n\nI was basing this patch on the tip of dl/merge-cleanup-scissors-fix. I\ncan rebase or merge my work to something else but I'll wait for\nadditional directions since I don't know what would be best.\n\n> \n> >diff --git a/builtin/revert.c b/builtin/revert.c\n> >index 9a66720cfc..fe18036be7 100644\n> >--- a/builtin/revert.c\n> >+++ b/builtin/revert.c\n> >@@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> > {\n> > \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n> > \tconst char *me = action_name(opts);\n> >+\tconst char *cleanup_arg = NULL;\n> > \tint cmd = 0;\n> > \tstruct option base_options[] = {\n> > \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n> > \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n> > \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n> >+\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n> > \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n> > \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n> > \t\tOPT_NOOP_NOARG('r', NULL),\n> >@@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> > \tif (opts->keep_redundant_commits)\n> > \t\topts->allow_empty = 1;\n> >+\tif (cleanup_arg)\n> >+\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n> >+\n> > \t/* Check for incompatible command line arguments */\n> > \tif (cmd) {\n> > \t\tchar *this_operation;\n> >diff --git a/sequencer.c b/sequencer.c\n> >index 707e72fb39..5c04bae7ac 100644\n> >--- a/sequencer.c\n> >+++ b/sequencer.c\n> >@@ -153,6 +153,11 @@ static GIT_PATH_FUNC(rebase_path_strategy, \"rebase-merge/strategy\")\n> > static GIT_PATH_FUNC(rebase_path_strategy_opts, \"rebase-merge/strategy_opts\")\n> > static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, \"rebase-merge/allow_rerere_autoupdate\")\n> >+static inline int is_rebase_i(const struct replay_opts *opts)\n> >+{\n> >+\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n> >+}\n> >+\n> \n> This is already in sequencer.c in a different place in recent versions\n\nI was just moving this function from below here so that the compiler\nwould pick it up.\n\n> \n> Best Wishes\n> \n> Phillip\n> \n> > static int git_sequencer_config(const char *k, const char *v, void *cb)\n> > {\n> > \tstruct replay_opts *opts = cb;\n> >@@ -165,17 +170,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n> > \t\tif (status)\n> > \t\t\treturn status;\n> >-\t\tif (!strcmp(s, \"verbatim\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n> >-\t\telse if (!strcmp(s, \"whitespace\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> >-\t\telse if (!strcmp(s, \"strip\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n> >-\t\telse if (!strcmp(s, \"scissors\"))\n> >-\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n> >-\t\telse\n> >-\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n> >-\t\t\t\t s);\n> >+\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n> > \t\tfree((char *)s);\n> > \t\treturn status;\n> >@@ -199,11 +194,6 @@ void sequencer_init_config(struct replay_opts *opts)\n> > \tgit_config(git_sequencer_config, opts);\n> > }\n> >-static inline int is_rebase_i(const struct replay_opts *opts)\n> >-{\n> >-\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n> >-}\n> >-\n> > static const char *get_dir(const struct replay_opts *opts)\n> > {\n> > \tif (is_rebase_i(opts))\n> >@@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> > \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> > }\n> >-void append_conflicts_hint(struct strbuf *msgbuf)\n> >+void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n> > {\n> > \tint i;\n> >+\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n> >+\t\tstrbuf_addch(msgbuf, '\\n');\n> >+\t\twt_status_append_cut_line(msgbuf);\n> >+\t\tstrbuf_addch(msgbuf, comment_line_char);\n> >+\t}\n> >+\n> > \tstrbuf_addch(msgbuf, '\\n');\n> > \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n> > \tfor (i = 0; i < active_nr;) {\n> >@@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n> > \t\t\t_(action_name(opts)));\n> > \tif (!clean)\n> >-\t\tappend_conflicts_hint(msgbuf);\n> >+\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n> > \treturn !clean;\n> > }\n> >diff --git a/sequencer.h b/sequencer.h\n> >index 5690e0c27e..aa99503dd7 100644\n> >--- a/sequencer.h\n> >+++ b/sequencer.h\n> >@@ -91,7 +91,8 @@ int rearrange_squash(void);\n> > extern const char sign_off_header[];\n> > void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n> >-void append_conflicts_hint(struct strbuf *msgbuf);\n> >+void append_conflicts_hint(struct strbuf *msgbuf,\n> >+\t\tenum commit_msg_cleanup_mode cleanup_mode);\n> > enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> > \tint use_editor);\n> >diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n> >index 74ff925526..c3894ca9d6 100755\n> >--- a/t/t3507-cherry-pick-conflict.sh\n> >+++ b/t/t3507-cherry-pick-conflict.sh\n> >@@ -189,6 +189,46 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n> > \ttest_cmp expected actual\n> > '\n> >+test_expect_success \\\n> >+\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config commit.cleanup scissors &&\n> >+\tcat <<-EOF >expected &&\n> >+\t\tpicked\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git cherry-pick picked &&\n> >+\n> >+\ttest_i18ncmp expected .git/MERGE_MSG\n> >+'\n> >+\n> >+test_expect_success \\\n> >+\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config --unset commit.cleanup &&\n> >+\tcat <<-EOF >expected &&\n> >+\t\tpicked\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n> >+\n> >+\ttest_i18ncmp expected .git/MERGE_MSG\n> >+'\n> >+\n> > test_expect_success 'failed cherry-pick describes conflict in work tree' '\n> > \tpristine_detach initial &&\n> > \tcat <<-EOF >expected &&\n> >@@ -335,6 +375,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n> > \ttest_cmp expected actual\n> > '\n> >+test_expect_success \\\n> >+\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config commit.cleanup scissors &&\n> >+\tcat >expected <<-EOF &&\n> >+\t\tRevert \"picked\"\n> >+\n> >+\t\tThis reverts commit OBJID.\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git revert picked &&\n> >+\n> >+\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n> >+\ttest_i18ncmp expected actual\n> >+'\n> >+\n> >+test_expect_success \\\n> >+\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n> >+\tpristine_detach initial &&\n> >+\tgit config --unset commit.cleanup &&\n> >+\tcat >expected <<-EOF &&\n> >+\t\tRevert \"picked\"\n> >+\n> >+\t\tThis reverts commit OBJID.\n> >+\n> >+\t\t# ------------------------ >8 ------------------------\n> >+\t\t# Do not modify or remove the line above.\n> >+\t\t# Everything below it will be ignored.\n> >+\t\t#\n> >+\t\t# Conflicts:\n> >+\t\t#\tfoo\n> >+\t\tEOF\n> >+\n> >+\ttest_must_fail git revert --cleanup=scissors picked &&\n> >+\n> >+\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n> >+\ttest_i18ncmp expected actual\n> >+'\n> >+\n> > test_expect_success 'failed cherry-pick does not forget -s' '\n> > \tpristine_detach initial &&\n> > \ttest_must_fail git cherry-pick -s picked &&\n> >\n\nThanks,\n\nDenton\n"},{"id":"370919","messageId":"8404ea52-6556-1232-176e-f74f8d9583f8@gmail.com","threadId":"50667","inReplyTo":"20190307175628.GA14130@dev-l","subject":"Re: [PATCH v3 3/4] cherry-pick/revert: add scissors line on merge conflict","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-03-07T18:36:39Z","receivedAt":"2019-03-07T18:36:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 07/03/2019 17:56, Denton Liu wrote:\n> Hi Phillip,\n> \n> On Thu, Mar 07, 2019 at 03:24:16PM +0000, Phillip Wood wrote:\n>> Hi Denton\n>>\n>> On 07/03/2019 09:58, Denton Liu wrote:\n>>> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n>>> index f7c2a5fdc8..5f1bc9d383 100644\n>>> --- a/builtin/rebase--helper.c\n>>> +++ b/builtin/rebase--helper.c\n>>> @@ -48,10 +48,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n>>> \t\tOPT_END()\n>>> \t};\n>>> +\topts.action = REPLAY_INTERACTIVE_REBASE;\n>>> \tsequencer_init_config(&opts);\n>>> \tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n>>> -\topts.action = REPLAY_INTERACTIVE_REBASE;\n>>> \topts.allow_ff = 1;\n>>> \topts.allow_empty = 1;\n>>\n>>\n>> What are you basing this series on? builtin/rebase--helper.c was removed\n>> last September in 34b47315d9 (\"rebase -i: move rebase--helper modes to\n>> rebase--interactive\", 2018-09-27)\n> \n> I was basing this patch on the tip of dl/merge-cleanup-scissors-fix. I\n> can rebase or merge my work to something else but I'll wait for\n> additional directions since I don't know what would be best.\n\nSee what Junio says, I think it might be simple enough for him to fix\nthat up when he merges it into pu.\n\n\nI've just realized that if you're cherry-picking a range of commits and\nit stops for a conflict resolution then the new option is not saved.\nYou'll need to update populate_opts_cb() and save_opts() in sequencer.c\nto do that.\n\nBest Wishes\n\nPhillip\n\n\n>>> diff --git a/builtin/revert.c b/builtin/revert.c\n>>> index 9a66720cfc..fe18036be7 100644\n>>> --- a/builtin/revert.c\n>>> +++ b/builtin/revert.c\n>>> @@ -95,11 +95,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n>>> {\n>>> \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n>>> \tconst char *me = action_name(opts);\n>>> +\tconst char *cleanup_arg = NULL;\n>>> \tint cmd = 0;\n>>> \tstruct option base_options[] = {\n>>> \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n>>> \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n>>> \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n>>> +\t\tOPT_STRING(0, \"cleanup\", &cleanup_arg, N_(\"default\"), N_(\"how to strip spaces and #comments from message\")),\n>>> \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n>>> \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n>>> \t\tOPT_NOOP_NOARG('r', NULL),\n>>> @@ -136,6 +138,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n>>> \tif (opts->keep_redundant_commits)\n>>> \t\topts->allow_empty = 1;\n>>> +\tif (cleanup_arg)\n>>> +\t\topts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);\n>>> +\n>>> \t/* Check for incompatible command line arguments */\n>>> \tif (cmd) {\n>>> \t\tchar *this_operation;\n>>> diff --git a/sequencer.c b/sequencer.c\n>>> index 707e72fb39..5c04bae7ac 100644\n>>> --- a/sequencer.c\n>>> +++ b/sequencer.c\n>>> @@ -153,6 +153,11 @@ static GIT_PATH_FUNC(rebase_path_strategy, \"rebase-merge/strategy\")\n>>> static GIT_PATH_FUNC(rebase_path_strategy_opts, \"rebase-merge/strategy_opts\")\n>>> static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, \"rebase-merge/allow_rerere_autoupdate\")\n>>> +static inline int is_rebase_i(const struct replay_opts *opts)\n>>> +{\n>>> +\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n>>> +}\n>>> +\n>>\n>> This is already in sequencer.c in a different place in recent versions\n> \n> I was just moving this function from below here so that the compiler\n> would pick it up.\n> \n>>\n>> Best Wishes\n>>\n>> Phillip\n>>\n>>> static int git_sequencer_config(const char *k, const char *v, void *cb)\n>>> {\n>>> \tstruct replay_opts *opts = cb;\n>>> @@ -165,17 +170,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)\n>>> \t\tif (status)\n>>> \t\t\treturn status;\n>>> -\t\tif (!strcmp(s, \"verbatim\"))\n>>> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;\n>>> -\t\telse if (!strcmp(s, \"whitespace\"))\n>>> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n>>> -\t\telse if (!strcmp(s, \"strip\"))\n>>> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;\n>>> -\t\telse if (!strcmp(s, \"scissors\"))\n>>> -\t\t\topts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;\n>>> -\t\telse\n>>> -\t\t\twarning(_(\"invalid commit message cleanup mode '%s'\"),\n>>> -\t\t\t\t s);\n>>> +\t\topts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));\n>>> \t\tfree((char *)s);\n>>> \t\treturn status;\n>>> @@ -199,11 +194,6 @@ void sequencer_init_config(struct replay_opts *opts)\n>>> \tgit_config(git_sequencer_config, opts);\n>>> }\n>>> -static inline int is_rebase_i(const struct replay_opts *opts)\n>>> -{\n>>> -\treturn opts->action == REPLAY_INTERACTIVE_REBASE;\n>>> -}\n>>> -\n>>> static const char *get_dir(const struct replay_opts *opts)\n>>> {\n>>> \tif (is_rebase_i(opts))\n>>> @@ -516,10 +506,16 @@ enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n>>> \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n>>> }\n>>> -void append_conflicts_hint(struct strbuf *msgbuf)\n>>> +void append_conflicts_hint(struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode)\n>>> {\n>>> \tint i;\n>>> +\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n>>> +\t\tstrbuf_addch(msgbuf, '\\n');\n>>> +\t\twt_status_append_cut_line(msgbuf);\n>>> +\t\tstrbuf_addch(msgbuf, comment_line_char);\n>>> +\t}\n>>> +\n>>> \tstrbuf_addch(msgbuf, '\\n');\n>>> \tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n>>> \tfor (i = 0; i < active_nr;) {\n>>> @@ -586,7 +582,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n>>> \t\t\t_(action_name(opts)));\n>>> \tif (!clean)\n>>> -\t\tappend_conflicts_hint(msgbuf);\n>>> +\t\tappend_conflicts_hint(msgbuf, opts->default_msg_cleanup);\n>>> \treturn !clean;\n>>> }\n>>> diff --git a/sequencer.h b/sequencer.h\n>>> index 5690e0c27e..aa99503dd7 100644\n>>> --- a/sequencer.h\n>>> +++ b/sequencer.h\n>>> @@ -91,7 +91,8 @@ int rearrange_squash(void);\n>>> extern const char sign_off_header[];\n>>> void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n>>> -void append_conflicts_hint(struct strbuf *msgbuf);\n>>> +void append_conflicts_hint(struct strbuf *msgbuf,\n>>> +\t\tenum commit_msg_cleanup_mode cleanup_mode);\n>>> enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n>>> \tint use_editor);\n>>> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n>>> index 74ff925526..c3894ca9d6 100755\n>>> --- a/t/t3507-cherry-pick-conflict.sh\n>>> +++ b/t/t3507-cherry-pick-conflict.sh\n>>> @@ -189,6 +189,46 @@ test_expect_success 'failed cherry-pick registers participants in index' '\n>>> \ttest_cmp expected actual\n>>> '\n>>> +test_expect_success \\\n>>> +\t'cherry-pick conflict, ensure commit.cleanup = scissors places scissors line properly' '\n>>> +\tpristine_detach initial &&\n>>> +\tgit config commit.cleanup scissors &&\n>>> +\tcat <<-EOF >expected &&\n>>> +\t\tpicked\n>>> +\n>>> +\t\t# ------------------------ >8 ------------------------\n>>> +\t\t# Do not modify or remove the line above.\n>>> +\t\t# Everything below it will be ignored.\n>>> +\t\t#\n>>> +\t\t# Conflicts:\n>>> +\t\t#\tfoo\n>>> +\t\tEOF\n>>> +\n>>> +\ttest_must_fail git cherry-pick picked &&\n>>> +\n>>> +\ttest_i18ncmp expected .git/MERGE_MSG\n>>> +'\n>>> +\n>>> +test_expect_success \\\n>>> +\t'cherry-pick conflict, ensure cleanup=scissors places scissors line properly' '\n>>> +\tpristine_detach initial &&\n>>> +\tgit config --unset commit.cleanup &&\n>>> +\tcat <<-EOF >expected &&\n>>> +\t\tpicked\n>>> +\n>>> +\t\t# ------------------------ >8 ------------------------\n>>> +\t\t# Do not modify or remove the line above.\n>>> +\t\t# Everything below it will be ignored.\n>>> +\t\t#\n>>> +\t\t# Conflicts:\n>>> +\t\t#\tfoo\n>>> +\t\tEOF\n>>> +\n>>> +\ttest_must_fail git cherry-pick --cleanup=scissors picked &&\n>>> +\n>>> +\ttest_i18ncmp expected .git/MERGE_MSG\n>>> +'\n>>> +\n>>> test_expect_success 'failed cherry-pick describes conflict in work tree' '\n>>> \tpristine_detach initial &&\n>>> \tcat <<-EOF >expected &&\n>>> @@ -335,6 +375,52 @@ test_expect_success 'revert conflict, diff3 -m style' '\n>>> \ttest_cmp expected actual\n>>> '\n>>> +test_expect_success \\\n>>> +\t'revert conflict, ensure commit.cleanup = scissors places scissors line properly' '\n>>> +\tpristine_detach initial &&\n>>> +\tgit config commit.cleanup scissors &&\n>>> +\tcat >expected <<-EOF &&\n>>> +\t\tRevert \"picked\"\n>>> +\n>>> +\t\tThis reverts commit OBJID.\n>>> +\n>>> +\t\t# ------------------------ >8 ------------------------\n>>> +\t\t# Do not modify or remove the line above.\n>>> +\t\t# Everything below it will be ignored.\n>>> +\t\t#\n>>> +\t\t# Conflicts:\n>>> +\t\t#\tfoo\n>>> +\t\tEOF\n>>> +\n>>> +\ttest_must_fail git revert picked &&\n>>> +\n>>> +\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n>>> +\ttest_i18ncmp expected actual\n>>> +'\n>>> +\n>>> +test_expect_success \\\n>>> +\t'revert conflict, ensure cleanup=scissors places scissors line properly' '\n>>> +\tpristine_detach initial &&\n>>> +\tgit config --unset commit.cleanup &&\n>>> +\tcat >expected <<-EOF &&\n>>> +\t\tRevert \"picked\"\n>>> +\n>>> +\t\tThis reverts commit OBJID.\n>>> +\n>>> +\t\t# ------------------------ >8 ------------------------\n>>> +\t\t# Do not modify or remove the line above.\n>>> +\t\t# Everything below it will be ignored.\n>>> +\t\t#\n>>> +\t\t# Conflicts:\n>>> +\t\t#\tfoo\n>>> +\t\tEOF\n>>> +\n>>> +\ttest_must_fail git revert --cleanup=scissors picked &&\n>>> +\n>>> +\tsed \"s/$OID_REGEX/OBJID/\" .git/MERGE_MSG >actual &&\n>>> +\ttest_i18ncmp expected actual\n>>> +'\n>>> +\n>>> test_expect_success 'failed cherry-pick does not forget -s' '\n>>> \tpristine_detach initial &&\n>>> \ttest_must_fail git cherry-pick -s picked &&\n>>>\n> \n> Thanks,\n> \n> Denton\n> \n\n"},{"id":"370932","messageId":"xmqqo96mkz0d.fsf@gitster-ct.c.googlers.com","threadId":"50667","inReplyTo":"8404ea52-6556-1232-176e-f74f8d9583f8@gmail.com","subject":"Re: [PATCH v3 3/4] cherry-pick/revert: add scissors line on merge conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-08T00:09:38Z","receivedAt":"2019-03-08T00:09:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>>> What are you basing this series on? builtin/rebase--helper.c was removed\n>>> last September in 34b47315d9 (\"rebase -i: move rebase--helper modes to\n>>> rebase--interactive\", 2018-09-27)\n>> \n>> I was basing this patch on the tip of dl/merge-cleanup-scissors-fix. I\n>> can rebase or merge my work to something else but I'll wait for\n>> additional directions since I don't know what would be best.\n>\n> See what Junio says, I think it might be simple enough for him to fix\n> that up when he merges it into pu.\n\nPerhaps it is a good time to kick dl/merge-cleanup-scissors-fix out\nof the 'next' branch, so that the whole thing can be rebuilt on top\nof a more recent base like v2.21.0, without the need for these\n\"oops, that was wrong, so let's patch it up\" fixes.\n\n> I've just realized that if you're cherry-picking a range of commits and\n> it stops for a conflict resolution then the new option is not saved.\n> You'll need to update populate_opts_cb() and save_opts() in sequencer.c\n> to do that.\n\nYeah, good point.\n"},{"id":"370940","messageId":"xmqqo96mjap5.fsf@gitster-ct.c.googlers.com","threadId":"50667","inReplyTo":"1076d4c45108b1edad48fcac0f203c8c4e521b79.1551951770.git.liu.denton@gmail.com","subject":"Re: [PATCH v3 1/4] merge-options.txt: correct typo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-08T03:40:06Z","receivedAt":"2019-03-08T03:40:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> The scissors line is included after the human commit message. Therefore,\n> saying \"scissors will be prepended\" was incorrect. Update to say\n> \"appended\" which is more correct.\n>\n> Signed-off-by: Denton Liu \n> ---\n>\n> Notes:\n> Note that this can be squashed into 6f06b6aeef (merge: add scissors line on\n> merge conflict, 2019-01-22) if it makes life easier.\n\nThe reason why you had to make this as an incremental on top of an\nolder codebase (cf. Phillip's comment on 3/4) is because it is\nalready in 'next' and we do not wholesale replace. I suspect that\nwe merged it 'next' before it was ready---sorry about that.\n\nAs the original series dl/merge-cleanup-scissors-fix is a short\nseries that is only 4 patches long, let me kick it out of 'next' to\nmake our lives easier. That way, you can send the squashed result\nas a fresh iteration, solving what these new 4 patches tries to as\nthe proper part of the series, instead of making them as if they are\nafterthought on top of the original 4-patch series that was\nincomplete.\n\nThanks.\n"}]}