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, 15:54 UTC
- Message-ID
- <CAOLa=ZSzXyQZJksNqyroU6Td+LG7ZRTF_WNNryusAGZxTYTmDg@mail.gmail.com>
- In-Reply-To
- <vpqa8tn2sgy.fsf@anie.imag.fr>
On Wed, Aug 19, 2015 at 9:14 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
Show 25 quoted lines
> 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.
-- Regards, Karthik Nayak