From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Wed, 19 Aug 2015 15:44:29 GMT
Subject: Re: [PATCH v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery
Message-ID: <vpqa8tn2sgy.fsf@anie.imag.fr>
In-Reply-To: <CAOLa=ZTy8QO=H9g9g3SKU4HaG=qg71GidGuXKLXNgu7Su3FQ9A@mail.gmail.com>

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.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/