Re: [PATCH] sequencer: beautify subject of reverts of reverts
From: Junio C Hamano <hidden>
Date: 2023-03-28 14:04:21
Oswald Buddenhagen [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/sequencer.c b/sequencer.c index 3be23d7ca2..853b4ed334 100644 --- a/sequencer.c +++ b/sequencer.c@@ -2234,6 +2234,9 @@ 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 (starts_with(msg.subject, "Revert \"")) { + strbuf_addstr(&msgbuf, "Reapply "); + strbuf_addstr(&msgbuf, msg.subject + 7); } else { strbuf_addstr(&msgbuf, "Revert \""); strbuf_addstr(&msgbuf, msg.subject);
Two and half comments: * The hard-coded +7 looks fragile. Perhaps use skip_prefix? * During transition to introduce a new version of Git with this feature, when reverting an existing revert of a revert, care must be taken. Such a commit would begin as Revert "Revert ... and turning it to Reapply "Revert ... may not be a good way to label such a reversion of a double revert without end-user confusion. As it is very likely that such a reversion commit was created by existing versions of Git, the easiest and least confusing way out would be to notice and refrain from touching such a commit. * The change lacks tests. Removal of hardcoded +7 (i.e. the first point) may look like this. sequencer.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git i/sequencer.c w/sequencer.c
index 853b4ed334..520113ec63 100644
--- i/sequencer.c
+++ w/sequencer.c@@ -2227,6 +2227,8 @@ static int do_pick_commit(struct repository *r, */ if (command == TODO_REVERT) { + const char *original_title; + base = commit; base_label = msg.label; next = parent;
@@ -2234,9 +2236,9 @@ 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 (starts_with(msg.subject, "Revert \"")) { + } else if (skip_prefix(msg.subject, "Revert \"", &original_title)) { - strbuf_addstr(&msgbuf, "Reapply "); + strbuf_addstr(&msgbuf, "Reapply \""); - strbuf_addstr(&msgbuf, msg.subject + 7); + strbuf_addstr(&msgbuf, original_title); } else { strbuf_addstr(&msgbuf, "Revert \""); strbuf_addstr(&msgbuf, msg.subject);