Re: [PATCH v6 01/10] ref-filter: introduce 'ref_formatting_state'

Eric Sunshine [off-list ref] writes:

quoted

@@ -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().