Re: [PATCH 2/4] t7004-tag.sh: re-arrange git tag comment for clarity

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 14, 2010, 17:57 UTC
Message-ID: <AANLkTik7E6B0oLnSjqw05NhhbSrz51k+O272iFjr1swk@mail.gmail.com>
In-Reply-To: <20101114173241.GB26459@burratino>

On Sun, Nov 14, 2010 at 18:32, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 21 quoted lines

> Ævar Arnfjörð Bjarmason wrote:
>
>>                           I used ! instead of test_must_fail so that
>> the GIT_EDITOR variable was only used in this command invocation,
>
>        (
>                GIT_EDITOR=cat &&
>                export GIT_EDITOR &&
>                test_must_fail ...
>        )
>
> ?
>
>>                                                                   and
>> because the surrounding tests use this style.
>
> Oh, okay.  I had the test_must_fail version sitting in my local tree
> as part of the unsent part of the &&-chaining series
> http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160419
>
> I am happy to see this fixed sooner.

Better to fix it up later IMO in something like your series. One surgery at a time.

Show 26 quoted lines

> [...]
>> +test_expect_success \
>> +     'message in editor has initial comment: first line' '
>>       # check the first line --- should be empty
>> -     first=$(sed -e 1q <actual) &&
>> -     test -z "$first" &&
>> +     echo >first.expect &&
>> +     sed -e 1q <actual >first.actual &&
>> +     test_cmp first.expect first.actual
>> +'
>> +
>> +test_expect_success \
>> +     'message in editor has initial comment: remainder' '
>>       # remove commented lines from the remainder -- should be empty
>> -     rest=$(sed -e 1d -e '/^#/d' <actual) &&
>> -     test -z "$rest"
>> +     >rest.expect
>> +     sed -e 1d -e '/^#/d' <actual >rest.actual &&
>> +     test_cmp rest.expect rest.actual
>>  '
>
> Nice.  Note that you've changed the semantics of the test here
> (probably for the better): the $() backquoting operator strips
> trailing newlines, while your sed expression does not.
>
> I assume you've run the tests and made sure they still pass.

Yeah, I've been running all these (also the others in the series) for months on several different platforms with different options. They all work.

> For what it's worth,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.