From: Karthik Nayak <karthik.188@gmail.com>
Date: Wed, 12 Aug 2015 13:26:40 GMT
Subject: Re: [PATCH v10 03/13] ref-filter: introduce ref_formatting_state
Message-ID: <CAOLa=ZTKvNq0meFfHA8CjWsXOznmW4QUudQnizypzThhBQvFZg@mail.gmail.com>
In-Reply-To: <xmqqd1ytsnfx.fsf@gitster.dls.corp.google.com>

On Tue, Aug 11, 2015 at 11:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>               get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
>> -             format_quote_value(atomv, quote_style, &output);
>> +             set_formatting_state(atomv, &state);
>> +             format_quote_value(atomv, &state);
>> +             perform_state_formatting(&state, &final_buf);
>>       }
>>       if (*cp) {
>>               sp = cp + strlen(cp);
>> -             append_non_atom(cp, sp, &output);
>> +             append_non_atom(cp, sp, &state);
>> +             perform_state_formatting(&state, &final_buf);
>>       }
>
> With the two helpers being very sketchy at this stage, it is very
> hard to judge if they make sense.  At the conceptual level, I can
> see that set-formatting-state is to allow an atom to affect the
> state before the value of the atom is emitted into the buffer.
> I cannot tell what perform-state-formatting is meant to do from
> these call sites.


True, set formatting state is to ensure that the state is manipulated for
a given atom.

perform_state_formatting() is meant to act on the state set by the atom,
It performs formatting based on the state values, here is just copies the
strbuf set within the state to the final_buf.

-- 
Regards,
Karthik Nayak