Karthik Nayak <karthik....@gmail.com> writes:
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1195,6 +1197,11 @@ void ref_array_sort(struct ref_sorting *sorting,
> struct ref_array *array)
> static void ref_formatting(struct ref_formatting_state *state,
> struct atom_value *v, struct strbuf *value)
> {
> + if (state->color) {
> + strbuf_addstr(value, state->color);
> + free(state->color);
> + state->color = NULL;
> + }
> strbuf_addf(value, "%s", v->s);
> }
>
> @@ -1266,6 +1273,13 @@ static void emit(const char *cp, const char *ep)
> }
> }
>
> +static void apply_pseudo_state(struct ref_formatting_state *state,
> + struct atom_value *v)
> +{
> + if (v->color)
> + state->color = (char *)v->s;
> +}
> +
> void show_ref_array_item(struct ref_array_item *info, const char *format,
> int quote_style)
> {
> const char *cp, *sp, *ep;
It's not clear enough in the code and history that these these two
functions are symmetrical.
You can find better names. 'apply_pseudo_state' seems wrong it two ways:
it does not _apply_ the state, but it stores it. And it's a "pseudo-atom
related state", but not a "pseudo-state".
How about
ref_formatting -> apply_formatting_state
apply_pseudo_state -> store_formatting_state
?
Actually, I would even call these functions right from
show_ref_array_item, so that the result look like this:
if (atomv->pseudo_atom)
store_formatting_state(&state, atomv);
else {
apply_formatting_state(&state, atomv);
reset_formatting_state(&state);
print_value(&state, atomv);
}
In the history, if you are to introduce a dumb version of ref_formatting
in PATCH 1, I think you should also introduce a dumb (actually, totally
empty) version of apply_pseudo_state. Then, further patches would just
add a few lines in each function, and ...
> @@ -1281,7 +1295,10 @@ void show_ref_array_item(struct ref_array_item *info,
> const char *format, int qu
> if (cp < sp)
> emit(cp, sp);
> get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep),
> &atomv);
> - print_value(&state, atomv);
> + if (atomv->pseudo_atom)
> + apply_pseudo_state(&state, atomv);
> + else
> + print_value(&state, atomv);
> }
... this hunk would belong to PATCH 1.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
