[WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public

Rename some of the functions and make them publically available.
This is a preparatory step for moving code from 'for-each-ref'
to 'ref-filter' to make meaningful, targeted services available to
other commands via public APIs.

Based-on-patch-by: Jeff King [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Matthieu Moy [off-list ref]
Signed-off-by: Karthik Nayak <redacted>
---
 builtin/for-each-ref.c | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f896e1c..8fed04b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c

@@ -112,7 +112,7 @@ static int need_color_reset_at_eol;
 /*
  * Used to parse format string and sort specifiers
  */
-static int parse_atom(const char *atom, const char *ep)
+int parse_ref_filter_atom(const char *atom, const char *ep)
 {
 	const char *sp;
 	int i, at;

@@ -189,7 +189,7 @@ static const char *find_next(const char *cp)
  * Make sure the format string is well formed, and parse out
  * the used atoms.
  */
-static int verify_format(const char *format)
+int verify_ref_format(const char *format)
 {
 	const char *cp, *sp;
 

@@ -201,7 +201,7 @@ static int verify_format(const char *format)
 		if (!ep)
 			return error("malformed format string %s", sp);
 		/* sp points at "%(" and ep points at the closing ")" */
-		at = parse_atom(sp + 2, ep);
+		at = parse_ref_filter_atom(sp + 2, ep);
 		cp = ep + 1;
 
 		if (skip_prefix(used_atom[at], "color:", &color))

@@ -408,7 +408,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	/*
 	 * We got here because atomname ends in "date" or "date<something>";
 	 * it's not possible that <something> is not ":<format>" because
-	 * parse_atom() wouldn't have allowed it, so we can assume that no
+	 * parse_ref_filter_atom() wouldn't have allowed it, so we can assume that no
 	 * ":" means no format is specified, and use the default.
 	 */
 	formatp = strchr(atomname, ':');

@@ -835,7 +835,7 @@ static void populate_value(struct ref_array_item *ref)
  * Given a ref, return the value for the atom.  This lazily gets value
  * out of the object by calling populate value.
  */
-static void get_value(struct ref_array_item *ref, int atom, struct atom_value **v)
+static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
 {
 	if (!ref->value) {
 		populate_value(ref);

@@ -882,10 +882,10 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 }
 
 /*
- * A call-back given to for_each_ref().  Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
  * later object processing.
  */
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
 	struct ref_filter *filter = &ref_cbdata->filter;

@@ -925,8 +925,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref
 	int cmp;
 	cmp_type cmp_type = used_atom_type[s->atom];
 
-	get_value(a, s->atom, &va);
-	get_value(b, s->atom, &vb);
+	get_ref_atom_value(a, s->atom, &va);
+	get_ref_atom_value(b, s->atom, &vb);
 	switch (cmp_type) {
 	case FIELD_STR:
 		cmp = strcmp(va->s, vb->s);

@@ -958,7 +958,7 @@ static int compare_refs(const void *a_, const void *b_)
 	return 0;
 }
 
-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)
 {
 	ref_sort = sort;
 	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);

@@ -1028,7 +1028,7 @@ static void emit(const char *cp, const char *ep)
 	}
 }
 
-static void show_ref(struct ref_array_item *info, const char *format, int quote_style)
+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
 {
 	const char *cp, *sp, *ep;
 

@@ -1038,7 +1038,7 @@ static void show_ref(struct ref_array_item *info, const char *format, int quote_
 		ep = strchr(sp, ')');
 		if (cp < sp)
 			emit(cp, sp);
-		get_value(info, parse_atom(sp + 2, ep), &atomv);
+		get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
 		print_value(atomv, quote_style);
 	}
 	if (*cp) {

@@ -1057,18 +1057,19 @@ static void show_ref(struct ref_array_item *info, const char *format, int quote_
 	putchar('\n');
 }
 
-static struct ref_sort *default_sort(void)
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sort *ref_default_sort(void)
 {
 	static const char cstr_name[] = "refname";
 
 	struct ref_sort *sort = xcalloc(1, sizeof(*sort));
 
 	sort->next = NULL;
-	sort->atom = parse_atom(cstr_name, cstr_name + strlen(cstr_name));
+	sort->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
 	return sort;
 }
 
-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)
 {
 	struct ref_sort **sort_tail = opt->value;
 	struct ref_sort *s;

@@ -1086,7 +1087,7 @@ static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
 		arg++;
 	}
 	len = strlen(arg);
-	s->atom = parse_atom(arg, arg+len);
+	s->atom = parse_ref_filter_atom(arg, arg+len);
 	return 0;
 }
 

@@ -1118,7 +1119,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
 		OPT_CALLBACK(0 , "sort", sort_tail, N_("key"),
-			    N_("field name to sort on"), &opt_parse_sort),
+			    N_("field name to sort on"), &opt_parse_ref_sort),
 		OPT_END(),
 	};
 

@@ -1131,24 +1132,24 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		error("more than one quoting style?");
 		usage_with_options(for_each_ref_usage, opts);
 	}
-	if (verify_format(format))
+	if (verify_ref_format(format))
 		usage_with_options(for_each_ref_usage, opts);
 
 	if (!sort)
-		sort = default_sort();
+		sort = ref_default_sort();
 
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
 	ref_cbdata.filter.name_patterns = argv;
-	for_each_rawref(grab_single_ref, &ref_cbdata);
+	for_each_rawref(ref_filter_handler, &ref_cbdata);
 
-	sort_refs(sort, &ref_cbdata.array);
+	sort_ref_array(sort, &ref_cbdata.array);
 
 	if (!maxcount || ref_cbdata.array.nr < maxcount)
 		maxcount = ref_cbdata.array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref(ref_cbdata.array.items[i], format, quote_style);
+		show_ref_array_item(ref_cbdata.array.items[i], format, quote_style);
 	ref_filter_clear_data(&ref_cbdata);
 	return 0;
 }

-- 
2.4.2

On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak [off-list ref] wrote:

Rename some of the functions and make them publically available.

s/publically/publicly/

Sneaking in whitespace change?

  * later object processing.
  */
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
        struct ref_filter_cbdata *ref_cbdata = cb_data;
        struct ref_filter *filter = &ref_cbdata->filter;

On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak [off-list ref] wrote:

-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)

It is probably better to call the above function ref_array_sort()...

[...]

-static struct ref_sort *default_sort(void)
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sort *ref_default_sort(void)

... especially since you call the above ref_default_sort()...

-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)

... and the above opt_parse_sort().

On Sun, May 31, 2015 at 10:04 AM, Christian Couder
[off-list ref] wrote:

On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak [off-list ref] wrote:

quoted

-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)

It is probably better to call the above function ref_array_sort()...

[...]

quoted

-static struct ref_sort *default_sort(void)
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sort *ref_default_sort(void)

... especially since you call the above ref_default_sort()...

quoted

-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)

... and the above opt_parse_sort().

After saying that I realize that these two other functions are not
doing the same thing.
This might suggest that they are not named very well as well.

On 05/31/2015 08:51 AM, Eric Sunshine wrote:

On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak [off-list ref] wrote:

quoted

Rename some of the functions and make them publically available.

s/publically/publicly/

quoted

This is a preparatory step for moving code from 'for-each-ref'
to 'ref-filter' to make meaningful, targeted services available to
other commands via public APIs.

Based-on-patch-by: Jeff King [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Matthieu Moy [off-list ref]
Signed-off-by: Karthik Nayak <redacted>
---

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f896e1c..8fed04b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c

@@ -882,10 +882,10 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
  }

  /*
- * A call-back given to for_each_ref().  Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for

Sneaking in whitespace change?

quoted

   * later object processing.
   */
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
  {
         struct ref_filter_cbdata *ref_cbdata = cb_data;
         struct ref_filter *filter = &ref_cbdata->filter;

Noted. Will fix!

-- 
Regards,
Karthik

On 05/31/2015 01:41 PM, Christian Couder wrote:

On Sun, May 31, 2015 at 10:04 AM, Christian Couder
[off-list ref] wrote:

quoted

On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak [off-list ref] wrote:

quoted

-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)

It is probably better to call the above function ref_array_sort()...

[...]

quoted

-static struct ref_sort *default_sort(void)
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sort *ref_default_sort(void)

... especially since you call the above ref_default_sort()...

quoted

-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)

... and the above opt_parse_sort().

After saying that I realize that these two other functions are not
doing the same thing.
This might suggest that they are not named very well as well.

What do you mean by "not doing the same thing."

-- 
Regards,
Karthik

On Sun, May 31, 2015 at 11:17 AM, Karthik Nayak [off-list ref] wrote:

On 05/31/2015 01:41 PM, Christian Couder wrote:

quoted

On Sun, May 31, 2015 at 10:04 AM, Christian Couder
[off-list ref] wrote:

quoted

On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak [off-list ref]
wrote:

quoted

-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)

It is probably better to call the above function ref_array_sort()...

[...]

quoted

-static struct ref_sort *default_sort(void)
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sort *ref_default_sort(void)

... especially since you call the above ref_default_sort()...

quoted

-static int opt_parse_sort(const struct option *opt, const char *arg,
int unset)
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int
unset)

... and the above opt_parse_sort().

After saying that I realize that these two other functions are not
doing the same thing.
This might suggest that they are not named very well as well.

What do you mean by "not doing the same thing."

sort_ref_array() is actually sorting a ref_array, while
ref_default_sort() and opt_parse_ref_sort() are not sorting anything.

Maybe it would all be clearer with a renaming like the following:

sort_refs() -> ref_array_sort()
struct ref_sort -> struct ref_sort_criteria
default_sort() -> ref_default_sort_criteria()
opt_parse_sort() -> opt_parse_ref_sort_criteria()

On 05/31/2015 07:33 PM, Christian Couder wrote:

On Sun, May 31, 2015 at 11:17 AM, Karthik Nayak [off-list ref] wrote:

quoted

On 05/31/2015 01:41 PM, Christian Couder wrote:

quoted

On Sun, May 31, 2015 at 10:04 AM, Christian Couder
[off-list ref] wrote:

quoted

On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak [off-list ref]
wrote:

quoted

-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)

It is probably better to call the above function ref_array_sort()...

[...]

quoted

-static struct ref_sort *default_sort(void)
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sort *ref_default_sort(void)

... especially since you call the above ref_default_sort()...

quoted

-static int opt_parse_sort(const struct option *opt, const char *arg,
int unset)
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int
unset)

... and the above opt_parse_sort().

After saying that I realize that these two other functions are not
doing the same thing.
This might suggest that they are not named very well as well.

What do you mean by "not doing the same thing."

sort_ref_array() is actually sorting a ref_array, while
ref_default_sort() and opt_parse_ref_sort() are not sorting anything.

Maybe it would all be clearer with a renaming like the following:

sort_refs() -> ref_array_sort()
struct ref_sort -> struct ref_sort_criteria
default_sort() -> ref_default_sort_criteria()
opt_parse_sort() -> opt_parse_ref_sort_criteria()

Thanks will follow this, but will change opt_parse_ref_sort_criteria() to
parse_opt_ref_sort_criteria() as this is what is commonly followed.
-- 
Regards,
Karthik