Re: [PATCH v3 07/23] cat-file: start migrating formatting to ref-filter

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Move mark_atom_in_object_info() from cat-file to ref-filter and
> start using it in verify_ref_format().
> It also means that we start reusing verify_ref_format() in cat-file.
> 
> Start from simple moving of mark_atom_in_object_info(),
> it would be removed later by integrating all needed processes into
> ref-filter logic.

I see where you're going here, but it feels like there's a lot of
boilerplate to move this thing whole, when we plan to get rid of it in
the long run. After all, we _already_ support most of these placeholders
in ref-filter. I'll keep reading...

-Peff

[PATCH v3 07/23] cat-file: start migrating formatting to ref-filter

[Olga Telezhnaya]

Move mark_atom_in_object_info() from cat-file to ref-filter and
start using it in verify_ref_format().
It also means that we start reusing verify_ref_format() in cat-file.

Start from simple moving of mark_atom_in_object_info(),
it would be removed later by integrating all needed processes into
ref-filter logic.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 35 +--
 ref-filter.c   | 41 -
 ref-filter.h   | 12 ++--
 3 files changed, 47 insertions(+), 41 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index edb04a96d9bd3..67e7790d2f319 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,25 +182,6 @@ static int is_atom(const char *atom, const char *s, int 
slen)
return alen == slen && !memcmp(atom, s, alen);
 }
 
-static void mark_atom_in_object_info(const char *atom, int len,
-struct expand_data *data)
-{
-   if (is_atom("objectname", atom, len))
-   ; /* do nothing */
-   else if (is_atom("objecttype", atom, len))
-   data->info.typep = >type;
-   else if (is_atom("objectsize", atom, len))
-   data->info.sizep = >size;
-   else if (is_atom("objectsize:disk", atom, len))
-   data->info.disk_sizep = >disk_size;
-   else if (is_atom("rest", atom, len))
-   data->split_on_whitespace = 1;
-   else if (is_atom("deltabase", atom, len))
-   data->info.delta_base_sha1 = data->delta_base_oid.hash;
-   else
-   die("unknown format element: %.*s", len, atom);
-}
-
 static void expand_atom(struct strbuf *sb, const char *atom, int len,
 struct expand_data *data)
 {
@@ -230,11 +211,7 @@ static size_t expand_format(struct strbuf *sb, const char 
*start, void *vdata)
if (!end)
die("format element '%s' does not end in ')'", start);
 
-   if (data->mark_query)
-   mark_atom_in_object_info(start + 1, end - start - 1, data);
-   else
-   expand_atom(sb, start + 1, end - start - 1, data);
-
+   expand_atom(sb, start + 1, end - start - 1, data);
return end - start + 1;
 }
 
@@ -413,14 +390,12 @@ static int batch_objects(struct batch_options *opt)
opt->format.format = "%(objectname) %(objecttype) 
%(objectsize)";
 
/*
-* Expand once with our special mark_query flag, which will prime the
-* object_info to be handed to sha1_object_info_extended for each
-* object.
+* Call verify_ref_format to prepare object_info to be handed to
+* sha1_object_info_extended for each object.
 */
memset(, 0, sizeof(data));
-   data.mark_query = 1;
-   strbuf_expand(, opt->format.format, expand_format, );
-   data.mark_query = 0;
+   opt->format.cat_file_data = 
+   verify_ref_format(>format);
if (opt->cmdmode)
data.split_on_whitespace = 1;
 
diff --git a/ref-filter.c b/ref-filter.c
index 5e7ed0f338490..ff87e632f463c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -392,6 +392,31 @@ struct atom_value {
struct used_atom *atom;
 };
 
+static int is_atom(const char *atom, const char *s, int slen)
+{
+   int alen = strlen(atom);
+   return alen == slen && !memcmp(atom, s, alen);
+}
+
+static void mark_atom_in_object_info(const char *atom, int len,
+   struct expand_data *data)
+{
+   if (is_atom("objectname", atom, len))
+   ; /* do nothing */
+   else if (is_atom("objecttype", atom, len))
+   data->info.typep = >type;
+   else if (is_atom("objectsize", atom, len))
+   data->info.sizep = >size;
+   else if (is_atom("objectsize:disk", atom, len))
+   data->info.disk_sizep = >disk_size;
+   else if (is_atom("rest", atom, len))
+   data->split_on_whitespace = 1;
+   else if (is_atom("deltabase", atom, len))
+   data->info.delta_base_sha1 = data->delta_base_oid.hash;
+   else
+   die("unknown format element: %.*s", len, atom);
+}
+
 /*
  * Used to parse format string and sort specifiers
  */
@@ -709,12 +734,18 @@ int verify_ref_format(struct ref_format *format)
if (!ep)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
-   at = parse_ref_filter_atom(format, valid_atom,
-  ARRAY_SIZE(valid_atom), sp + 2, ep);
-   cp = ep + 1;
 
-   if (skip_prefix(used_atom[at].name, "color:", ))
-   format->need_color_reset_at_eol = !!strcmp(color, 
"reset");
+   if