Re: [PATCHv6 1/3] git-rebase -i: add command "drop" to remove a commit

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 23, 2015, 19:17 UTC
Message-ID: <CAPig+cRbtNV8YfvOQNPD_wEiyEkyDeTtbahYA-Vd9Q+9uv6z9w@mail.gmail.com>
In-Reply-To: <1637608183.731706.1435086079947.JavaMail.zimbra@ensimag.grenoble-inp.fr>

On Tue, Jun 23, 2015 at 3:01 PM, Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr> wrote:

Show 31 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > +test_rebase_end () {
>> > +       test_when_finished "git checkout master &&
>> > +       git branch -D $1 &&
>> > +       test_might_fail git rebase --abort" &&
>> > +       git checkout -b $1 master
>> > +}
>>
>> The way this is indented makes it difficult to see that lines 2 and 3
>> are continuations of 1. Perhaps format it like this instead?
>>
>>     test_rebase_end () {
>>         test_when_finished "git checkout master &&
>>             git branch -D $1 &&
>>             test_might_fail git rebase --abort" &&
>>         git checkout -b $1 master
>>     }
>
> I completely agree with you, moreover it was indented like this before.
> I'll change it in my local version for now.
>
> Ironically, it was modified after the following:
>
> Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > > +test_expect_success 'rebase -i respects rebase.missingCommitsCheck=ignore' '
>> > > +       test_config rebase.missingCommitsCheck ignore &&
>> > > +       test_when_finished "git checkout master &&
>> > > +               git branch -D tmp2" &&
>> >
>> > Strange indentation.

Clearly, that guy who made the "Strange indentation" review comment didn't know what he was talking about. ;-)

In that earlier review, I must have missed the fact that the quoted string spanned multiple lines, and, unfortunately, got too busy with other things to say "ah, the indentation makes perfect sense" when your response pointed out its true nature.

Matthieu's suggestion of formatting it like:

    test_when_finished "
        ... &&
        ...
    " &&

should help to avoid such misconceptions.