[PATCH v6 0/10] port tag.c to use ref-filter APIs

[Karthik Nayak <karthik.188@gmail.com>]

This is part of my GSoC project to unify git tag -l, git branch -l,
git for-each-ref.  This patch series is continued from: Git (next)
https://github.com/git/git/commit/bf5418f49ff0cebc6e5ce04ad1417e1a47c81b61

Version 5 can be found here:
http://article.gmane.org/gmane.comp.version-control.git/274650

Changes in v6:
* Change the flow of commits, introduce rest_formatting_state.
* Rename
ref_formatting -> apply_formatting_state
apply_pseudo_state -> store_formatting_state
* Remove the patch for making color a pseudo atom, and rename
pseudo_atom to modifier_atom.
* Small grammatical changes.

Side Note: --format="%(padright:X)" applies to the next available atom
and not to the next span. I find this more accurate as I don't see why
we'd want to pad something of known length. But its up for discussion
:D

Interdiff:
diff --git a/ref-filter.c b/ref-filter.c
index 597b189..0a34924 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -664,8 +664,6 @@ static void populate_value(struct ref_array_item *ref)
             if (color_parse(name + 6, color) < 0)
                 die(_("unable to parse format"));
             v->s = xstrdup(color);
-            v->pseudo_atom = 1;
-            v->color = 1;
             continue;
         } else if (!strcmp(name, "flag")) {
             char buf[256], *cp = buf;
@@ -702,7 +700,7 @@ static void populate_value(struct ref_array_item *ref)
             if (strtoul_ui(valp, 10, (unsigned int *)&v->ul))
                 die(_("positive integer expected after ':' in padright:%u\n"),
                     (unsigned int)v->ul);
-            v->pseudo_atom = 1;
+            v->modifier_atom = 1;
             v->pad_to_right = 1;
             continue;
         } else
@@ -973,8 +971,8 @@ static int match_pattern(const char **patterns,
const char *refname)
 /*
  * Return 1 if the refname matches one of the patterns, otherwise 0.
  * A pattern can be path prefix (e.g. a refname "refs/heads/master"
- * matches a pattern "refs/heads/") or a wildcard (e.g. the same ref
- * matches "refs/heads/m*",too).
+ * matches a pattern "refs/heads/" but not "refs/heads/m") or a
+ * wildcard (e.g. the same ref matches "refs/heads/m*", too).
  */
 static int match_name_as_path(const char **pattern, const char *refname)
 {
@@ -1246,14 +1244,9 @@ void ref_array_sort(struct ref_sorting
*sorting, struct ref_array *array)
     qsort(array->items, array->nr, sizeof(struct ref_array_item *),
compare_refs);
 }

-static void ref_formatting(struct ref_formatting_state *state,
-               struct atom_value *v, struct strbuf *value)
+static void apply_formatting_state(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;
-    }
     if (state->pad_to_right) {
         if (!is_utf8(v->s))
             strbuf_addf(value, "%-*s", state->pad_to_right, v->s);
@@ -1263,15 +1256,22 @@ static void ref_formatting(struct
ref_formatting_state *state,
         }
         return;
     }
-    strbuf_addf(value, "%s", v->s);
+
+    strbuf_addstr(value, v->s);
 }

-static void print_value(struct ref_formatting_state *state, struct
atom_value *v)
+static void print_value(struct atom_value *v, struct
ref_formatting_state *state)
 {
     struct strbuf value = STRBUF_INIT;
     struct strbuf formatted = STRBUF_INIT;

-    ref_formatting(state, v, &value);
+    /*
+     * Some (pesudo) atoms have no immediate side effect, but only
+     * affect the next atom. Store the relevant information from
+     * these atoms in the 'state' variable for use when displaying
+     * the next atom.
+     */
+    apply_formatting_state(state, v, &value);

     switch (state->quote_style) {
     case QUOTE_NONE:
@@ -1292,8 +1292,8 @@ static void print_value(struct
ref_formatting_state *state, struct atom_value *v
     }
     if (state->quote_style != QUOTE_NONE)
         fputs(formatted.buf, stdout);
-    strbuf_release(&formatted);
     strbuf_release(&value);
+    strbuf_release(&formatted);
 }

 static int hex1(char ch)
@@ -1334,13 +1334,18 @@ static void emit(const char *cp, const char *ep)
     }
 }

-static void apply_pseudo_state(struct ref_formatting_state *state,
-                   struct atom_value *v)
+static void store_formatting_state(struct ref_formatting_state *state,
+                   struct atom_value *atomv)
 {
-    if (v->color)
-        state->color = (char *)v->s;
-    if (v->pad_to_right)
-        state->pad_to_right = v->ul;
+    if (atomv->pad_to_right)
+        state->pad_to_right = atomv->ul;
+}
+
+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;
 }

 /*
@@ -1402,10 +1407,12 @@ void show_ref_array_item(struct ref_array_item
*info, const char *format,
         if (cp < sp)
             emit(cp, sp);
         get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
-        if (atomv->pseudo_atom)
-            apply_pseudo_state(&state, atomv);
-        else
-            print_value(&state, atomv);
+        if (atomv->modifier_atom)
+            store_formatting_state(&state, atomv);
+        else {
+            print_value(atomv, &state);
+            reset_formatting_state(&state);
+        }
     }
     if (*cp) {
         sp = cp + strlen(cp);
@@ -1418,7 +1425,7 @@ void show_ref_array_item(struct ref_array_item
*info, const char *format,
         if (color_parse("reset", color) < 0)
             die("BUG: couldn't parse 'reset' as a color");
         resetv.s = color;
-        print_value(&state, &resetv);
+        print_value(&resetv, &state);
     }
     if (lines > 0) {
         struct object_id oid;
diff --git a/ref-filter.h b/ref-filter.h
index a27745f..fcf469e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -20,14 +20,12 @@
 struct atom_value {
     const char *s;
     unsigned long ul; /* used for sorting when not FIELD_STR */
-    unsigned int pseudo_atom : 1, /*  atoms which aren't placeholders
for ref attributes */
-        color : 1,
+    unsigned int modifier_atom : 1, /*  atoms which act as modifiers
for the next atom */
         pad_to_right : 1;
 };

 struct ref_formatting_state {
     int quote_style;
-    char *color;
     unsigned int pad_to_right;
 };

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index de872db..68688a9 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -83,17 +83,17 @@ test_expect_success 'filtering with --contains' '

 test_expect_success 'padding to the right using `padright`' '
     cat >expect <<-\EOF &&
-    refs/heads/master        |
-    refs/heads/side          |
-    refs/odd/spot            |
-    refs/tags/double-tag     |
-    refs/tags/four           |
-    refs/tags/one            |
-    refs/tags/signed-tag     |
-    refs/tags/three          |
-    refs/tags/two            |
+    refs/heads/master|refs/heads/master        |refs/heads/master|
+    refs/heads/side|refs/heads/side          |refs/heads/side|
+    refs/odd/spot|refs/odd/spot            |refs/odd/spot|
+    refs/tags/double-tag|refs/tags/double-tag     |refs/tags/double-tag|
+    refs/tags/four|refs/tags/four           |refs/tags/four|
+    refs/tags/one|refs/tags/one            |refs/tags/one|
+    refs/tags/signed-tag|refs/tags/signed-tag     |refs/tags/signed-tag|
+    refs/tags/three|refs/tags/three          |refs/tags/three|
+    refs/tags/two|refs/tags/two            |refs/tags/two|
     EOF
-    git for-each-ref --format="%(padright:25)%(refname)|" >actual &&
+    git for-each-ref
--format="%(refname)%(padright:25)|%(refname)|%(refname)|" >actual &&
     test_cmp expect actual
 '

-- 
Regards,
Karthik Nayak