Re: [PATCH v13 04/12] ref-filter: implement an `align` atom
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 25, 2015, 17:52 UTC
- Message-ID
- <xmqqpp2bz266.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <vpqk2sj7tlb.fsf@anie.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 21 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> You can see that I expected that "if !state.stack->prev" check to be
>> inside append_atom(), and I would imagine future readers would have
>> the same expectation when reading this code. I.e.
>>
>> append_atom(struct atom_value *v, struct ref_f_s *state)
>> {
>> if (state->stack->prev)
>> strbuf_addstr(&state->stack->output, v->s);
>> else
>> quote_format(&state->stack->output, v->s, state->quote_style);
>> }
>>
>> The end result may be the same,
>
> There's another call to append_atom() when inserting the "reset color at
> end of line if needed", so moving this if inside append_atom means we
> would do the check also for the reset color. It would not change the
> behavior (by construction, we insert it only when the stack has only the
> initial element), so it's OK.Thanks for checking---I did overlook that other callsite and did not check if the suggested change was sensible there.