# [PATCH 11/20] cat-file: get rid of duplicate checking

24 messages from 2018-01-09 to 2018-01-10. Participants: Olga Telezhnaya, Junio C Hamano, Оля Тележная.
Thread: https://gitlist.dev/t/47573

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 11/20] cat-file: get rid of duplicate checking
Message-ID: <01020160db067b11-6e867688-40cb-4467-954d-847076ff6157-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067b11-6e867688-40cb-4467-954d-847076ff6157-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
We could remove this because we have already checked that
at verify_ref_format function in ref-filter.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index dd43457c0ad7e..1f331559e55c7 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -197,8 +197,6 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,
 		strbuf_addstr(sb, data->rest);
 	else if (is_atom("deltabase", atom, len))
 		strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
-	else
-		die("unknown format element: %.*s", len, atom);
 }
 
 static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 10/20] cat-file: simplify expand_atom function
Message-ID: <01020160db067b07-5333ec0b-ce92-4f85-bceb-bbb9761793a9-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067b07-5333ec0b-ce92-4f85-bceb-bbb9761793a9-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Not sure, but looks like there is no need in that checking.
There is a checking before whether it is null and we die in such case.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 9055fa3a8b0ae..dd43457c0ad7e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -193,10 +193,9 @@ static void expand_atom(struct strbuf *sb, const char *atom, int 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))
+	else if (is_atom("rest", atom, len))
+		strbuf_addstr(sb, data->rest);
+	else if (is_atom("deltabase", atom, len))
 		strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
 	else
 		die("unknown format element: %.*s", len, atom);

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 09/20] cat-file: get rid of goto in ref-filter
Message-ID: <01020160db067b13-009fca6a-7eb1-4246-8966-9e5f8e881642-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067b13-009fca6a-7eb1-4246-8966-9e5f8e881642-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Get rid of goto command in ref-filter for better readability.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 ref-filter.c | 103 ++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 53 insertions(+), 50 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 2bd9a8fc4e39c..1b8c8787190a9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1405,16 +1405,60 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
 	return show_ref(&atom->u.refname, ref->refname);
 }
 
+static void need_object(struct ref_array_item *ref) {
+	struct object *obj;
+	const struct object_id *tagged;
+	unsigned long size;
+	int eaten;
+	void *buf = get_obj(&ref->objectname, &obj, &size, &eaten);
+	if (!buf)
+		die(_("missing object %s for %s"),
+		    oid_to_hex(&ref->objectname), ref->refname);
+	if (!obj)
+		die(_("parse_object_buffer failed on %s for %s"),
+		    oid_to_hex(&ref->objectname), ref->refname);
+
+	grab_values(ref->value, 0, obj, buf, size);
+	if (!eaten)
+		free(buf);
+
+	/*
+	 * If there is no atom that wants to know about tagged
+	 * object, we are done.
+	 */
+	if (!need_tagged || (obj->type != OBJ_TAG))
+		return;
+
+	/*
+	 * If it is a tag object, see if we use a value that derefs
+	 * the object, and if we do grab the object it refers to.
+	 */
+	tagged = &((struct tag *)obj)->tagged->oid;
+
+	/*
+	 * NEEDSWORK: This derefs tag only once, which
+	 * is good to deal with chains of trust, but
+	 * is not consistent with what deref_tag() does
+	 * which peels the onion to the core.
+	 */
+	buf = get_obj(tagged, &obj, &size, &eaten);
+	if (!buf)
+		die(_("missing object %s for %s"),
+		    oid_to_hex(tagged), ref->refname);
+	if (!obj)
+		die(_("parse_object_buffer failed on %s for %s"),
+		    oid_to_hex(tagged), ref->refname);
+	grab_values(ref->value, 1, obj, buf, size);
+	if (!eaten)
+		free(buf);
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
 static void populate_value(struct ref_array_item *ref)
 {
-	void *buf;
-	struct object *obj;
-	int eaten, i;
-	unsigned long size;
-	const struct object_id *tagged;
+	int i;
 
 	ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
 
@@ -1528,53 +1572,12 @@ static void populate_value(struct ref_array_item *ref)
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct atom_value *v = &ref->value[i];
-		if (v->s == NULL)
-			goto need_obj;
+		if (v->s == NULL) {
+			need_object(ref);
+			break;
+		}
 	}
 	return;
-
- need_obj:
-	buf = get_obj(&ref->objectname, &obj, &size, &eaten);
-	if (!buf)
-		die(_("missing object %s for %s"),
-		    oid_to_hex(&ref->objectname), ref->refname);
-	if (!obj)
-		die(_("parse_object_buffer failed on %s for %s"),
-		    oid_to_hex(&ref->objectname), ref->refname);
-
-	grab_values(ref->value, 0, obj, buf, size);
-	if (!eaten)
-		free(buf);
-
-	/*
-	 * If there is no atom that wants to know about tagged
-	 * object, we are done.
-	 */
-	if (!need_tagged || (obj->type != OBJ_TAG))
-		return;
-
-	/*
-	 * If it is a tag object, see if we use a value that derefs
-	 * the object, and if we do grab the object it refers to.
-	 */
-	tagged = &((struct tag *)obj)->tagged->oid;
-
-	/*
-	 * NEEDSWORK: This derefs tag only once, which
-	 * is good to deal with chains of trust, but
-	 * is not consistent with what deref_tag() does
-	 * which peels the onion to the core.
-	 */
-	buf = get_obj(tagged, &obj, &size, &eaten);
-	if (!buf)
-		die(_("missing object %s for %s"),
-		    oid_to_hex(tagged), ref->refname);
-	if (!obj)
-		die(_("parse_object_buffer failed on %s for %s"),
-		    oid_to_hex(tagged), ref->refname);
-	grab_values(ref->value, 1, obj, buf, size);
-	if (!eaten)
-		free(buf);
 }
 
 /*

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 15/20] cat-file: start reusing populate_value
Message-ID: <01020160db067c88-b3fd99cb-2155-43d7-875a-f3f0d2ad0e1e-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067c88-b3fd99cb-2155-43d7-875a-f3f0d2ad0e1e-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Move logic related to getting object info from cat-file to ref-filter.
It will help to reuse whole formatting logic from ref-filter further.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 16 +++-------------
 ref-filter.c       | 15 +++++++++++++++
 ref-filter.h       |  2 ++
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1c92194faaede..e11dbf88e386c 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -284,21 +284,11 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
 	struct strbuf buf = STRBUF_INIT;
 	struct ref_array_item item;
 
-	if (!data->skip_object_info &&
-	    sha1_object_info_extended(data->oid.hash, &data->info,
-				      OBJECT_INFO_LOOKUP_REPLACE) < 0) {
-		printf("%s missing\n",
-		       obj_name ? obj_name : oid_to_hex(&data->oid));
-		fflush(stdout);
-		return;
-	}
-
 	item.objectname = data->oid;
-	item.type = data->type;
-	item.size = data->size;
-	item.disk_size = data->disk_size;
 	item.rest = data->rest;
-	item.delta_base_oid = data->delta_base_oid;
+	item.start_of_request = obj_name;
+
+	if (populate_value(&item)) return;
 
 	strbuf_expand(&buf, opt->format->format, expand_format, &item);
 	strbuf_addch(&buf, '\n');
diff --git a/ref-filter.c b/ref-filter.c
index ed1b78943b8b4..7df6d7e3d6511 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1469,6 +1469,21 @@ int populate_value(struct ref_array_item *ref)
 			ref->symref = "";
 	}
 
+	if (cat_file_info) {
+		if (!cat_file_info->skip_object_info &&
+		    sha1_object_info_extended(ref->objectname.hash, &cat_file_info->info,
+					      OBJECT_INFO_LOOKUP_REPLACE) < 0) {
+			const char *e = ref->start_of_request;
+			printf("%s missing\n", e ? e : oid_to_hex(&ref->objectname));
+			fflush(stdout);
+			return -1;
+		}
+		ref->type = cat_file_info->type;
+		ref->size = cat_file_info->size;
+		ref->disk_size = cat_file_info->disk_size;
+		ref->delta_base_oid = cat_file_info->delta_base_oid;
+	}
+
 	/* Fill in specials first */
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct used_atom *atom = &used_atoms[i];
diff --git a/ref-filter.h b/ref-filter.h
index d541749f9b1dc..9e4444cf3ef9a 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -45,6 +45,8 @@ struct ref_array_item {
 	off_t disk_size;
 	const char *rest;
 	struct object_id delta_base_oid;
+	/* Need it for better explanation in error log. */
+	const char *start_of_request;
 	char refname[FLEX_ARRAY];
 };
 

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 19/20] cat-file: move skip_object_info into ref-filter
Message-ID: <01020160db067c8a-c3ce9582-bc4d-40fd-ac96-b530f93b2752-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067c8a-c3ce9582-bc4d-40fd-ac96-b530f93b2752-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Move logic related to skip_object_info into ref-filter,
so that cat-file does not use that field at all.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 7 +------
 ref-filter.c       | 5 +++++
 ref-filter.h       | 1 +
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5aac10b9808ff..19cee0d22fabe 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -395,16 +395,11 @@ static int batch_objects(struct batch_options *opt)
 	opt->format->cat_file_data = &data;
 	opt->format->is_cat = 1;
 	opt->format->split_on_whitespace = &data.split_on_whitespace;
+	opt->format->all_objects = opt->all_objects;
 	verify_ref_format(opt->format);
 	if (opt->cmdmode)
 		data.split_on_whitespace = 1;
 
-	if (opt->all_objects) {
-		struct object_info empty = OBJECT_INFO_INIT;
-		if (!memcmp(&data.info, &empty, sizeof(empty)))
-			data.skip_object_info = 1;
-	}
-
 	/*
 	 * If we are printing out the object, then always fill in the type,
 	 * since we will want to decide whether or not to stream.
diff --git a/ref-filter.c b/ref-filter.c
index ee27edb2dad56..dbca6856432c0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -766,6 +766,11 @@ int verify_ref_format(struct ref_format *format)
 	}
 	if (format->need_color_reset_at_eol && !want_color(format->use_color))
 		format->need_color_reset_at_eol = 0;
+	if (is_cat && format->all_objects) {
+		struct object_info empty = OBJECT_INFO_INIT;
+		if (!memcmp(&cat_file_info->info, &empty, sizeof(empty)))
+			cat_file_info->skip_object_info = 1;
+	}
 	return 0;
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index 3228661414623..7f7b17e659241 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -128,6 +128,7 @@ struct ref_format {
 	struct expand_data *cat_file_data;
 	int is_cat;
 	int *split_on_whitespace;
+	int all_objects;
 };
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 14/20] cat-file: make populate_value global
Message-ID: <01020160db067bda-4be1f1f1-22c8-45bf-983d-053ae428cb2d-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067bda-4be1f1f1-22c8-45bf-983d-053ae428cb2d-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Make function global for further using in cat-file.
Also added return value for handling errors.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 ref-filter.c | 4 ++--
 ref-filter.h | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 56d2687e07df9..ed1b78943b8b4 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1456,7 +1456,7 @@ static void need_object(struct ref_array_item *ref) {
 /*
  * Parse the object referred by ref, and grab needed value.
  */
-static void populate_value(struct ref_array_item *ref)
+int populate_value(struct ref_array_item *ref)
 {
 	int i;
 
@@ -1577,7 +1577,7 @@ static void populate_value(struct ref_array_item *ref)
 			break;
 		}
 	}
-	return;
+	return 0;
 }
 
 /*
diff --git a/ref-filter.h b/ref-filter.h
index 28774e8e0f771..d541749f9b1dc 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -176,4 +176,7 @@ void setup_ref_filter_porcelain_msg(void);
 void pretty_print_ref(const char *name, const unsigned char *sha1,
 		      const struct ref_format *format);
 
+/* Fill the values of request and prepare all data for final string creation */
+int populate_value(struct ref_array_item *ref);
+
 #endif /*  REF_FILTER_H  */

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 20/20] cat-file: make cat_file_info variable independent
Message-ID: <01020160db067ca1-d14d3b76-31fe-44cc-a655-7a548a0fe17d-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067ca1-d14d3b76-31fe-44cc-a655-7a548a0fe17d-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Remove connection between expand_data variable
in cat-file and in ref-filter.
It will help further to get rid of using expand_data in cat-file.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c |  2 +-
 ref-filter.c       | 28 ++++++++++++++--------------
 ref-filter.h       |  1 -
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 19cee0d22fabe..ffa8e61213e36 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -289,6 +289,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
 	item.start_of_request = obj_name;
 
 	if (populate_value(&item)) return;
+	data->type = item.type;
 
 	strbuf_expand(&buf, opt->format->format, expand_format, &item);
 	strbuf_addch(&buf, '\n');
@@ -392,7 +393,6 @@ static int batch_objects(struct batch_options *opt)
 	 * object.
 	 */
 	memset(&data, 0, sizeof(data));
-	opt->format->cat_file_data = &data;
 	opt->format->is_cat = 1;
 	opt->format->split_on_whitespace = &data.split_on_whitespace;
 	opt->format->all_objects = opt->all_objects;
diff --git a/ref-filter.c b/ref-filter.c
index dbca6856432c0..a8a93b488db37 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,7 +100,7 @@ static struct used_atom {
 	} u;
 } *used_atoms;
 static int used_atom_cnt, need_tagged, need_symref;
-struct expand_data *cat_file_info;
+struct expand_data cat_file_info;
 static int is_cat = 0;
 
 static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
@@ -256,9 +256,9 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
 static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
 {
 	if (!arg)
-		cat_file_info->info.sizep = &cat_file_info->size;
+		cat_file_info.info.sizep = &cat_file_info.size;
 	else if (!strcmp(arg, "disk"))
-		cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
+		cat_file_info.info.disk_sizep = &cat_file_info.disk_size;
 	else
 		die(_("urecognized %%(objectsize) argument: %s"), arg);
 }
@@ -266,7 +266,7 @@ static void objectsize_atom_parser(const struct ref_format *format, struct used_
 static void objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
 {
 	if (!arg)
-		cat_file_info->info.typep = &cat_file_info->type;
+		cat_file_info.info.typep = &cat_file_info.type;
 	else
 		die(_("urecognized %%(objecttype) argument: %s"), arg);
 }
@@ -274,7 +274,7 @@ static void objecttype_atom_parser(const struct ref_format *format, struct used_
 static void deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
 {
 	if (!arg)
-		cat_file_info->info.delta_base_sha1 = cat_file_info->delta_base_oid.hash;
+		cat_file_info.info.delta_base_sha1 = cat_file_info.delta_base_oid.hash;
 	else
 		die(_("urecognized %%(deltabase) argument: %s"), arg);
 }
@@ -739,7 +739,6 @@ int verify_ref_format(struct ref_format *format)
 {
 	const char *cp, *sp;
 
-	cat_file_info = format->cat_file_data;
 	is_cat = format->is_cat;
 	format->need_color_reset_at_eol = 0;
 	for (cp = format->format; *cp && (sp = find_next(cp)); ) {
@@ -768,8 +767,8 @@ int verify_ref_format(struct ref_format *format)
 		format->need_color_reset_at_eol = 0;
 	if (is_cat && format->all_objects) {
 		struct object_info empty = OBJECT_INFO_INIT;
-		if (!memcmp(&cat_file_info->info, &empty, sizeof(empty)))
-			cat_file_info->skip_object_info = 1;
+		if (!memcmp(&cat_file_info.info, &empty, sizeof(empty)))
+			cat_file_info.skip_object_info = 1;
 	}
 	return 0;
 }
@@ -1474,18 +1473,19 @@ int populate_value(struct ref_array_item *ref)
 	}
 
 	if (is_cat) {
-		if (!cat_file_info->skip_object_info &&
-		    sha1_object_info_extended(ref->objectname.hash, &cat_file_info->info,
+		if (!cat_file_info.info.typep) cat_file_info.info.typep = &cat_file_info.type;
+		if (!cat_file_info.skip_object_info &&
+		    sha1_object_info_extended(ref->objectname.hash, &cat_file_info.info,
 					      OBJECT_INFO_LOOKUP_REPLACE) < 0) {
 			const char *e = ref->start_of_request;
 			printf("%s missing\n", e ? e : oid_to_hex(&ref->objectname));
 			fflush(stdout);
 			return -1;
 		}
-		ref->type = cat_file_info->type;
-		ref->size = cat_file_info->size;
-		ref->disk_size = cat_file_info->disk_size;
-		ref->delta_base_oid = cat_file_info->delta_base_oid;
+		ref->type = cat_file_info.type;
+		ref->size = cat_file_info.size;
+		ref->disk_size = cat_file_info.disk_size;
+		ref->delta_base_oid = cat_file_info.delta_base_oid;
 	}
 
 	/* Fill in specials first */
diff --git a/ref-filter.h b/ref-filter.h
index 7f7b17e659241..ebaed0412fac2 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -125,7 +125,6 @@ struct ref_format {
 	 * Helps to move all formatting logic from cat-file to ref-filter,
 	 * hopefully would be reduced later.
 	 */
-	struct expand_data *cat_file_data;
 	int is_cat;
 	int *split_on_whitespace;
 	int all_objects;

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 04/20] cat-file: make valid_atoms as a function parameter
Message-ID: <01020160db067ae8-81d31cf2-0f69-4f36-9de8-27e9716d2f4b-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067ae8-81d31cf2-0f69-4f36-9de8-27e9716d2f4b-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Make valid_atoms as a function parameter,
there could be another variable further.
Need that for further reusing of formatting logic in cat-file.c.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 ref-filter.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 1426f0c28bce7..2d6e81fe1ab00 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -396,6 +396,7 @@ struct atom_value {
  * Used to parse format string and sort specifiers
  */
 static int parse_ref_filter_atom(const struct ref_format *format,
+				 const struct valid_atom *valid_atoms, int n_atoms,
 				 const char *atom, const char *ep)
 {
 	const char *sp;
@@ -425,13 +426,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,
 	atom_len = (arg ? arg : ep) - sp;
 
 	/* Is the atom a valid one? */
-	for (i = 0; i < ARRAY_SIZE(valid_atoms); i++) {
+	for (i = 0; i < n_atoms; i++) {
 		int len = strlen(valid_atoms[i].name);
 		if (len == atom_len && !memcmp(valid_atoms[i].name, sp, len))
 			break;
 	}
 
-	if (ARRAY_SIZE(valid_atoms) <= i)
+	if (n_atoms <= i)
 		die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
 
 	/* Add it in, including the deref prefix */
@@ -708,7 +709,8 @@ 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, sp + 2, ep);
+		at = parse_ref_filter_atom(format, valid_atoms,
+					   ARRAY_SIZE(valid_atoms), sp + 2, ep);
 		cp = ep + 1;
 
 		if (skip_prefix(used_atoms[at].name, "color:", &color))
@@ -2139,7 +2141,9 @@ void format_ref_array_item(struct ref_array_item *info,
 		if (cp < sp)
 			append_literal(cp, sp, &state);
 		get_ref_atom_value(info,
-				   parse_ref_filter_atom(format, sp + 2, ep),
+				   parse_ref_filter_atom(format, valid_atoms,
+							 ARRAY_SIZE(valid_atoms),
+							 sp + 2, ep),
 				   &atomv);
 		atomv->handler(atomv, &state);
 	}
@@ -2187,7 +2191,8 @@ static int parse_sorting_atom(const char *atom)
 	 */
 	struct ref_format dummy = REF_FORMAT_INIT;
 	const char *end = atom + strlen(atom);
-	return parse_ref_filter_atom(&dummy, atom, end);
+	return parse_ref_filter_atom(&dummy, valid_atoms,
+				     ARRAY_SIZE(valid_atoms), atom, end);
 }
 
 /*  If no sorting option is given, use refname to sort as default */

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 07/20] cat-file: reuse parse_ref_filter_atom
Message-ID: <01020160db067b4f-60d6012f-7cb8-4ba2-891f-1d1ed5bb5786-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067b4f-60d6012f-7cb8-4ba2-891f-1d1ed5bb5786-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Continue migrating formatting logic from cat-file to ref-filter.
Reuse parse_ref_filter_atom for unifying all processes in ref-filter
and further reducing of expand_atom_into_fields function.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 ref-filter.c | 73 +++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 98bb10185ae96..2bd9a8fc4e39c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,6 +100,7 @@ static struct used_atom {
 	} u;
 } *used_atoms;
 static int used_atom_cnt, need_tagged, need_symref;
+struct expand_data *cat_file_info;
 
 static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
 {
@@ -251,6 +252,16 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
 		die(_("unrecognized %%(objectname) argument: %s"), arg);
 }
 
+static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+	if (!arg)
+		; /* default to normal object size */
+	else if (!strcmp(arg, "disk"))
+		cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
+	else
+		die(_("urecognized %%(objectsize) argument: %s"), arg);
+}
+
 static void refname_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
 {
 	refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
@@ -371,6 +382,14 @@ static struct valid_atom {
 	{ "else" },
 };
 
+static struct valid_atom valid_cat_file_atoms[] = {
+	{ "objectname" },
+	{ "objecttype" },
+	{ "objectsize", FIELD_ULONG, objectsize_atom_parser },
+	{ "rest" },
+	{ "deltabase" },
+};
+
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
 
 struct ref_formatting_stack {
@@ -392,6 +411,25 @@ 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 expand_atom_into_fields(const char *atom, int len,
+				    struct expand_data *data)
+{
+	if (is_atom("objecttype", atom, len))
+		data->info.typep = &data->type;
+	else if (is_atom("objectsize", atom, len))
+		data->info.sizep = &data->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;
+}
+
 /*
  * Used to parse format string and sort specifiers
  */
@@ -458,6 +496,8 @@ static int parse_ref_filter_atom(const struct ref_format *format,
 		need_tagged = 1;
 	if (!strcmp(valid_atoms[i].name, "symref"))
 		need_symref = 1;
+	if (cat_file_info)
+		expand_atom_into_fields(atom, atom_len, cat_file_info);
 	return at;
 }
 
@@ -693,31 +733,6 @@ static const char *find_next(const char *cp)
 	return NULL;
 }
 
-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 expand_atom_into_fields(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 = &data->type;
-	else if (is_atom("objectsize", atom, len))
-		data->info.sizep = &data->size;
-	else if (is_atom("objectsize:disk", atom, len))
-		data->info.disk_sizep = &data->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);
-}
-
 /*
  * Make sure the format string is well formed, and parse out
  * the used atoms.
@@ -726,6 +741,7 @@ int verify_ref_format(struct ref_format *format)
 {
 	const char *cp, *sp;
 
+	cat_file_info = format->cat_file_data;
 	format->need_color_reset_at_eol = 0;
 	for (cp = format->format; *cp && (sp = find_next(cp)); ) {
 		const char *color, *ep = strchr(sp, ')');
@@ -736,9 +752,10 @@ int verify_ref_format(struct ref_format *format)
 		/* sp points at "%(" and ep points at the closing ")" */
 
 		if (format->cat_file_data)
-			expand_atom_into_fields(sp + 2, ep - sp - 2,
-						format->cat_file_data);
-		else
+		{
+			at = parse_ref_filter_atom(format, valid_cat_file_atoms,
+						   ARRAY_SIZE(valid_cat_file_atoms), sp + 2, ep);
+		} else
 		{
 			at = parse_ref_filter_atom(format, valid_atoms,
 						   ARRAY_SIZE(valid_atoms), sp + 2, ep);

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 12/20] cat-file: rename variable in ref-filter
Message-ID: <01020160db067af3-3108a276-f04a-4eb0-8434-5006f6596f56-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067af3-3108a276-f04a-4eb0-8434-5006f6596f56-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Rename variable for easier reading.
It points not to values, but to arrays.
Added "s" ending so that it could be obvious.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 ref-filter.c | 16 ++++++++--------
 ref-filter.h |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 1b8c8787190a9..56d2687e07df9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1418,7 +1418,7 @@ static void need_object(struct ref_array_item *ref) {
 		die(_("parse_object_buffer failed on %s for %s"),
 		    oid_to_hex(&ref->objectname), ref->refname);
 
-	grab_values(ref->value, 0, obj, buf, size);
+	grab_values(ref->values, 0, obj, buf, size);
 	if (!eaten)
 		free(buf);
 
@@ -1448,7 +1448,7 @@ static void need_object(struct ref_array_item *ref) {
 	if (!obj)
 		die(_("parse_object_buffer failed on %s for %s"),
 		    oid_to_hex(tagged), ref->refname);
-	grab_values(ref->value, 1, obj, buf, size);
+	grab_values(ref->values, 1, obj, buf, size);
 	if (!eaten)
 		free(buf);
 }
@@ -1460,7 +1460,7 @@ static void populate_value(struct ref_array_item *ref)
 {
 	int i;
 
-	ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
+	ref->values = xcalloc(used_atom_cnt, sizeof(struct atom_value));
 
 	if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
 		ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
@@ -1473,7 +1473,7 @@ static void populate_value(struct ref_array_item *ref)
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct used_atom *atom = &used_atoms[i];
 		const char *name = used_atoms[i].name;
-		struct atom_value *v = &ref->value[i];
+		struct atom_value *v = &ref->values[i];
 		int deref = 0;
 		const char *refname;
 		struct branch *branch = NULL;
@@ -1571,7 +1571,7 @@ static void populate_value(struct ref_array_item *ref)
 	}
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		struct atom_value *v = &ref->value[i];
+		struct atom_value *v = &ref->values[i];
 		if (v->s == NULL) {
 			need_object(ref);
 			break;
@@ -1586,11 +1586,11 @@ static void populate_value(struct ref_array_item *ref)
  */
 static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
 {
-	if (!ref->value) {
+	if (!ref->values) {
 		populate_value(ref);
-		fill_missing_values(ref->value);
+		fill_missing_values(ref->values);
 	}
-	*v = &ref->value[atom];
+	*v = &ref->values[atom];
 }
 
 /*
diff --git a/ref-filter.h b/ref-filter.h
index 590a60ffe034d..de3fd3263ac64 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -39,7 +39,7 @@ struct ref_array_item {
 	unsigned int kind;
 	const char *symref;
 	struct commit *commit;
-	struct atom_value *value;
+	struct atom_value *values;
 	char refname[FLEX_ARRAY];
 };
 

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 06/20] cat-file: start migrating to ref-filter
Message-ID: <01020160db067b66-c22a78cb-5297-426b-9b11-a20a8c27f74e-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067b66-c22a78cb-5297-426b-9b11-a20a8c27f74e-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Start moving all formatting stuff from cat-file to ref-filter.
Start from simple moving, it would be integrated into
all ref-filter processes further.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c |  5 ++---
 ref-filter.c       | 42 +++++++++++++++++++++++++++++++++++++-----
 ref-filter.h       |  6 ++++++
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 7fd5b960ad698..0a3f4a47bf1ae 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -420,9 +420,8 @@ static int batch_objects(struct batch_options *opt)
 	 * object.
 	 */
 	memset(&data, 0, sizeof(data));
-	data.mark_query = 1;
-	strbuf_expand(&buf, opt->format->format, expand_format, &data);
-	data.mark_query = 0;
+	opt->format->cat_file_data = &data;
+	verify_ref_format(opt->format);
 	if (opt->cmdmode)
 		data.split_on_whitespace = 1;
 
diff --git a/ref-filter.c b/ref-filter.c
index 2d6e81fe1ab00..98bb10185ae96 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -693,6 +693,31 @@ static const char *find_next(const char *cp)
 	return NULL;
 }
 
+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 expand_atom_into_fields(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 = &data->type;
+	else if (is_atom("objectsize", atom, len))
+		data->info.sizep = &data->size;
+	else if (is_atom("objectsize:disk", atom, len))
+		data->info.disk_sizep = &data->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);
+}
+
 /*
  * Make sure the format string is well formed, and parse out
  * the used atoms.
@@ -709,12 +734,19 @@ 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_atoms,
-					   ARRAY_SIZE(valid_atoms), sp + 2, ep);
-		cp = ep + 1;
 
-		if (skip_prefix(used_atoms[at].name, "color:", &color))
-			format->need_color_reset_at_eol = !!strcmp(color, "reset");
+		if (format->cat_file_data)
+			expand_atom_into_fields(sp + 2, ep - sp - 2,
+						format->cat_file_data);
+		else
+		{
+			at = parse_ref_filter_atom(format, valid_atoms,
+						   ARRAY_SIZE(valid_atoms), sp + 2, ep);
+			if (skip_prefix(used_atoms[at].name, "color:", &color))
+				format->need_color_reset_at_eol = !!strcmp(color, "reset");
+		}
+
+		cp = ep + 1;
 	}
 	if (format->need_color_reset_at_eol && !want_color(format->use_color))
 		format->need_color_reset_at_eol = 0;
diff --git a/ref-filter.h b/ref-filter.h
index 16d00e4b1bded..97169548939cd 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -119,6 +119,12 @@ struct ref_format {
 
 	/* Internal state to ref-filter */
 	int need_color_reset_at_eol;
+
+	/*
+	 * Helps to move all formatting logic from cat-file to ref-filter,
+	 * hopefully would be reduced later.
+	 */
+	struct expand_data *cat_file_data;
 };
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 02/20] cat-file: reuse struct ref_format
Message-ID: <01020160db067a62-874402f0-c7c0-4fb4-91c3-ac644fcf9a1f-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067a62-874402f0-c7c0-4fb4-91c3-ac644fcf9a1f-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Start using ref_format struct instead of simple char*.
Need that for further reusing of formatting logic from ref-filter.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f783b39b9bd5c..7655a9a726773 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -13,15 +13,16 @@
 #include "tree-walk.h"
 #include "sha1-array.h"
 #include "packfile.h"
+#include "ref-filter.h"
 
 struct batch_options {
+	struct ref_format *format;
 	int enabled;
 	int follow_symlinks;
 	int print_contents;
 	int buffer_output;
 	int all_objects;
 	int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
-	const char *format;
 };
 
 static const char *force_path;
@@ -353,7 +354,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
 		return;
 	}
 
-	strbuf_expand(&buf, opt->format, expand_format, data);
+	strbuf_expand(&buf, opt->format->format, expand_format, data);
 	strbuf_addch(&buf, '\n');
 	batch_write(opt, buf.buf, buf.len);
 	strbuf_release(&buf);
@@ -446,8 +447,8 @@ static int batch_objects(struct batch_options *opt)
 	int save_warning;
 	int retval = 0;
 
-	if (!opt->format)
-		opt->format = "%(objectname) %(objecttype) %(objectsize)";
+	if (!opt->format->format)
+		opt->format->format = "%(objectname) %(objecttype) %(objectsize)";
 
 	/*
 	 * Expand once with our special mark_query flag, which will prime the
@@ -456,7 +457,7 @@ static int batch_objects(struct batch_options *opt)
 	 */
 	memset(&data, 0, sizeof(data));
 	data.mark_query = 1;
-	strbuf_expand(&buf, opt->format, expand_format, &data);
+	strbuf_expand(&buf, opt->format->format, expand_format, &data);
 	data.mark_query = 0;
 	if (opt->cmdmode)
 		data.split_on_whitespace = 1;
@@ -548,7 +549,7 @@ static int batch_option_callback(const struct option *opt,
 
 	bo->enabled = 1;
 	bo->print_contents = !strcmp(opt->long_name, "batch");
-	bo->format = arg;
+	bo->format->format = arg;
 
 	return 0;
 }
@@ -557,7 +558,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 {
 	int opt = 0;
 	const char *exp_type = NULL, *obj_name = NULL;
-	struct batch_options batch = {0};
+	struct ref_format format = REF_FORMAT_INIT;
+	struct batch_options batch = {&format};
 	int unknown_type = 0;
 
 	const struct option options[] = {

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 18/20] cat-file: add split_on_whitespace flag
Message-ID: <01020160db067c88-f01ed048-da02-43a7-8b61-db62027911bf-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067c88-f01ed048-da02-43a7-8b61-db62027911bf-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Add flag to ref_format struct so that we could pass needed info
to cat-file.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 1 +
 ref-filter.c       | 4 ++--
 ref-filter.h       | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 289912ab1f858..5aac10b9808ff 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -394,6 +394,7 @@ static int batch_objects(struct batch_options *opt)
 	memset(&data, 0, sizeof(data));
 	opt->format->cat_file_data = &data;
 	opt->format->is_cat = 1;
+	opt->format->split_on_whitespace = &data.split_on_whitespace;
 	verify_ref_format(opt->format);
 	if (opt->cmdmode)
 		data.split_on_whitespace = 1;
diff --git a/ref-filter.c b/ref-filter.c
index 4259c9b9cd767..ee27edb2dad56 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -494,8 +494,8 @@ static int parse_ref_filter_atom(const struct ref_format *format,
 		need_tagged = 1;
 	if (!strcmp(valid_atoms[i].name, "symref"))
 		need_symref = 1;
-	if (is_cat && !strcmp(valid_atoms[i].name, "rest"))
-		cat_file_info->split_on_whitespace = 1;
+	if (!strcmp(valid_atoms[i].name, "rest"))
+		*format->split_on_whitespace = 1;
 	return at;
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index 5ba4724a472f6..3228661414623 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -127,6 +127,7 @@ struct ref_format {
 	 */
 	struct expand_data *cat_file_data;
 	int is_cat;
+	int *split_on_whitespace;
 };
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 13/20] cat-file: start use ref_array_item struct
Message-ID: <01020160db067baa-645d1cd7-0fa5-404e-b3f0-afba7dac7d18-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067baa-645d1cd7-0fa5-404e-b3f0-afba7dac7d18-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Moving from using expand_data to ref_array_item structure.
That helps us to reuse functions from ref-filter easier.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 30 +++++++++++++++++++-----------
 ref-filter.h       |  5 +++++
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1f331559e55c7..1c92194faaede 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -183,26 +183,26 @@ static int is_atom(const char *atom, const char *s, int slen)
 }
 
 static void expand_atom(struct strbuf *sb, const char *atom, int len,
-			 struct expand_data *data)
+			 struct ref_array_item *item)
 {
 	if (is_atom("objectname", atom, len))
-		strbuf_addstr(sb, oid_to_hex(&data->oid));
+		strbuf_addstr(sb, oid_to_hex(&item->objectname));
 	else if (is_atom("objecttype", atom, len))
-		strbuf_addstr(sb, typename(data->type));
+		strbuf_addstr(sb, typename(item->type));
 	else if (is_atom("objectsize", atom, len))
-		strbuf_addf(sb, "%lu", data->size);
+		strbuf_addf(sb, "%lu", item->size);
 	else if (is_atom("objectsize:disk", atom, len))
-		strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+		strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)item->disk_size);
 	else if (is_atom("rest", atom, len))
-		strbuf_addstr(sb, data->rest);
+		strbuf_addstr(sb, item->rest);
 	else if (is_atom("deltabase", atom, len))
-		strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
+		strbuf_addstr(sb, oid_to_hex(&item->delta_base_oid));
 }
 
-static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
+static size_t expand_format(struct strbuf *sb, const char *start, void *data)
 {
 	const char *end;
-	struct expand_data *data = vdata;
+	struct ref_array_item *item = data;
 
 	if (*start != '(')
 		return 0;
@@ -210,7 +210,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);
 
-	expand_atom(sb, start + 1, end - start - 1, data);
+	expand_atom(sb, start + 1, end - start - 1, item);
 	return end - start + 1;
 }
 
@@ -282,6 +282,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
 			       struct expand_data *data)
 {
 	struct strbuf buf = STRBUF_INIT;
+	struct ref_array_item item;
 
 	if (!data->skip_object_info &&
 	    sha1_object_info_extended(data->oid.hash, &data->info,
@@ -292,7 +293,14 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
 		return;
 	}
 
-	strbuf_expand(&buf, opt->format->format, expand_format, data);
+	item.objectname = data->oid;
+	item.type = data->type;
+	item.size = data->size;
+	item.disk_size = data->disk_size;
+	item.rest = data->rest;
+	item.delta_base_oid = data->delta_base_oid;
+
+	strbuf_expand(&buf, opt->format->format, expand_format, &item);
 	strbuf_addch(&buf, '\n');
 	batch_write(opt, buf.buf, buf.len);
 	strbuf_release(&buf);
diff --git a/ref-filter.h b/ref-filter.h
index de3fd3263ac64..28774e8e0f771 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -40,6 +40,11 @@ struct ref_array_item {
 	const char *symref;
 	struct commit *commit;
 	struct atom_value *values;
+	enum object_type type;
+	unsigned long size;
+	off_t disk_size;
+	const char *rest;
+	struct object_id delta_base_oid;
 	char refname[FLEX_ARRAY];
 };
 

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 17/20] cat-file: add is_cat flag in ref-filter
Message-ID: <01020160db067c89-c76828dd-d98c-45c6-8de1-2c9d7fc61841-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067c89-c76828dd-d98c-45c6-8de1-2c9d7fc61841-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Add is_cat flag, further it helps to get rid of cat_file_data field
in ref_format.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 1 +
 ref-filter.c       | 8 +++++---
 ref-filter.h       | 1 +
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index e11dbf88e386c..289912ab1f858 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -393,6 +393,7 @@ static int batch_objects(struct batch_options *opt)
 	 */
 	memset(&data, 0, sizeof(data));
 	opt->format->cat_file_data = &data;
+	opt->format->is_cat = 1;
 	verify_ref_format(opt->format);
 	if (opt->cmdmode)
 		data.split_on_whitespace = 1;
diff --git a/ref-filter.c b/ref-filter.c
index 112336edbe871..4259c9b9cd767 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -101,6 +101,7 @@ static struct used_atom {
 } *used_atoms;
 static int used_atom_cnt, need_tagged, need_symref;
 struct expand_data *cat_file_info;
+static int is_cat = 0;
 
 static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
 {
@@ -493,7 +494,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,
 		need_tagged = 1;
 	if (!strcmp(valid_atoms[i].name, "symref"))
 		need_symref = 1;
-	if (cat_file_info && !strcmp(valid_atoms[i].name, "rest"))
+	if (is_cat && !strcmp(valid_atoms[i].name, "rest"))
 		cat_file_info->split_on_whitespace = 1;
 	return at;
 }
@@ -739,6 +740,7 @@ int verify_ref_format(struct ref_format *format)
 	const char *cp, *sp;
 
 	cat_file_info = format->cat_file_data;
+	is_cat = format->is_cat;
 	format->need_color_reset_at_eol = 0;
 	for (cp = format->format; *cp && (sp = find_next(cp)); ) {
 		const char *color, *ep = strchr(sp, ')');
@@ -748,7 +750,7 @@ int verify_ref_format(struct ref_format *format)
 			return error(_("malformed format string %s"), sp);
 		/* sp points at "%(" and ep points at the closing ")" */
 
-		if (format->cat_file_data)
+		if (is_cat)
 		{
 			at = parse_ref_filter_atom(format, valid_cat_file_atoms,
 						   ARRAY_SIZE(valid_cat_file_atoms), sp + 2, ep);
@@ -1466,7 +1468,7 @@ int populate_value(struct ref_array_item *ref)
 			ref->symref = "";
 	}
 
-	if (cat_file_info) {
+	if (is_cat) {
 		if (!cat_file_info->skip_object_info &&
 		    sha1_object_info_extended(ref->objectname.hash, &cat_file_info->info,
 					      OBJECT_INFO_LOOKUP_REPLACE) < 0) {
diff --git a/ref-filter.h b/ref-filter.h
index 9e4444cf3ef9a..5ba4724a472f6 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -126,6 +126,7 @@ struct ref_format {
 	 * hopefully would be reduced later.
 	 */
 	struct expand_data *cat_file_data;
+	int is_cat;
 };
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 16/20] cat-file: get rid of expand_atom_into_fields
Message-ID: <01020160db067c7c-74dd1ef6-598b-4010-bc2c-4315fd3125ee-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067c7c-74dd1ef6-598b-4010-bc2c-4315fd3125ee-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Remove expand_atom_into_fields function and create same logic
in terms of ref-filter style.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 ref-filter.c | 45 +++++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 7df6d7e3d6511..112336edbe871 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -255,13 +255,29 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
 static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
 {
 	if (!arg)
-		; /* default to normal object size */
+		cat_file_info->info.sizep = &cat_file_info->size;
 	else if (!strcmp(arg, "disk"))
 		cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
 	else
 		die(_("urecognized %%(objectsize) argument: %s"), arg);
 }
 
+static void objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+	if (!arg)
+		cat_file_info->info.typep = &cat_file_info->type;
+	else
+		die(_("urecognized %%(objecttype) argument: %s"), arg);
+}
+
+static void deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+	if (!arg)
+		cat_file_info->info.delta_base_sha1 = cat_file_info->delta_base_oid.hash;
+	else
+		die(_("urecognized %%(deltabase) argument: %s"), arg);
+}
+
 static void refname_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
 {
 	refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
@@ -384,10 +400,10 @@ static struct valid_atom {
 
 static struct valid_atom valid_cat_file_atoms[] = {
 	{ "objectname" },
-	{ "objecttype" },
+	{ "objecttype", FIELD_STR, objecttype_atom_parser },
 	{ "objectsize", FIELD_ULONG, objectsize_atom_parser },
 	{ "rest" },
-	{ "deltabase" },
+	{ "deltabase", FIELD_STR, deltabase_atom_parser },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -411,25 +427,6 @@ 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 expand_atom_into_fields(const char *atom, int len,
-				    struct expand_data *data)
-{
-	if (is_atom("objecttype", atom, len))
-		data->info.typep = &data->type;
-	else if (is_atom("objectsize", atom, len))
-		data->info.sizep = &data->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;
-}
-
 /*
  * Used to parse format string and sort specifiers
  */
@@ -496,8 +493,8 @@ static int parse_ref_filter_atom(const struct ref_format *format,
 		need_tagged = 1;
 	if (!strcmp(valid_atoms[i].name, "symref"))
 		need_symref = 1;
-	if (cat_file_info)
-		expand_atom_into_fields(atom, atom_len, cat_file_info);
+	if (cat_file_info && !strcmp(valid_atoms[i].name, "rest"))
+		cat_file_info->split_on_whitespace = 1;
 	return at;
 }
 

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 03/20] cat-file: rename variables in ref-filter
Message-ID: <01020160db067bde-7f500636-b80e-4099-a84e-2613126c9aa1-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067bde-7f500636-b80e-4099-a84e-2613126c9aa1-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Rename some variables for easier reading.
They point not to values, but to arrays.
Added "s" ending so that it could be obvious.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 ref-filter.c | 60 ++++++++++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 3f9161707e66b..1426f0c28bce7 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -98,7 +98,7 @@ static struct used_atom {
 		struct refname_atom refname;
 		char *head;
 	} u;
-} *used_atom;
+} *used_atoms;
 static int used_atom_cnt, need_tagged, need_symref;
 
 static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
@@ -325,11 +325,11 @@ static void head_atom_parser(const struct ref_format *format, struct used_atom *
 	atom->u.head = resolve_refdup("HEAD", RESOLVE_REF_READING, NULL, NULL);
 }
 
-static struct {
+static struct valid_atom {
 	const char *name;
 	cmp_type cmp_type;
 	void (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg);
-} valid_atom[] = {
+} valid_atoms[] = {
 	{ "refname" , FIELD_STR, refname_atom_parser },
 	{ "objecttype" },
 	{ "objectsize", FIELD_ULONG },
@@ -410,38 +410,38 @@ static int parse_ref_filter_atom(const struct ref_format *format,
 
 	/* Do we have the atom already used elsewhere? */
 	for (i = 0; i < used_atom_cnt; i++) {
-		int len = strlen(used_atom[i].name);
-		if (len == ep - atom && !memcmp(used_atom[i].name, atom, len))
+		int len = strlen(used_atoms[i].name);
+		if (len == ep - atom && !memcmp(used_atoms[i].name, atom, len))
 			return i;
 	}
 
 	/*
 	 * If the atom name has a colon, strip it and everything after
 	 * it off - it specifies the format for this entry, and
-	 * shouldn't be used for checking against the valid_atom
+	 * shouldn't be used for checking against the valid_atoms
 	 * table.
 	 */
 	arg = memchr(sp, ':', ep - sp);
 	atom_len = (arg ? arg : ep) - sp;
 
 	/* Is the atom a valid one? */
-	for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
-		int len = strlen(valid_atom[i].name);
-		if (len == atom_len && !memcmp(valid_atom[i].name, sp, len))
+	for (i = 0; i < ARRAY_SIZE(valid_atoms); i++) {
+		int len = strlen(valid_atoms[i].name);
+		if (len == atom_len && !memcmp(valid_atoms[i].name, sp, len))
 			break;
 	}
 
-	if (ARRAY_SIZE(valid_atom) <= i)
+	if (ARRAY_SIZE(valid_atoms) <= i)
 		die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
 
 	/* Add it in, including the deref prefix */
 	at = used_atom_cnt;
 	used_atom_cnt++;
-	REALLOC_ARRAY(used_atom, used_atom_cnt);
-	used_atom[at].name = xmemdupz(atom, ep - atom);
-	used_atom[at].type = valid_atom[i].cmp_type;
+	REALLOC_ARRAY(used_atoms, used_atom_cnt);
+	used_atoms[at].name = xmemdupz(atom, ep - atom);
+	used_atoms[at].type = valid_atoms[i].cmp_type;
 	if (arg) {
-		arg = used_atom[at].name + (arg - atom) + 1;
+		arg = used_atoms[at].name + (arg - atom) + 1;
 		if (!*arg) {
 			/*
 			 * Treat empty sub-arguments list as NULL (i.e.,
@@ -450,12 +450,12 @@ static int parse_ref_filter_atom(const struct ref_format *format,
 			arg = NULL;
 		}
 	}
-	memset(&used_atom[at].u, 0, sizeof(used_atom[at].u));
-	if (valid_atom[i].parser)
-		valid_atom[i].parser(format, &used_atom[at], arg);
+	memset(&used_atoms[at].u, 0, sizeof(used_atoms[at].u));
+	if (valid_atoms[i].parser)
+		valid_atoms[i].parser(format, &used_atoms[at], arg);
 	if (*atom == '*')
 		need_tagged = 1;
-	if (!strcmp(valid_atom[i].name, "symref"))
+	if (!strcmp(valid_atoms[i].name, "symref"))
 		need_symref = 1;
 	return at;
 }
@@ -711,7 +711,7 @@ int verify_ref_format(struct ref_format *format)
 		at = parse_ref_filter_atom(format, sp + 2, ep);
 		cp = ep + 1;
 
-		if (skip_prefix(used_atom[at].name, "color:", &color))
+		if (skip_prefix(used_atoms[at].name, "color:", &color))
 			format->need_color_reset_at_eol = !!strcmp(color, "reset");
 	}
 	if (format->need_color_reset_at_eol && !want_color(format->use_color))
@@ -762,7 +762,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
 	int i;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
+		const char *name = used_atoms[i].name;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
 			continue;
@@ -775,7 +775,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
 			v->s = xstrfmt("%lu", sz);
 		}
 		else if (deref)
-			grab_objectname(name, obj->oid.hash, v, &used_atom[i]);
+			grab_objectname(name, obj->oid.hash, v, &used_atoms[i]);
 	}
 }
 
@@ -786,7 +786,7 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob
 	struct tag *tag = (struct tag *) obj;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
+		const char *name = used_atoms[i].name;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
 			continue;
@@ -808,7 +808,7 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 	struct commit *commit = (struct commit *) obj;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
+		const char *name = used_atoms[i].name;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
 			continue;
@@ -938,7 +938,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	const char *wholine = NULL;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
+		const char *name = used_atoms[i].name;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
 			continue;
@@ -976,7 +976,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	if (!wholine)
 		return;
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
+		const char *name = used_atoms[i].name;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
 			continue;
@@ -1066,7 +1066,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 	unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		struct used_atom *atom = &used_atom[i];
+		struct used_atom *atom = &used_atoms[i];
 		const char *name = atom->name;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
@@ -1127,7 +1127,7 @@ static void fill_missing_values(struct atom_value *val)
 
 /*
  * val is a list of atom_value to hold returned values.  Extract
- * the values for atoms in used_atom array out of (obj, buf, sz).
+ * the values for atoms in used_atoms array out of (obj, buf, sz).
  * when deref is false, (obj, buf, sz) is the object that is
  * pointed at by the ref itself; otherwise it is the object the
  * ref (which is a tag) refers to.
@@ -1376,8 +1376,8 @@ static void populate_value(struct ref_array_item *ref)
 
 	/* Fill in specials first */
 	for (i = 0; i < used_atom_cnt; i++) {
-		struct used_atom *atom = &used_atom[i];
-		const char *name = used_atom[i].name;
+		struct used_atom *atom = &used_atoms[i];
+		const char *name = used_atoms[i].name;
 		struct atom_value *v = &ref->value[i];
 		int deref = 0;
 		const char *refname;
@@ -2059,7 +2059,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
 {
 	struct atom_value *va, *vb;
 	int cmp;
-	cmp_type cmp_type = used_atom[s->atom].type;
+	cmp_type cmp_type = used_atoms[s->atom].type;
 	int (*cmp_fn)(const char *, const char *);
 
 	get_ref_atom_value(a, s->atom, &va);

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 08/20] cat-file: remove unused code
Message-ID: <01020160db067add-6a32c398-0438-4bd1-8622-d3870adf9167-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067add-6a32c398-0438-4bd1-8622-d3870adf9167-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
No further need in mark_query parameter.
All logic related to expand_atom_into_fields is not needed here also,
we are doing that in ref-filter.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 25 +------------------------
 ref-filter.h       |  6 ------
 2 files changed, 1 insertion(+), 30 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0a3f4a47bf1ae..9055fa3a8b0ae 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 expand_atom_into_fields(struct strbuf *sb, 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 = &data->type;
-	else if (is_atom("objectsize", atom, len))
-		data->info.sizep = &data->size;
-	else if (is_atom("objectsize:disk", atom, len))
-		data->info.disk_sizep = &data->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)
 {
@@ -232,11 +213,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)
-		expand_atom_into_fields(sb, 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;
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index 97169548939cd..590a60ffe034d 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -80,12 +80,6 @@ struct expand_data {
 	const char *rest;
 	struct object_id delta_base_oid;
 
-	/*
-	 * If mark_query is true, we do not expand anything, but rather
-	 * just mark the object_info with items we wish to query.
-	 */
-	int mark_query;
-
 	/*
 	 * Whether to split the input on whitespace before feeding it to
 	 * get_sha1; this is decided during the mark_query phase based on

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

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 01/20] cat-file: split expand_atom into 2 functions
Message-ID: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000%40eu-west-1.amazonses.com

```
Split expand_atom function into 2 different functions,
expand_atom_into_fields prepares variable for further filling,
(new) expand_atom creates resulting string.
Need that for further reusing of formatting logic from ref-filter.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 73 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f5fa4fd75af26..f783b39b9bd5c 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -217,47 +217,49 @@ 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 expand_atom_into_fields(struct strbuf *sb, 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 = &data->type;
+	else if (is_atom("objectsize", atom, len))
+		data->info.sizep = &data->size;
+	else if (is_atom("objectsize:disk", atom, len))
+		data->info.disk_sizep = &data->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(&data->oid));
-	} else if (is_atom("objecttype", atom, len)) {
-		if (data->mark_query)
-			data->info.typep = &data->type;
-		else
-			strbuf_addstr(sb, typename(data->type));
-	} else if (is_atom("objectsize", atom, len)) {
-		if (data->mark_query)
-			data->info.sizep = &data->size;
-		else
-			strbuf_addf(sb, "%lu", data->size);
-	} else if (is_atom("objectsize:disk", atom, len)) {
-		if (data->mark_query)
-			data->info.disk_sizep = &data->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(&data->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(&data->delta_base_oid));
-	} else
+	} else if (is_atom("deltabase", atom, len))
+		strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
+	else
 		die("unknown format element: %.*s", len, atom);
 }
 
-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;
@@ -265,7 +267,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)
+		expand_atom_into_fields(sb, 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/450

```

## Olga Telezhnaya, 2018-01-09 13:05

Subject: [PATCH 05/20] cat-file: move struct expand_data into ref-filter
Message-ID: <01020160db067ae4-79bd5ef7-3402-4333-93c1-c9ae8059680c-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020160db067ae4-79bd5ef7-3402-4333-93c1-c9ae8059680c-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020160db0679c9-799a0bc4-b6d1-43e2-ad3b-80be4e4c55e9-000000@eu-west-1.amazonses.com>

```
Need that for further reusing of formatting logic in cat-file.
Have plans to get rid of using expand_data in cat-file at all,
and use it only in ref-filter for collecting, formatting and printing
needed data.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c | 36 ------------------------------------
 ref-filter.h       | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 7655a9a726773..7fd5b960ad698 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,42 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	return 0;
 }
 
-struct expand_data {
-	struct object_id oid;
-	enum object_type type;
-	unsigned long size;
-	off_t disk_size;
-	const char *rest;
-	struct object_id delta_base_oid;
-
-	/*
-	 * If mark_query is true, we do not expand anything, but rather
-	 * just mark the object_info with items we wish to query.
-	 */
-	int mark_query;
-
-	/*
-	 * Whether to split the input on whitespace before feeding it to
-	 * get_sha1; this is decided during the mark_query phase based on
-	 * whether we have a %(rest) token in our format.
-	 */
-	int split_on_whitespace;
-
-	/*
-	 * After a mark_query run, this object_info is set up to be
-	 * passed to sha1_object_info_extended. It will point to the data
-	 * elements above, so you can retrieve the response from there.
-	 */
-	struct object_info info;
-
-	/*
-	 * This flag will be true if the requested batch format and options
-	 * don't require us to call sha1_object_info, which can then be
-	 * optimized out.
-	 */
-	unsigned skip_object_info : 1;
-};
-
 static int is_atom(const char *atom, const char *s, int slen)
 {
 	int alen = strlen(atom);
diff --git a/ref-filter.h b/ref-filter.h
index 0d98342b34319..16d00e4b1bded 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -72,6 +72,42 @@ struct ref_filter {
 		verbose;
 };
 
+struct expand_data {
+	struct object_id oid;
+	enum object_type type;
+	unsigned long size;
+	off_t disk_size;
+	const char *rest;
+	struct object_id delta_base_oid;
+
+	/*
+	 * If mark_query is true, we do not expand anything, but rather
+	 * just mark the object_info with items we wish to query.
+	 */
+	int mark_query;
+
+	/*
+	 * Whether to split the input on whitespace before feeding it to
+	 * get_sha1; this is decided during the mark_query phase based on
+	 * whether we have a %(rest) token in our format.
+	 */
+	int split_on_whitespace;
+
+	/*
+	 * After a mark_query run, this object_info is set up to be
+	 * passed to sha1_object_info_extended. It will point to the data
+	 * elements above, so you can retrieve the response from there.
+	 */
+	struct object_info info;
+
+	/*
+	 * This flag will be true if the requested batch format and options
+	 * don't require us to call sha1_object_info, which can then be
+	 * optimized out.
+	 */
+	unsigned skip_object_info : 1;
+};
+
 struct ref_format {
 	/*
 	 * Set these to define the format; make sure you call

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

```

## Junio C Hamano, 2018-01-09 22:04

Subject: Re: [PATCH 03/20] cat-file: rename variables in ref-filter
Message-ID: <xmqqtvvu3d0a.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqtvvu3d0a.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <01020160db067bde-7f500636-b80e-4099-a84e-2613126c9aa1-000000@eu-west-1.amazonses.com>

```
Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

> Rename some variables for easier reading.
> They point not to values, but to arrays.

Once the code is written and people start to build on top, a change
like this is not worth the code churn, especially because there are
two equally valid schools of naming convention.

 - When you have an array, each of whose 20 slots holds a single
   dosh, I would prefer to call the array dosh[20], not doshes[20],
   so that I can refer to the seventh dosh as "dosh[7]".

 - If you more often refer to the array as a whole (than you refer
   to individual elements) and want to stress the fact that the
   array holds multiple elements in it, I can understand that you
   may be tempted to call the whole array "doshes[]".

So please drop this and other "rename variables" patches from the
series.



```

## Junio C Hamano, 2018-01-09 22:16

Subject: Re: [PATCH 04/20] cat-file: make valid_atoms as a function parameter
Message-ID: <xmqqpo6i3cg0.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqpo6i3cg0.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <01020160db067ae8-81d31cf2-0f69-4f36-9de8-27e9716d2f4b-000000@eu-west-1.amazonses.com>

```
Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

> Make valid_atoms as a function parameter,
> there could be another variable further.
> Need that for further reusing of formatting logic in cat-file.c.
>
> Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored by: Jeff King <peff@peff.net>
> ---
>  ref-filter.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Please be a bit more careful about your title.  This change does not
seem to have anything to do with "cat-file".

I was not sure what you meant by "make X as a function parameter"
after reading the proposed log message twice, but I am guessing that
you are allowing these functions to operate on not just the global
singleton but a different instance of array.  

I also suspect that this step may make the references to the
valid_atom[] array somewhat inconsistent in that the functions that
are touched by this patch would refer to the passed-in array while
the remainder of the existing code still works on the global
singleton.  For example, parse_ref_filter_atom() is called by
verify_ref_format(), but this patch does not make _it_ take the
array as a parameter, and instead uses the global singleton, so
anybody who wants to use verify_ref_format() with different
valid_atom[] are SOL.  I am not saying that this inconsistency will
be a problem, but that the patch (including the proposed log
message) does not explain why it is not---and it should.

Thanks.

> diff --git a/ref-filter.c b/ref-filter.c
> index 1426f0c28bce7..2d6e81fe1ab00 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -396,6 +396,7 @@ struct atom_value {
>   * Used to parse format string and sort specifiers
>   */
>  static int parse_ref_filter_atom(const struct ref_format *format,
> +				 const struct valid_atom *valid_atoms, int n_atoms,
>  				 const char *atom, const char *ep)
>  {
>  	const char *sp;
> @@ -425,13 +426,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,
>  	atom_len = (arg ? arg : ep) - sp;
>  
>  	/* Is the atom a valid one? */
> -	for (i = 0; i < ARRAY_SIZE(valid_atoms); i++) {
> +	for (i = 0; i < n_atoms; i++) {
>  		int len = strlen(valid_atoms[i].name);
>  		if (len == atom_len && !memcmp(valid_atoms[i].name, sp, len))
>  			break;
>  	}
>  
> -	if (ARRAY_SIZE(valid_atoms) <= i)
> +	if (n_atoms <= i)
>  		die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
>  
>  	/* Add it in, including the deref prefix */
> @@ -708,7 +709,8 @@ 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, sp + 2, ep);
> +		at = parse_ref_filter_atom(format, valid_atoms,
> +					   ARRAY_SIZE(valid_atoms), sp + 2, ep);
>  		cp = ep + 1;
>  
>  		if (skip_prefix(used_atoms[at].name, "color:", &color))
> @@ -2139,7 +2141,9 @@ void format_ref_array_item(struct ref_array_item *info,
>  		if (cp < sp)
>  			append_literal(cp, sp, &state);
>  		get_ref_atom_value(info,
> -				   parse_ref_filter_atom(format, sp + 2, ep),
> +				   parse_ref_filter_atom(format, valid_atoms,
> +							 ARRAY_SIZE(valid_atoms),
> +							 sp + 2, ep),
>  				   &atomv);
>  		atomv->handler(atomv, &state);
>  	}
> @@ -2187,7 +2191,8 @@ static int parse_sorting_atom(const char *atom)
>  	 */
>  	struct ref_format dummy = REF_FORMAT_INIT;
>  	const char *end = atom + strlen(atom);
> -	return parse_ref_filter_atom(&dummy, atom, end);
> +	return parse_ref_filter_atom(&dummy, valid_atoms,
> +				     ARRAY_SIZE(valid_atoms), atom, end);
>  }
>  
>  /*  If no sorting option is given, use refname to sort as default */
>
> --
> https://github.com/git/git/pull/450

```

## Junio C Hamano, 2018-01-09 23:32

Subject: Re: [PATCH 07/20] cat-file: reuse parse_ref_filter_atom
Message-ID: <xmqqh8ru38y2.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqh8ru38y2.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <01020160db067b4f-60d6012f-7cb8-4ba2-891f-1d1ed5bb5786-000000@eu-west-1.amazonses.com>

```
Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

> +static int is_atom(const char *atom, const char *s, int slen)
> +{
> +...
> +}
> +
> +static void expand_atom_into_fields(const char *atom, int len,
> +				    struct expand_data *data)
> +{
> +...
> +}
> ...
> -static int is_atom(const char *atom, const char *s, int slen)
> -{
> -...
> -}
> -
> -static void expand_atom_into_fields(const char *atom, int len,
> -				    struct expand_data *data)
> -{
> -...
> -}
> -
>  /*
>   * Make sure the format string is well formed, and parse out
>   * the used atoms.

There is no need to move these in this step if the previous step
planned ahead well, and that is something you can correct by
pretending to be perfect with "rebase -i" ;-) That also will help
reduce reviewers' load.

> @@ -726,6 +741,7 @@ int verify_ref_format(struct ref_format *format)
>  {
>  	const char *cp, *sp;
>  
> +	cat_file_info = format->cat_file_data;
>  	format->need_color_reset_at_eol = 0;
>  	for (cp = format->format; *cp && (sp = find_next(cp)); ) {
>  		const char *color, *ep = strchr(sp, ')');
> @@ -736,9 +752,10 @@ int verify_ref_format(struct ref_format *format)
>  		/* sp points at "%(" and ep points at the closing ")" */
>  
>  		if (format->cat_file_data)
> -			expand_atom_into_fields(sp + 2, ep - sp - 2,
> -						format->cat_file_data);
> -		else
> +		{
> +			at = parse_ref_filter_atom(format, valid_cat_file_atoms,
> +						   ARRAY_SIZE(valid_cat_file_atoms), sp + 2, ep);
> +		} else
>  		{
>  			at = parse_ref_filter_atom(format, valid_atoms,
>  						   ARRAY_SIZE(valid_atoms), sp + 2, ep);

if/else with bodies that are compound statements are formatted like
so:

	if (condition) {
		do this;
	} else {
		do that;
		do that, too;
	}


```

## Оля Тележная, 2018-01-10 07:07

Subject: Re: [PATCH 03/20] cat-file: rename variables in ref-filter
Message-ID: <CAL21BmnBcYD1ws+2e18o-X4awzFteMkZ0fGYtoJyYdB51JqdzQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAL21BmnBcYD1ws%2B2e18o-X4awzFteMkZ0fGYtoJyYdB51JqdzQ%40mail.gmail.com
In-Reply-To: <xmqqtvvu3d0a.fsf@gitster.mtv.corp.google.com>

```
2018-01-10 1:04 GMT+03:00 Junio C Hamano <gitster@pobox.com>:
> Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:
>
>> Rename some variables for easier reading.
>> They point not to values, but to arrays.
>
> Once the code is written and people start to build on top, a change
> like this is not worth the code churn, especially because there are
> two equally valid schools of naming convention.
>
>  - When you have an array, each of whose 20 slots holds a single
>    dosh, I would prefer to call the array dosh[20], not doshes[20],
>    so that I can refer to the seventh dosh as "dosh[7]".
>
>  - If you more often refer to the array as a whole (than you refer
>    to individual elements) and want to stress the fact that the
>    array holds multiple elements in it, I can understand that you
>    may be tempted to call the whole array "doshes[]".
>
> So please drop this and other "rename variables" patches from the
> series.
>
>

OK, I will revert that. I have done this because it's hard for me to
keep in mind that it's not just a simple pointer to a single value,
and I tried to make the code more intuitive.

```