On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> Introduce the 'used_array' structure which would replace the existing
I guess you meant s/used_array/used_atom/ or something?
Also, s/which would/to/
> implementation of 'used_array' (which a list of atoms). This helps us
s/which a/which is a/
> parse atom's before hand and store required details into the
s/atom's/atoms/
s/before hand/beforehand/
> 'used_array' for future usage.
>
> Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -16,6 +16,23 @@
> +/*
> + * An atom is a valid field atom listed below, possibly prefixed with
> + * a "*" to denote deref_tag().
> + *
> + * We parse given format string and sort specifiers, and make a list
> + * of properties that we need to extract out of objects. ref_array_item
> + * structure will hold an array of values extracted that can be
> + * indexed with the "atom number", which is an index into this
> + * array.
> + */
> +static struct used_atom {
> + const char *str;
This is really the atom's name, isn't it? If so, perhaps "name" would
be a better field name.
> + cmp_type type;
> +} *used_atom;
> +static int used_atom_cnt, need_tagged, need_symref;
> +static int need_color_reset_at_eol;
> +
> static struct {
> const char *name;
> cmp_type cmp_type;
> @@ -93,21 +110,6 @@ struct atom_value {
> };
>
> /*
> - * An atom is a valid field atom listed above, possibly prefixed with
> - * a "*" to denote deref_tag().
> - *
> - * We parse given format string and sort specifiers, and make a list
> - * of properties that we need to extract out of objects. ref_array_item
> - * structure will hold an array of values extracted that can be
> - * indexed with the "atom number", which is an index into this
> - * array.
> - */
> -static const char **used_atom;
> -static cmp_type *used_atom_type;
> -static int used_atom_cnt, need_tagged, need_symref;
> -static int need_color_reset_at_eol;
You're moving this block of declarations up above the valid_atom[]
array because the previous patch added a new field named "parser" to
valid_atom[] which references 'struct used_atom' added by this patch
(2). I wonder if this movement should be done as a separate
preparatory patch to make it easier to review since, as it stands, the
reviewer has to read much more carefully to detect changes in the
moved block.
> -/*
> * Used to parse format string and sort specifiers
> */
> int parse_ref_filter_atom(const char *atom, const char *ep)
> @@ -787,7 +788,8 @@ static void populate_value(struct ref_array_item *ref)
>
> /* Fill in specials first */
> for (i = 0; i < used_atom_cnt; i++) {
> - const char *name = used_atom[i];
> + struct used_atom *atom = &used_atom[i];
> + const char *name = atom->str;
Why not just:
const char *name = used_atom[i].str;
?
> struct atom_value *v = &ref->value[i];
> int deref = 0;
> const char *refname;
--
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