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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 17, 2015, 18:22 UTC
Message-ID: <CAPig+cQiPKv15dGFskwxwoJceYevnNy9j-6oW7A3UySadOdBBw@mail.gmail.com>
In-Reply-To: <CAOLa=ZRfA-8_w6VKgWQsoL7TrdyjEq5LTHwas=_04tmx9MWhqA@mail.gmail.com>

On Mon, Aug 17, 2015 at 10:28 AM, Karthik Nayak <karthik.188@gmail.com> wrote:

Show 24 quoted lines

> On Mon, Aug 17, 2015 at 7:37 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>>> +{
>>> +       struct strbuf aligned = STRBUF_INIT;
>>> +       struct ref_formatting_state *return_to = state->prev;
>>> +       struct align *align = (struct align *)state->cb_data;
>>> +
>>> +       strbuf_utf8_align(&aligned, align->position, align->width, state->output.buf);
>>> +       strbuf_addbuf(&return_to->output, &aligned);
>>
>> Second, I realize that Junio suggested the 'return_to' idea, but it
>> seems like it could become overly painful since each handler of this
>> sort is going to have to perform the same manipulation to append its
>> collected output to its parent state's output. What if you instead
>> make it the responsibility of pop_state() to append the 'output' from
>> the state being popped to the "prev" state's 'output'? This way, it
>> happens automatically, thus reducing code in each individual handler,
>> and reducing the burden of having to keep writing the same code.
>
> Good question, what if we don't want to append to strbuf at all?
> For e.g., We were discussing an "%(if).....%(then)......%(end)"
> atom structure, here if the if condition isn't met we wouldn't want to
> append to the prev strbuf, hence I thought it's better if the handler
> decided whether or not to append to prev strbuf.

An %(if)/%(then)/%(end) construct should not present a problem. As long as the processing of the conditional ensures that the current state's 'output' contains no content, when pop_state() appends that (empty) content to the parent state's 'output', then the net result is exactly the desired behavior.

The question of "how" the conditional processing ensures that the current state's 'output' is empty when the %(if) condition is false is unimportant (for this discussion). It might be the case that it just doesn't collect any content at all for a false condition, or it collects it but throws it away before the state is popped. Either way, that's an implementation detail which needn't impact the decision to retire 'return_to' and instead make pop_state() responsible for appending the current state's output to the parent state's.