From: Josef Kufner <josef@kufner.cz>
Date: Fri, 11 Sep 2015 20:25:30 GMT
Subject: Re: [PATCH] Add tests for "Pass graph width to pretty formatting, so '%>|' can work properly"
Message-ID: <55F338BA.6070201@kufner.cz>
In-Reply-To: <CAPig+cRkSRSC0eaBencw8cFrd64sUBQE44aSJ0T2WyPg2JTFCw@mail.gmail.com>

Eric Sunshine wrote, on 11.9.2015 21:37:
> On Fri, Sep 11, 2015 at 1:50 PM, Josef Kufner <josef@kufner.cz> wrote:
>> ---
> 
> Missing sign-off. Or is this intended to be concatenated with the
> patch you sent earlier?

Just forgot to add it. Fixed patch on the way.


>> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
>> index 7398605..3358837 100755
>> --- a/t/t4205-log-pretty-formats.sh
>> +++ b/t/t4205-log-pretty-formats.sh
>> @@ -319,6 +319,18 @@ EOF
>>         test_cmp expected actual
>>  '
>>
>> +# Note: Space between 'message' and 'two' should be in the same column as in previous test.
>> +test_expect_success 'right alignment formatting at the nth column with --graph. i18n.logOutputEncoding' '
>> +       git -c i18n.logOutputEncoding=$test_encoding log --graph --pretty="tformat:%h %>|(40)%s" >actual &&
>> +       qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
> 
> You don't seem to be taking advantage of qz_to_tab_space's
> transliteration of Q to tab and Z to space, so s/qz_to_tab_space/cat/
> would make the code clearer.

I've copied another test which tests the padding without --graph and
added it to test the new case. I have no idea what qz_to_tab_space can
do. If you wish some clean up, it should be done on older tests too :)