revert: optionally refer to commit in the "reference" format

A typical "git revert" commit uses the full title of the original
commit in its title, and starts its body of the message with:

    This reverts commit 8fa7f667cf61386257c00d6e954855cc3215ae91.

This does not encourage the best practice of describing not just
"what" (i.e. "Revert X" on the title says what we did) but "why"
(i.e. and it does not say why X was undesirable).

We can instead phrase this first line of the body to be more like

    This reverts commit 8fa7f667 (do this and that, 2022-04-25)

so that the title does not have to be

    Revert "do this and that"

We can instead use the title to describe "why" we are reverting the
original commit.

Introduce the "--reference" option to "git revert", and also the
revert.reference configuration variable, which defaults to false, to
tweak the title and the first line of the draft commit message for
when creating a "revert" commit.

When this option is in use, the first line of the pre-filled editor
buffer becomes a comment line that tells the user to say _why_.  If
the user exits the editor without touching this line by mistake,
what we prepare to become the first line of the body, i.e. "This
reverts commit 8fa7f667 (do this and that, 2022-04-25)", ends up to
be the title of the resulting commit.  This behaviour is designed to
help such a user to identify such a revert in "git log --oneline"
easily so that it can be further reworded with "git rebase -i" later.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/revert.txt b/Documentation/config/revert.txt
new file mode 100644 (file)
index 0000000..797bfb6
--- /dev/null
+++ b/Documentation/config/revert.txt
@@ -0,0 +1,3 @@
+revert.reference::
+       Setting this variable to true makes `git revert` to behave
+       as if the `--reference` option is given.

diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index bb92a4a4516d2a93a6e85facb0831aabc1be126c..8463fe9cf75a3e9aa9cd2dced9907c6c00a87693 100644 (file)
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -117,6 +117,15 @@ effect to your index in a row.
        Allow the rerere mechanism to update the index with the
        result of auto-conflict resolution if possible.
 
+--reference::
+       Instead of starting the body of the log message with "This
+       reverts <full object name of the commit being reverted>.",
+       refer to the commit using "--pretty=reference" format
+       (cf. linkgit:git-log[1]).  The `revert.reference`
+       configuration variable can be used to enable this option by
+       default.
+
+
 SEQUENCER SUBCOMMANDS
 ---------------------
 include::sequencer.txt[]

diff --git a/builtin/revert.c b/builtin/revert.c
index 51776abea63adf400e02339a716531feb26ad03e..ada51e46b9f2616ea4ac46a7c82bccca915462cb 100644 (file)
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -116,6 +116,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
                        N_("option for merge strategy"), option_parse_x),
                { OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
                  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+               OPT_BOOL(0, "reference", &opts->commit_use_reference,
+                        N_("use the 'reference' format to refer to commits")),
                OPT_END()
        };
        struct option *options = base_options;

diff --git a/sequencer.c b/sequencer.c
index a5f678f4529b1953af8dd7b6e168a4a03333e2e9..96fec6ef6d908d4966bccd3482a0d1eb76906da8 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -221,6 +221,9 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)
                return ret;
        }
 
+       if (!strcmp(k, "revert.reference"))
+               opts->commit_use_reference = git_config_bool(k, v);
+
        status = git_gpg_config(k, v, NULL);
        if (status)
                return status;
@@ -2059,6 +2062,20 @@ static int should_edit(struct replay_opts *opts) {
        return opts->edit;
 }
 
+static void refer_to_commit(struct replay_opts *opts,
+                           struct strbuf *msgbuf, struct commit *commit)
+{
+       if (opts->commit_use_reference) {
+               struct pretty_print_context ctx = {
+                       .abbrev = DEFAULT_ABBREV,
+                       .date_mode.type = DATE_SHORT,
+               };
+               format_commit_message(commit, "%h (%s, %ad)", msgbuf, &ctx);
+       } else {
+               strbuf_addstr(msgbuf, oid_to_hex(&commit->object.oid));
+       }
+}
+
 static int do_pick_commit(struct repository *r,
                          struct todo_item *item,
                          struct replay_opts *opts,
@@ -2167,14 +2184,20 @@ static int do_pick_commit(struct repository *r,
                base_label = msg.label;
                next = parent;
                next_label = msg.parent_label;
-               strbuf_addstr(&msgbuf, "Revert \"");
-               strbuf_addstr(&msgbuf, msg.subject);
-               strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
-               strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));
+               if (opts->commit_use_reference) {
+                       strbuf_addstr(&msgbuf,
+                               "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
+               } else {
+                       strbuf_addstr(&msgbuf, "Revert \"");
+                       strbuf_addstr(&msgbuf, msg.subject);
+                       strbuf_addstr(&msgbuf, "\"");
+               }
+               strbuf_addstr(&msgbuf, "\n\nThis reverts commit ");
+               refer_to_commit(opts, &msgbuf, commit);
 
                if (commit->parents && commit->parents->next) {
                        strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
-                       strbuf_addstr(&msgbuf, oid_to_hex(&parent->object.oid));
+                       refer_to_commit(opts, &msgbuf, parent);
                }
                strbuf_addstr(&msgbuf, ".\n");
        } else {

diff --git a/sequencer.h b/sequencer.h
index da64473636b4bcf991925daf90647503097e4c05..698599fe4e915ed4acbbf17995b0df201b723321 100644 (file)
--- a/sequencer.h
+++ b/sequencer.h
@@ -49,6 +49,7 @@ struct replay_opts {
        int reschedule_failed_exec;
        int committer_date_is_author_date;
        int ignore_date;
+       int commit_use_reference;
 
        int mainline;
 

diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 8617efaaf1e66f6f5d8246a817085ebd2d43c477..a386ae9e885db4c977c7b689b9c6d42be7764438 100755 (executable)
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -159,6 +159,7 @@ test_expect_success 'cherry-pick works with dirty renamed file' '
 '
 
 test_expect_success 'advice from failed revert' '
+       test_when_finished "git reset --hard" &&
        test_commit --no-tag "add dream" dream dream &&
        dream_oid=$(git rev-parse --short HEAD) &&
        cat <<-EOF >expected &&
@@ -174,4 +175,34 @@ test_expect_success 'advice from failed revert' '
        test_must_fail git revert HEAD^ 2>actual &&
        test_cmp expected actual
 '
+
+test_expect_success 'identification of reverted commit (default)' '
+       test_commit to-ident &&
+       test_when_finished "git reset --hard to-ident" &&
+       git checkout --detach to-ident &&
+       git revert --no-edit HEAD &&
+       git cat-file commit HEAD >actual.raw &&
+       grep "^This reverts " actual.raw >actual &&
+       echo "This reverts commit $(git rev-parse HEAD^)." >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'identification of reverted commit (--reference)' '
+       git checkout --detach to-ident &&
+       git revert --reference --no-edit HEAD &&
+       git cat-file commit HEAD >actual.raw &&
+       grep "^This reverts " actual.raw >actual &&
+       echo "This reverts commit $(git show -s --pretty=reference HEAD^)." >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'identification of reverted commit (revert.reference)' '
+       git checkout --detach to-ident &&
+       git -c revert.reference=true revert --no-edit HEAD &&
+       git cat-file commit HEAD >actual.raw &&
+       grep "^This reverts " actual.raw >actual &&
+       echo "This reverts commit $(git show -s --pretty=reference HEAD^)." >expect &&
+       test_cmp expect actual
+'
+
 test_done