sequencer: beautify subject of reverts of reverts

Instead of generating a silly-looking `Revert "Revert "foo""`, make it
a more humane `Reapply "foo"`.

This is done for two reasons:
- To cover the actually common case of just a double revert.
- To encourage people to rewrite summaries of recursive reverts by
  setting an example (a subsequent commit will also do this explicitly
  in the documentation).

To achieve these goals, the mechanism does not need to be particularly
sophisticated. Therefore, more complicated alternatives which would
"compress more efficiently" have not been implemented.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index 5e0c15a16b73b3bf04fc76afc2bc2bd1c15f677d..7dc13fdccaa06d7a1285d4e2464a6bbb2ea8ebeb 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -2249,6 +2249,8 @@ static int do_pick_commit(struct repository *r,
         */
 
        if (command == TODO_REVERT) {
+               const char *orig_subject;
+
                base = commit;
                base_label = msg.label;
                next = parent;
@@ -2256,6 +2258,15 @@ static int do_pick_commit(struct repository *r,
                if (opts->commit_use_reference) {
                        strbuf_addstr(&msgbuf,
                                "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
+               } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) &&
+                          /*
+                           * We don't touch pre-existing repeated reverts, because
+                           * theoretically these can be nested arbitrarily deeply,
+                           * thus requiring excessive complexity to deal with.
+                           */
+                          !starts_with(orig_subject, "Revert \"")) {
+                       strbuf_addstr(&msgbuf, "Reapply \"");
+                       strbuf_addstr(&msgbuf, orig_subject);
                } else {
                        strbuf_addstr(&msgbuf, "Revert \"");
                        strbuf_addstr(&msgbuf, msg.subject);

diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index e2ef6193233dd0b7a2341c391695aabd73d03200..4158590322321742dcc0fef90d43591db3c0ab09 100755 (executable)
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -176,6 +176,29 @@ test_expect_success 'advice from failed revert' '
        test_cmp expected actual
 '
 
+test_expect_subject () {
+       echo "$1" >expect &&
+       git log -1 --pretty=%s >actual &&
+       test_cmp expect actual
+}
+
+test_expect_success 'titles of fresh reverts' '
+       test_commit --no-tag A file1 &&
+       test_commit --no-tag B file1 &&
+       git revert --no-edit HEAD &&
+       test_expect_subject "Revert \"B\"" &&
+       git revert --no-edit HEAD &&
+       test_expect_subject "Reapply \"B\"" &&
+       git revert --no-edit HEAD &&
+       test_expect_subject "Revert \"Reapply \"B\"\""
+'
+
+test_expect_success 'title of legacy double revert' '
+       test_commit --no-tag "Revert \"Revert \"B\"\"" file1 &&
+       git revert --no-edit HEAD &&
+       test_expect_subject "Revert \"Revert \"Revert \"B\"\"\""
+'
+
 test_expect_success 'identification of reverted commit (default)' '
        test_commit to-ident &&
        test_when_finished "git reset --hard to-ident" &&