Skip to content

Commit 43966ab

Browse files
committed
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 <[email protected]>
1 parent 6cd33dc commit 43966ab

File tree

6 files changed

+74
-5
lines changed

6 files changed

+74
-5
lines changed

Documentation/config/revert.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
revert.reference::
2+
Setting this variable to true makes `git revert` to behave
3+
as if the `--reference` option is given.

Documentation/git-revert.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,15 @@ effect to your index in a row.
117117
Allow the rerere mechanism to update the index with the
118118
result of auto-conflict resolution if possible.
119119

120+
--reference::
121+
Instead of starting the body of the log message with "This
122+
reverts <full object name of the commit being reverted>.",
123+
refer to the commit using "--pretty=reference" format
124+
(cf. linkgit:git-log[1]). The `revert.reference`
125+
configuration variable can be used to enable this option by
126+
default.
127+
128+
120129
SEQUENCER SUBCOMMANDS
121130
---------------------
122131
include::sequencer.txt[]

builtin/revert.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
116116
N_("option for merge strategy"), option_parse_x),
117117
{ OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
118118
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
119+
OPT_BOOL(0, "reference", &opts->commit_use_reference,
120+
N_("use the 'reference' format to refer to commits")),
119121
OPT_END()
120122
};
121123
struct option *options = base_options;

sequencer.c

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,9 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)
221221
return ret;
222222
}
223223

224+
if (!strcmp(k, "revert.reference"))
225+
opts->commit_use_reference = git_config_bool(k, v);
226+
224227
status = git_gpg_config(k, v, NULL);
225228
if (status)
226229
return status;
@@ -2059,6 +2062,20 @@ static int should_edit(struct replay_opts *opts) {
20592062
return opts->edit;
20602063
}
20612064

2065+
static void refer_to_commit(struct replay_opts *opts,
2066+
struct strbuf *msgbuf, struct commit *commit)
2067+
{
2068+
if (opts->commit_use_reference) {
2069+
struct pretty_print_context ctx = {
2070+
.abbrev = DEFAULT_ABBREV,
2071+
.date_mode.type = DATE_SHORT,
2072+
};
2073+
format_commit_message(commit, "%h (%s, %ad)", msgbuf, &ctx);
2074+
} else {
2075+
strbuf_addstr(msgbuf, oid_to_hex(&commit->object.oid));
2076+
}
2077+
}
2078+
20622079
static int do_pick_commit(struct repository *r,
20632080
struct todo_item *item,
20642081
struct replay_opts *opts,
@@ -2167,14 +2184,20 @@ static int do_pick_commit(struct repository *r,
21672184
base_label = msg.label;
21682185
next = parent;
21692186
next_label = msg.parent_label;
2170-
strbuf_addstr(&msgbuf, "Revert \"");
2171-
strbuf_addstr(&msgbuf, msg.subject);
2172-
strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
2173-
strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));
2187+
if (opts->commit_use_reference) {
2188+
strbuf_addstr(&msgbuf,
2189+
"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
2190+
} else {
2191+
strbuf_addstr(&msgbuf, "Revert \"");
2192+
strbuf_addstr(&msgbuf, msg.subject);
2193+
strbuf_addstr(&msgbuf, "\"");
2194+
}
2195+
strbuf_addstr(&msgbuf, "\n\nThis reverts commit ");
2196+
refer_to_commit(opts, &msgbuf, commit);
21742197

21752198
if (commit->parents && commit->parents->next) {
21762199
strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
2177-
strbuf_addstr(&msgbuf, oid_to_hex(&parent->object.oid));
2200+
refer_to_commit(opts, &msgbuf, parent);
21782201
}
21792202
strbuf_addstr(&msgbuf, ".\n");
21802203
} else {

sequencer.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ struct replay_opts {
4949
int reschedule_failed_exec;
5050
int committer_date_is_author_date;
5151
int ignore_date;
52+
int commit_use_reference;
5253

5354
int mainline;
5455

t/t3501-revert-cherry-pick.sh

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ test_expect_success 'cherry-pick works with dirty renamed file' '
159159
'
160160

161161
test_expect_success 'advice from failed revert' '
162+
test_when_finished "git reset --hard" &&
162163
test_commit --no-tag "add dream" dream dream &&
163164
dream_oid=$(git rev-parse --short HEAD) &&
164165
cat <<-EOF >expected &&
@@ -174,4 +175,34 @@ test_expect_success 'advice from failed revert' '
174175
test_must_fail git revert HEAD^ 2>actual &&
175176
test_cmp expected actual
176177
'
178+
179+
test_expect_success 'identification of reverted commit (default)' '
180+
test_commit to-ident &&
181+
test_when_finished "git reset --hard to-ident" &&
182+
git checkout --detach to-ident &&
183+
git revert --no-edit HEAD &&
184+
git cat-file commit HEAD >actual.raw &&
185+
grep "^This reverts " actual.raw >actual &&
186+
echo "This reverts commit $(git rev-parse HEAD^)." >expect &&
187+
test_cmp expect actual
188+
'
189+
190+
test_expect_success 'identification of reverted commit (--reference)' '
191+
git checkout --detach to-ident &&
192+
git revert --reference --no-edit HEAD &&
193+
git cat-file commit HEAD >actual.raw &&
194+
grep "^This reverts " actual.raw >actual &&
195+
echo "This reverts commit $(git show -s --pretty=reference HEAD^)." >expect &&
196+
test_cmp expect actual
197+
'
198+
199+
test_expect_success 'identification of reverted commit (revert.reference)' '
200+
git checkout --detach to-ident &&
201+
git -c revert.reference=true revert --no-edit HEAD &&
202+
git cat-file commit HEAD >actual.raw &&
203+
grep "^This reverts " actual.raw >actual &&
204+
echo "This reverts commit $(git show -s --pretty=reference HEAD^)." >expect &&
205+
test_cmp expect actual
206+
'
207+
177208
test_done

0 commit comments

Comments
 (0)