From: Karthik Nayak <karthik.188@gmail.com>
Date: Tue, 25 Aug 2015 13:28:56 GMT
Subject: Re: [PATCH v13 04/12] ref-filter: implement an `align` atom
Message-ID: <CAOLa=ZQfVbyX=7LhTbsFRyWY180zyG7=aeYz6zOjnwd=yUUivw@mail.gmail.com>
In-Reply-To: <xmqqfv382uzo.fsf@gitster.dls.corp.google.com>

On Tue, Aug 25, 2015 at 3:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> +static void end_atom_handler(struct atom_value *atomv, struct
>>> ref_formatting_state *state)
>>> +{
>>> +    struct ref_formatting_stack *current = state->stack;
>>> +    struct strbuf s = STRBUF_INIT;
>>> +
>>> +    if (!current->at_end)
>>> +            die(_("format: `end` atom used without a supporting atom"));
>>> +    current->at_end(current);
>>> +    /*
>>> +     * Whenever we have more than one stack element that means we
>>> +     * are using a certain modifier atom. In that case we need to
>>> +     * perform quote formatting.
>>> +     */
>>> +    if (!state->stack->prev->prev) {
>>
>> The comment and the condition seem to be saying opposite things.
>> The code says "If the stack only has one prev that is the very
>> initial one, then we do the quoting, i.e. the result of expanding
>> the enclosed string in %(start-something)...%(end) is quoted only
>> when that appears at the top level", which feels more correct...
>
> As this already allows us to use nested construct, I think we would
> want to have test that uses
>
>     %(align:30,left)%(align:20,right)%(refname:short)%(end)%(end)
>
> or something like that ;-)
>
> Very nice.

Ok, will do that :)

-- 
Regards,
Karthik Nayak