Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>> @@ -1254,9 +1273,26 @@ static void emit(const char *cp, const char *ep)
>> +static void reset_formatting_state(struct ref_formatting_state *state)
>> +{
>> + int quote_style = state->quote_style;
>> + memset(state, 0, sizeof(*state));
>> + state->quote_style = quote_style;
>
> I wonder if this sledge-hammer approach of saving one or two values
> before clearing the entire 'ref_formatting_state' and then restoring
> the saved values will scale well. Would it be better for this to just
> individually reset the fields which need resetting and not touch those
> that don't?
>
> Also, the fact that quote_style has to be handled specially may be an
> indication that it doesn't belong in this structure grouped with the
> other modifiers or that you need better classification within the
> structure.
Actually, I think it is wrong to have this function in the first
place. It is a sign that the caller is doing too little before
calling this function.
If the act of printing an atom uses the formatting state that says
"next one needs X", then it is responsible to clear that "next one
needs X" part of the state, as it is the one who consumed that
state. E.g. if it used to say "next one needs to be padded to the
right" before entering print_value(), then the function did that
"padded output", then the "next one needs to be padded to the
right" should be cleared inside print_value().
And with that arrangement, together with calling emit() with
formatting state, %(color:blue) can be handled as a normal part of
the formatting state mechanism. The pseudo/modifier atom should
update the state to "Start printing in blue", and either emit() or
print_value(), whichever is called first, would notice that state,
does what was requested, and flip that bit down (because we are
already printing in "blue" so the next output function does not have
to do the "blue" thing again).
--
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