Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes: > Junio C Hamano <gitster@xxxxxxxxx> 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. -- 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