Re: [PATCH v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Aug 19, 2015, 16:10 UTC
- Message-ID
- <CAOLa=ZRBTrWa5EEOxa-Rf+J+8DWP7gSGGEYaG75EsR=A1DofRA@mail.gmail.com>
- In-Reply-To
- <CAOLa=ZSzXyQZJksNqyroU6Td+LG7ZRTF_WNNryusAGZxTYTmDg@mail.gmail.com>
On Wed, Aug 19, 2015 at 9:24 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 31 quoted lines
> On Wed, Aug 19, 2015 at 9:14 PM, Matthieu Moy > <Matthieu.Moy@grenoble-inp.fr> wrote: >> Karthik Nayak <karthik.188@gmail.com> 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