Re: [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions

[Jeff King]

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

> Split expand_atom() into 2 different functions,
> mark_atom_in_object_info() prepares variable for further filling,
> (new) expand_atom() creates resulting string.
> Need that for further reusing of formatting logic from ref-filter.
> Both functions will be step-by-step removed by the end of this patch.

Hmm, this is duplicating the list of atoms. One of the reasons for the
mark_query flag in the original is to have a single list, so that it
can't go out of sync. So I'd hope to see this split go away by the end
of the series...

-Peff

[PATCH v3 06/23] cat-file: split expand_atom() into 2 functions

[Olga Telezhnaya]

Split expand_atom() into 2 different functions,
mark_atom_in_object_info() prepares variable for further filling,
(new) expand_atom() creates resulting string.
Need that for further reusing of formatting logic from ref-filter.
Both functions will be step-by-step removed by the end of this patch.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 73 --
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 37d6096d201b5..edb04a96d9bd3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,47 +182,47 @@ static int is_atom(const char *atom, const char *s, int 
slen)
return alen == slen && !memcmp(atom, s, alen);
 }
 
-static void expand_atom(struct strbuf *sb, const char *atom, int len,
-   void *vdata)
+static void mark_atom_in_object_info(const char *atom, int len,
+struct expand_data *data)
 {
-   struct expand_data *data = vdata;
+   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);
+}
 
-   if (is_atom("objectname", atom, len)) {
-   if (!data->mark_query)
-   strbuf_addstr(sb, oid_to_hex(>oid));
-   } else if (is_atom("objecttype", atom, len)) {
-   if (data->mark_query)
-   data->info.typep = >type;
-   else
-   strbuf_addstr(sb, typename(data->type));
-   } else if (is_atom("objectsize", atom, len)) {
-   if (data->mark_query)
-   data->info.sizep = >size;
-   else
-   strbuf_addf(sb, "%lu", data->size);
-   } else if (is_atom("objectsize:disk", atom, len)) {
-   if (data->mark_query)
-   data->info.disk_sizep = >disk_size;
-   else
-   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
-   } else if (is_atom("rest", atom, len)) {
-   if (data->mark_query)
-   data->split_on_whitespace = 1;
-   else if (data->rest)
+static void expand_atom(struct strbuf *sb, const char *atom, int len,
+struct expand_data *data)
+{
+   if (is_atom("objectname", atom, len))
+   strbuf_addstr(sb, oid_to_hex(>oid));
+   else if (is_atom("objecttype", atom, len))
+   strbuf_addstr(sb, typename(data->type));
+   else if (is_atom("objectsize", atom, len))
+   strbuf_addf(sb, "%lu", data->size);
+   else if (is_atom("objectsize:disk", atom, len))
+   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+   else if (is_atom("rest", atom, len)) {
+   if (data->rest)
strbuf_addstr(sb, data->rest);
-   } else if (is_atom("deltabase", atom, len)) {
-   if (data->mark_query)
-   data->info.delta_base_sha1 = data->delta_base_oid.hash;
-   else
-   strbuf_addstr(sb,
- oid_to_hex(>delta_base_oid));
-   } else
-   die("unknown format element: %.*s", len, atom);
+   } else if (is_atom("deltabase", atom, len))
+   strbuf_addstr(sb, oid_to_hex(>delta_base_oid));
 }
 
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
+static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
 {
const char *end;
+   struct expand_data *data = vdata;
 
if (*start != '(')
return 0;
@@ -230,7 +230,10 @@ static size_t expand_format(struct strbuf *sb, const char 
*start, void *data)
if (!end)
die("format element '%s' does not end in ')'", start);
 
-   expand_atom(sb, start + 1, end - start - 1, data);
+   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);
 
return end - start + 1;
 }

--
https://github.com/git/git/pull/452