Subject: Re: [PATCH v13 04/12] ref-filter: implement an `align` atom

From: Junio C Hamano <[email-masked]>
Date: Mon, 24 Aug 2015 15:15:55 -0700

Junio C Hamano <[email-masked]> writes:

> Karthik Nayak <[email-masked]> 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.