Re: [PATCH v4 8/9] rebase -i: teach --autosquash to work with amend!

On Tue, 2 Feb 2021 at 08:50, Eric Sunshine [off-list ref] wrote:

On Fri, Jan 29, 2021 at 1:25 PM Charvi Mendiratta [off-list ref] wrote:

quoted

If the commit subject starts with "amend!" then rearrange it like a
"fixup!" commit and replace `pick` command with `fixup -C` command,
which is used to fixup up the content if any and replaces the original
commit message with amend! commit's message.

Signed-off-by: Charvi Mendiratta <redacted>
---
 sequencer.c                     | 23 ++++++++++++++++-------
 t/t3437-rebase-fixup-options.sh | 12 ++++++++++++
 2 files changed, 28 insertions(+), 7 deletions(-)

Is this new behavior of recognizing "amend!" in the subject documented
anywhere? I checked the documentation patch [9/9] but didn't see any
mention of it.

No this patch series does not include that but included in the patch series of
amend! commit implementation that is not sent to the mailing list yet.
(Apology for the confusion)

quoted

diff --git a/sequencer.c b/sequencer.c

@@ -5662,6 +5662,12 @@ static int subject2item_cmp(const void *fndata,
+static inline int skip_fixup_amend_squash(const char *subject, const char **p) {
+       return skip_prefix(subject, "fixup! ", p) ||
+              skip_prefix(subject, "amend! ", p) ||
+              skip_prefix(subject, "squash! ", p);
+}

While the function name skip_fixup_amend_squash() may be accurate, it
won't scale well. What happens when additional fixup-like prefixes are
added in the future? Does the function name get extended to name them,
as well? How about choosing a more generic, yet still meaningful,
function name which doesn't suffer from this scaling problem. Perhaps
skip_fixupish() or fix_squashlike() or something.

I agree completely, and will change the function name  to skip_fixupish().

Also, making this function `inline` seems like a case of premature optimization.

Okay, will remove `inline`.

Thanks and Regards,
Charvi