On Wed, Aug 19, 2015 at 9:24 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote: > On Wed, Aug 19, 2015 at 9:14 PM, Matthieu Moy > <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: >> Karthik Nayak <karthik.188@xxxxxxxxx> writes: >> >>>> --- a/ref-filter.c >>>> +++ b/ref-filter.c >>>> @@ -155,6 +155,8 @@ static void push_new_state(struct ref_formatting_state **stack) >>>> >>>> strbuf_init(&s->output, 0); >>>> s->prev = *stack; >>>> + if (*stack) >>>> + s->quote_style = (*stack)->quote_style; >>>> *stack = s; >>>> } >>>> >>>> >>> >>> This seems about right, why do you think it's a stupid fix? >> >> If you have a stack of N elemments, why replicate a field N times if all >> the N instances always have the same value? >> >> There's nothing to be pushed or poped with quote_style, so having it in >> the stack is confusing to the reader (one has to infer the property "all >> instances have the same value" by reading the code instead of having >> just one variable), and error-prone for the author: you already got >> it wrong once. > > Thats also there, I'll guess it makes more sense to remove it from > ref_formatting_state. > Speaking of quote_value, The quote doesn't work well with color's for e.g. git for-each-ref --shell --format="%(color:green)%(refname)" '''refs/heads/allow-unknown-type''' Seems like an simple fix, probably after GSoC I'll do this :) -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html