Re: [PATCH v15 05/13] ref-filter: implement an `align` atom

From: Karthik Nayak <karthik.188@gmail.com>
Date: Sep 2, 2015, 12:51 UTC
Message-ID: <CAOLa=ZRTDAD1wi8=Nz6uWfgS=Nsowf9CkDFsv+BC7b3L9w9Z=Q@mail.gmail.com>
In-Reply-To: <vpq613tusat.fsf@anie.imag.fr>

On Wed, Sep 2, 2015 at 2:11 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 32 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +test_expect_success 'alignment with format quote' '
>> +     cat >expect <<-EOF &&
>> +     refname is ${sq}           ${sq}\\${sq}${sq}master${sq}\\${sq}${sq}           ${sq}|
>> +     refname is ${sq}            ${sq}\\${sq}${sq}side${sq}\\${sq}${sq}            ${sq}|
>> +     refname is ${sq}          ${sq}\\${sq}${sq}odd/spot${sq}\\${sq}${sq}          ${sq}|
>> +     refname is ${sq}         ${sq}\\${sq}${sq}double-tag${sq}\\${sq}${sq}         ${sq}|
>> +     refname is ${sq}            ${sq}\\${sq}${sq}four${sq}\\${sq}${sq}            ${sq}|
>> +     refname is ${sq}            ${sq}\\${sq}${sq}one${sq}\\${sq}${sq}             ${sq}|
>> +     refname is ${sq}         ${sq}\\${sq}${sq}signed-tag${sq}\\${sq}${sq}         ${sq}|
>> +     refname is ${sq}           ${sq}\\${sq}${sq}three${sq}\\${sq}${sq}            ${sq}|
>> +     refname is ${sq}            ${sq}\\${sq}${sq}two${sq}\\${sq}${sq}             ${sq}|
>> +     EOF
>> +     git for-each-ref --shell --format="refname is %(align:30,middle)${sq}%(refname:short)${sq}%(end)|" >actual &&
>> +     test_cmp expect actual
>> +'
>
> Someone (Eric IIRC) suggested using double-quotes around the last
> argument of test_expect_success. Even though I'm the one who suggested
> this ${sq}, I have to agree with this suggestion. The result looks like
>
> test_expect_success 'alignment with quote' "
>         cat >expect <<-\EOF &&
>         refname is '               '\\'''master'\\''
>         ...
> "
>
> Because you used "" at the toplevel, ' is not a special character
> anymore. You do have to be careful with \\ though, but adding before EOF
> as I did should do the trick. Untested.
>

Oops! I missed that out, thanks for bringing that up again, will do it.

Show 7 quoted lines

> You don't have test for nested alignment with quotes. I think it
> deserves to be tested, if only to cast in stone that the current
> behavior is your intention.
>
> Perhaps just adding --shell to the test below would be OK to avoid the
> proliferation of tests :
>

Makes sense will add it to the following test.

Show 21 quoted lines

>> +test_expect_success 'nested alignment' '
>> +     cat >expect <<-\EOF &&
>> +     |         master               |
>> +     |           side               |
>> +     |       odd/spot               |
>> +     |     double-tag               |
>> +     |           four               |
>> +     |            one               |
>> +     |     signed-tag               |
>> +     |          three               |
>> +     |            two               |
>> +     EOF
>> +     git for-each-ref --format="|%(align:30,left)%(align:15,right)%(refname:short)%(end)%(end)|" >actual &&
>> +     test_cmp expect actual
>> +'
>> +
>>  test_done
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

-- 
Regards,
Karthik Nayak