Teach merge the '[-e|--edit]' option

Implemented internally instead of as "git merge --no-commit && git commit"
so that "merge --edit" is otherwise consistent (hooks, etc) with "merge".

Note: the edit message does not include the status information that one
gets with "commit --status" and it is cleaned up after editing like one
gets with "commit --cleanup=default". A later patch could add the status
information if desired.

Note: previously we were not calling stripspace() after running the
prepare-commit-msg hook. Now we are, stripping comments and
leading/trailing whitespace lines if --edit is given, otherwise only
stripping leading/trailing whitespace lines if not given --edit.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index b613d4ed083d080797f7da90fc92212f98611d07..6bd0b041c3c1c0af5d03fce43d8016706e9256b1 100644 (file)
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -7,6 +7,12 @@ With --no-commit perform the merge but pretend the merge
 failed and do not autocommit, to give the user a chance to
 inspect and further tweak the merge result before committing.
 
+--edit::
+-e::
++
+       Invoke editor before committing successful merge to further
+       edit the default merge message.
+
 --ff::
 --no-ff::
        Do not generate a merge commit if the merge resolved as

diff --git a/builtin/merge.c b/builtin/merge.c
index ab4077f272919fb47e3c5f179dc42fc9baad68bb..a8dbf4a32f09a1377eb489459837e5a07e36f490 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,7 +46,7 @@ static const char * const builtin_merge_usage[] = {
 
 static int show_diffstat = 1, shortlog_len, squash;
 static int option_commit = 1, allow_fast_forward = 1;
-static int fast_forward_only;
+static int fast_forward_only, option_edit;
 static int allow_trivial = 1, have_message;
 static struct strbuf merge_msg;
 static struct commit_list *remoteheads;
@@ -190,6 +190,8 @@ static struct option builtin_merge_options[] = {
                "create a single commit instead of doing a merge"),
        OPT_BOOLEAN(0, "commit", &option_commit,
                "perform a commit if the merge succeeds (default)"),
+       OPT_BOOLEAN('e', "edit", &option_edit,
+               "edit message before committing"),
        OPT_BOOLEAN(0, "ff", &allow_fast_forward,
                "allow fast-forward (default)"),
        OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
@@ -832,30 +834,54 @@ static void add_strategies(const char *string, unsigned attr)
 
 }
 
-static void write_merge_msg(void)
+static void write_merge_msg(struct strbuf *msg)
 {
        int fd = open(git_path("MERGE_MSG"), O_WRONLY | O_CREAT, 0666);
        if (fd < 0)
                die_errno(_("Could not open '%s' for writing"),
                          git_path("MERGE_MSG"));
-       if (write_in_full(fd, merge_msg.buf, merge_msg.len) != merge_msg.len)
+       if (write_in_full(fd, msg->buf, msg->len) != msg->len)
                die_errno(_("Could not write to '%s'"), git_path("MERGE_MSG"));
        close(fd);
 }
 
-static void read_merge_msg(void)
+static void read_merge_msg(struct strbuf *msg)
 {
-       strbuf_reset(&merge_msg);
-       if (strbuf_read_file(&merge_msg, git_path("MERGE_MSG"), 0) < 0)
+       strbuf_reset(msg);
+       if (strbuf_read_file(msg, git_path("MERGE_MSG"), 0) < 0)
                die_errno(_("Could not read from '%s'"), git_path("MERGE_MSG"));
 }
 
-static void run_prepare_commit_msg(void)
+static void write_merge_state(void);
+static void abort_commit(const char *err_msg)
 {
-       write_merge_msg();
+       if (err_msg)
+               error("%s", err_msg);
+       fprintf(stderr,
+               _("Not committing merge; use 'git commit' to complete the merge.\n"));
+       write_merge_state();
+       exit(1);
+}
+
+static void prepare_to_commit(void)
+{
+       struct strbuf msg = STRBUF_INIT;
+       strbuf_addbuf(&msg, &merge_msg);
+       strbuf_addch(&msg, '\n');
+       write_merge_msg(&msg);
        run_hook(get_index_file(), "prepare-commit-msg",
                 git_path("MERGE_MSG"), "merge", NULL, NULL);
-       read_merge_msg();
+       if (option_edit) {
+               if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
+                       abort_commit(NULL);
+       }
+       read_merge_msg(&msg);
+       stripspace(&msg, option_edit);
+       if (!msg.len)
+               abort_commit(_("Empty commit message."));
+       strbuf_release(&merge_msg);
+       strbuf_addbuf(&merge_msg, &msg);
+       strbuf_release(&msg);
 }
 
 static int merge_trivial(void)
@@ -869,7 +895,7 @@ static int merge_trivial(void)
        parent->next = xmalloc(sizeof(*parent->next));
        parent->next->item = remoteheads->item;
        parent->next->next = NULL;
-       run_prepare_commit_msg();
+       prepare_to_commit();
        commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);
        finish(result_commit, "In-index merge");
        drop_save();
@@ -897,9 +923,9 @@ static int finish_automerge(struct commit_list *common,
                for (j = remoteheads; j; j = j->next)
                        pptr = &commit_list_insert(j->item, pptr)->next;
        }
-       free_commit_list(remoteheads);
        strbuf_addch(&merge_msg, '\n');
-       run_prepare_commit_msg();
+       prepare_to_commit();
+       free_commit_list(remoteheads);
        commit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);
        strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
        finish(result_commit, buf.buf);
@@ -1005,6 +1031,36 @@ static int setup_with_upstream(const char ***argv)
        return i;
 }
 
+static void write_merge_state(void)
+{
+       int fd;
+       struct commit_list *j;
+       struct strbuf buf = STRBUF_INIT;
+
+       for (j = remoteheads; j; j = j->next)
+               strbuf_addf(&buf, "%s\n",
+                       sha1_to_hex(j->item->object.sha1));
+       fd = open(git_path("MERGE_HEAD"), O_WRONLY | O_CREAT, 0666);
+       if (fd < 0)
+               die_errno(_("Could not open '%s' for writing"),
+                         git_path("MERGE_HEAD"));
+       if (write_in_full(fd, buf.buf, buf.len) != buf.len)
+               die_errno(_("Could not write to '%s'"), git_path("MERGE_HEAD"));
+       close(fd);
+       strbuf_addch(&merge_msg, '\n');
+       write_merge_msg(&merge_msg);
+       fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
+       if (fd < 0)
+               die_errno(_("Could not open '%s' for writing"),
+                         git_path("MERGE_MODE"));
+       strbuf_reset(&buf);
+       if (!allow_fast_forward)
+               strbuf_addf(&buf, "no-ff");
+       if (write_in_full(fd, buf.buf, buf.len) != buf.len)
+               die_errno(_("Could not write to '%s'"), git_path("MERGE_MODE"));
+       close(fd);
+}
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
        unsigned char result_tree[20];
@@ -1409,33 +1465,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
        if (squash)
                finish(NULL, NULL);
-       else {
-               int fd;
-               struct commit_list *j;
-
-               for (j = remoteheads; j; j = j->next)
-                       strbuf_addf(&buf, "%s\n",
-                               sha1_to_hex(j->item->object.sha1));
-               fd = open(git_path("MERGE_HEAD"), O_WRONLY | O_CREAT, 0666);
-               if (fd < 0)
-                       die_errno(_("Could not open '%s' for writing"),
-                                 git_path("MERGE_HEAD"));
-               if (write_in_full(fd, buf.buf, buf.len) != buf.len)
-                       die_errno(_("Could not write to '%s'"), git_path("MERGE_HEAD"));
-               close(fd);
-               strbuf_addch(&merge_msg, '\n');
-               write_merge_msg();
-               fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
-               if (fd < 0)
-                       die_errno(_("Could not open '%s' for writing"),
-                                 git_path("MERGE_MODE"));
-               strbuf_reset(&buf);
-               if (!allow_fast_forward)
-                       strbuf_addf(&buf, "no-ff");
-               if (write_in_full(fd, buf.buf, buf.len) != buf.len)
-                       die_errno(_("Could not write to '%s'"), git_path("MERGE_MODE"));
-               close(fd);
-       }
+       else
+               write_merge_state();
 
        if (merge_was_ok) {
                fprintf(stderr, _("Automatic merge went well; "

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 87aac835a1b864eab083b25940892b93af491326..3008e4e1210900be3bdc2e28dddd681ce8ec3551 100755 (executable)
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -643,4 +643,27 @@ test_expect_success 'amending no-ff merge commit' '
 
 test_debug 'git log --graph --decorate --oneline --all'
 
+cat >editor <<\EOF
+#!/bin/sh
+# Add a new message string that was not in the template
+(
+       echo "Merge work done on the side branch c1"
+       echo
+       cat <"$1"
+) >"$1.tmp" && mv "$1.tmp" "$1"
+# strip comments and blank lines from end of message
+sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
+EOF
+chmod 755 editor
+
+test_expect_success 'merge --no-ff --edit' '
+       git reset --hard c0 &&
+       EDITOR=./editor git merge --no-ff --edit c1 &&
+       verify_parents $c0 $c1 &&
+       git cat-file commit HEAD >raw &&
+       grep "work done on the side branch" raw &&
+       sed "1,/^$/d" >actual raw &&
+       test_cmp actual expected
+'
+
 test_done