Subject: Re: [PATCH 2/2] commit: add support for --fixup <commit> -m"<extra message>"

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 20 Dec 2017 22:40:53 +0100

On Wed, Dec 20 2017, Eric Sunshine jotted:

> On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason
> <[email-masked]> wrote:
>> Add support for supplying the -m option with --fixup. Doing so has
>> errored out ever since --fixup was introduced. Before this, the only
>> way to amend the fixup message while committing was to use --edit and
>> amend it in the editor.
>
> I can't tell, based upon this description, if '-m<msg> --edit' is a
> valid combination and, if so, does it correctly open the editor after
> appending <msg>?

Yes, that's works as expected for all the options, and this doesn't
break that.

>> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
>> ---
>> diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
>> @@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct one-line commit message' '
>>         commit_msg_is "fixup! target message subject line"
>>  '
>>
>> +test_expect_success 'commit --fixup -m"something" -m"extra"' '
>> +       commit_for_rebase_autosquash_setup &&
>> +       git commit --fixup HEAD~1 -m"something" -m"extra" &&
>> +       commit_msg_is "fixup! target message subject linesomething
>> +
>> +extra"
>> +'
>
> Hmm, so the first -m appended to the "fixup!" line, but the second -m
> appended after a blank line? That doesn't seem very intuitive.
>
> Also, doesn't the text following "fixup!" need to exactly match the
> summary line of the commit message in order for "git rebase -i
> --autosquash" to work? Am I overlooking something obvious?

It does the right thing and it's actually
"$fixup_line\n\n$first_m\n\n$second_m" etc. It's just that this
commit_msg_is function is testing against the "%s%b" format, so the
first line of the body comes right after the subject.

Thanks for the review of both patches. I'll clarify the points raised in
commit message for v2 pending further feedback.