[PATCH v7 0/12] Create ref-filter from for-each-ref

[PATCH v7 0/12] Create ref-filter from for-each-ref

[Karthik Nayak]

The previous version of this patch can be found here:
http://thread.gmane.org/gmane.comp.version-control.git/270922

Changes found in this version:
*    Various changes to the 'filter_refs()' function.
*    Split 'for-each-ref: clean up code' into two commits.
*    Other small changes.

-- 
Regards,
Karthik

[PATCH v7 01/12] for-each-ref: extract helper functions out of grab_single_ref()

[Karthik Nayak]

Extract two helper functions out of grab_single_ref(). Firstly,
new_refinfo() which is used to allocate memory for a new refinfo
structure and copy the objectname, refname and flag to it.
Secondly, match_name_as_path() which when given an array of patterns
and the refname checks if the refname matches any of the patterns
given while the pattern is a pathname, also supports wildcard
characters.

This is a preperatory patch for restructuring 'for-each-ref' and
eventually moving most of it to 'ref-filter' to provide the
functionality to similar commands via public API's.

Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 64 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f7e51a7..67c8b62 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -851,6 +851,44 @@ struct grab_ref_cbdata {
 };
 
 /*
+ * Return 1 if the refname matches with one of the patterns,
+ * otherwise 0.  The patterns can be literal prefix (e.g. a
+ * refname "refs/heads/master" matches a pattern "refs/heads/")
+ * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
+ */
+static int match_name_as_path(const char **pattern, const char *refname)
+{
+	int namelen = strlen(refname);
+	for (; *pattern; pattern++) {
+		const char *p = *pattern;
+		int plen = strlen(p);
+
+		if ((plen <= namelen) &&
+		    !strncmp(refname, p, plen) &&
+		    (refname[plen] == '\0' ||
+		     refname[plen] == '/' ||
+		     p[plen-1] == '/'))
+			return 1;
+		if (!wildmatch(p, refname, WM_PATHNAME, NULL))
+			return 1;
+	}
+	return 0;
+}
+
+/* Allocate space for a new refinfo and copy the objectname and flag to it */
+static struct refinfo *new_refinfo(const char *refname,
+				   const unsigned char *objectname,
+				   int flag)
+{
+	struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+	ref->refname = xstrdup(refname);
+	hashcpy(ref->objectname, objectname);
+	ref->flag = flag;
+
+	return ref;
+}
+
+/*
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
  */
@@ -866,35 +904,15 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 		  return 0;
 	}
 
-	if (*cb->grab_pattern) {
-		const char **pattern;
-		int namelen = strlen(refname);
-		for (pattern = cb->grab_pattern; *pattern; pattern++) {
-			const char *p = *pattern;
-			int plen = strlen(p);
-
-			if ((plen <= namelen) &&
-			    !strncmp(refname, p, plen) &&
-			    (refname[plen] == '\0' ||
-			     refname[plen] == '/' ||
-			     p[plen-1] == '/'))
-				break;
-			if (!wildmatch(p, refname, WM_PATHNAME, NULL))
-				break;
-		}
-		if (!*pattern)
-			return 0;
-	}
+	if (*cb->grab_pattern && !match_name_as_path(cb->grab_pattern, refname))
+		return 0;
 
 	/*
 	 * We do not open the object yet; sort may only need refname
 	 * to do its job and the resulting list may yet to be pruned
 	 * by maxcount logic.
 	 */
-	ref = xcalloc(1, sizeof(*ref));
-	ref->refname = xstrdup(refname);
-	hashcpy(ref->objectname, oid->hash);
-	ref->flag = flag;
+	ref = new_refinfo(refname, oid->hash, flag);
 
 	cnt = cb->grab_cnt;
 	REALLOC_ARRAY(cb->grab_array, cnt + 1);
-- 
2.4.2

[PATCH v7 02/12] for-each-ref: clean up code

[Karthik Nayak]

In 'grab_single_ref()' remove the extra count variable 'cnt' and
use the variable 'grab_cnt' of structure 'grab_ref_cbdata' directly
instead.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 67c8b62..0dd2df2 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -897,7 +897,6 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 {
 	struct grab_ref_cbdata *cb = cb_data;
 	struct refinfo *ref;
-	int cnt;
 
 	if (flag & REF_BAD_NAME) {
 		  warning("ignoring ref with broken name %s", refname);
@@ -914,10 +913,8 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	 */
 	ref = new_refinfo(refname, oid->hash, flag);
 
-	cnt = cb->grab_cnt;
-	REALLOC_ARRAY(cb->grab_array, cnt + 1);
-	cb->grab_array[cnt++] = ref;
-	cb->grab_cnt = cnt;
+	REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
+	cb->grab_array[cb->grab_cnt++] = ref;
 	return 0;
 }
 
-- 
2.4.2

[PATCH v7 03/12] for-each-ref: change comment in ref_sort

[Karthik Nayak]

The comment in 'ref_sort' hasn't been changed 9f613dd.
Change the comment to reflect changes made in the code since
9f613dd.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 0dd2df2..bfad03f 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -27,7 +27,7 @@ struct atom_value {
 
 struct ref_sort {
 	struct ref_sort *next;
-	int atom; /* index into used_atom array */
+	int atom; /* index into 'struct atom_value *' array */
 	unsigned reverse : 1;
 };
 
-- 
2.4.2

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> The comment in 'ref_sort' hasn't been changed 9f613dd.

Bad grammar?  "hasn't been changed since 9f613dd", perhaps?

But more importantly, don't just give an abbreviated object name.  I
think "the comment hasn't changed since the for-each-ref command was
originally introduced" is what you meant to say, and it is OK to
append "since 9f613ddd (Add git-for-each-ref: helper for language
bindings, 2006-09-15)" to that sentence as a supporting material.

> Change the comment to reflect changes made in the code since
> 9f613dd.

What change since 9f613dd do you have in mind, exactly, though?

I do not think the fact that this field indexes into used_atom[]
array has ever changed during the life of this implementation.
I see "static const char **used_atom;" in builtin/for-each-ref.c
still in the 'master', and that is the array that holds the atoms
that are used by the end-user request.

So I do not think "The comment was there from the beginning, it
described the initial implementation, the implementation was updated
and the comment has become stale" is a good justification for this
change, as I do not think that is what has happened here.

You may be changing used_atom to something else later in your
series, but then isn't that commit the appropriate place to update
this comment?

> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  builtin/for-each-ref.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 0dd2df2..bfad03f 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -27,7 +27,7 @@ struct atom_value {
>  
>  struct ref_sort {
>  	struct ref_sort *next;
> -	int atom; /* index into used_atom array */
> +	int atom; /* index into 'struct atom_value *' array */
>  	unsigned reverse : 1;
>  };

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

[Karthik Nayak]

On 06/12/2015 11:10 PM, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The comment in 'ref_sort' hasn't been changed 9f613dd.
>
> Bad grammar?  "hasn't been changed since 9f613dd", perhaps?

Yes! thanks :)

>
> But more importantly, don't just give an abbreviated object name.  I
> think "the comment hasn't changed since the for-each-ref command was
> originally introduced" is what you meant to say, and it is OK to
> append "since 9f613ddd (Add git-for-each-ref: helper for language
> bindings, 2006-09-15)" to that sentence as a supporting material.
>

Ok will do.

>> Change the comment to reflect changes made in the code since
>> 9f613dd.
>
> What change since 9f613dd do you have in mind, exactly, though?

Well initially the atoms were indexed into used_atom array, which
later was removed. Hence the comment becomes obsolete.

>
> I do not think the fact that this field indexes into used_atom[]
> array has ever changed during the life of this implementation.
> I see "static const char **used_atom;" in builtin/for-each-ref.c
> still in the 'master', and that is the array that holds the atoms
> that are used by the end-user request.
>
> So I do not think "The comment was there from the beginning, it
> described the initial implementation, the implementation was updated
> and the comment has become stale" is a good justification for this
> change, as I do not think that is what has happened here.
>
> You may be changing used_atom to something else later in your
> series, but then isn't that commit the appropriate place to update
> this comment?
>

But isn't that what happened here, the code was altered but the comment
was left the way it is.

What do you suggest I do?

-- 
Regards,
Karthik

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

>> What change since 9f613dd do you have in mind, exactly, though?
>
> Well initially the atoms were indexed into used_atom array, which
> later was removed. Hence the comment becomes obsolete.

Later in which commit?  In builtin/for-each-ref.c in the version
after applying patches 1-3 of this series on top of master, I still
see used_atom[] array there, so...?

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

[Karthik Nayak]

On 06/12/2015 11:34 PM, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>> What change since 9f613dd do you have in mind, exactly, though?
>>
>> Well initially the atoms were indexed into used_atom array, which
>> later was removed. Hence the comment becomes obsolete.
>
> Later in which commit?  In builtin/for-each-ref.c in the version
> after applying patches 1-3 of this series on top of master, I still
> see used_atom[] array there, so...?
>

Uh! My bad.
Ignore this! I think I got confused, On having a look now that patch is
not needed. Sorry.

-- 
Regards,
Karthik

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

[Christian Couder]

On Fri, Jun 12, 2015 at 8:29 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
> On 06/12/2015 11:34 PM, Junio C Hamano wrote:
>>
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>>> What change since 9f613dd do you have in mind, exactly, though?
>>>
>>>
>>> Well initially the atoms were indexed into used_atom array, which
>>> later was removed. Hence the comment becomes obsolete.
>>
>>
>> Later in which commit?  In builtin/for-each-ref.c in the version
>> after applying patches 1-3 of this series on top of master, I still
>> see used_atom[] array there, so...?
>>
>
> Uh! My bad.
> Ignore this! I think I got confused, On having a look now that patch is
> not needed. Sorry.

I think it is needed later when "struct ref_sort" is moved into
ref-filter.h, because then the used_atom[] array is not moved.

So either:

1) you update the comment when you move "struct ref_sort" into
ref-filter.h, but then the downside is that there is not only code
movement in the patch that moves "struct ref_sort" into ref-filter.h,
or

2) you explain that, as "struct ref_sort" will be moved later while
the used_atom[] array will not be moved, the direct connection between
the comment and used_atom[] will be broken, and you need to prepare
for that because you want to avoid solution 1) as you want only code
movement when moving "struct ref_sort" into ref-filter.h.

Hope this helps,
Christian.

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> I think it is needed later when "struct ref_sort" is moved into
> ref-filter.h, because then the used_atom[] array is not moved.

Now I am confused.  used_atom[] is the mechanism we use to give a
small integer to each atom used in the %(placeholder) string, so
that we do not have to refer to them as "placeholder" string and we
do not have to call !strcmp() to compare for equality.  How can a
field in ref_sort refer to it internally if used_atom[] is not
visible?

Indeed, ref-filter.c after the series does have used_atom[] and
get_ref_atom_value() does use atom to index into it.  So these two
lines do not make much sense to me.  I am puzzled.

If by "moved" you are referring to the fact that the structure and
its fields are exposed to the callers of the API while the
implementation detail of the mechanism to give short integer to each
used atom is not exposed to them, then I do agree that the comment
to the structure field as the external API definition should not
talk about used_atom[] array.

Perhaps in 03/12, you can stop talking about the implementation
(i.e. the value is used to index into used_atom[] to get to the
original string) and instead start talking about what the value
means to the callers (that are still internal to for-each-ref
implementation), to make things better.

Having said that, I am not sure if there is a sensible description
for that field if you avoid exposing the implementation detail.

You would probably want to start by asking what that value means.
For (evantual) external callers, the number is what they get from
parse_ref_filter_atom(); calling that function is the only way they
can get an appropriate value to stuff in the field, and
parse_opt_ref_sorting() is the most likely function external callers
use to make that happen.

"The number internally used to represent an attribute of a ref used
when sorting the set of refs" would be one way to say what it is
without exposing the implementation detail to the readers.

But does that help anybody?  I doubt it.  It is mouthful for
external users, and it is not concrete enough for implementors.

So either

 - treat ref-filter.h as information for API users, and not talk
   about what it means at all, or

 - treat ref-filter.h as information for both API users and API
   implementors, and describe it as an index into used_atom[],
   i.e. do not change anything.

I'd say the latter is a more sensible way to go.  I think it is also
OK to change this comment to "index into used_atom array (internal)"
when ref-filter.h is created as an external API definition.

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

[karthik nayak]

On Sat, Jun 13, 2015 at 1:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> I think it is needed later when "struct ref_sort" is moved into
>> ref-filter.h, because then the used_atom[] array is not moved.
>
> Now I am confused.  used_atom[] is the mechanism we use to give a
> small integer to each atom used in the %(placeholder) string, so
> that we do not have to refer to them as "placeholder" string and we
> do not have to call !strcmp() to compare for equality.  How can a
> field in ref_sort refer to it internally if used_atom[] is not
> visible?
>
> Indeed, ref-filter.c after the series does have used_atom[] and
> get_ref_atom_value() does use atom to index into it.  So these two
> lines do not make much sense to me.  I am puzzled.
>
> If by "moved" you are referring to the fact that the structure and
> its fields are exposed to the callers of the API while the
> implementation detail of the mechanism to give short integer to each
> used atom is not exposed to them, then I do agree that the comment
> to the structure field as the external API definition should not
> talk about used_atom[] array.
>
> Perhaps in 03/12, you can stop talking about the implementation
> (i.e. the value is used to index into used_atom[] to get to the
> original string) and instead start talking about what the value
> means to the callers (that are still internal to for-each-ref
> implementation), to make things better.
>
> Having said that, I am not sure if there is a sensible description
> for that field if you avoid exposing the implementation detail.
>
> You would probably want to start by asking what that value means.
> For (evantual) external callers, the number is what they get from
> parse_ref_filter_atom(); calling that function is the only way they
> can get an appropriate value to stuff in the field, and
> parse_opt_ref_sorting() is the most likely function external callers
> use to make that happen.
>
> "The number internally used to represent an attribute of a ref used
> when sorting the set of refs" would be one way to say what it is
> without exposing the implementation detail to the readers.
>
> But does that help anybody?  I doubt it.  It is mouthful for
> external users, and it is not concrete enough for implementors.
>
> So either
>
>  - treat ref-filter.h as information for API users, and not talk
>    about what it means at all, or
>
>  - treat ref-filter.h as information for both API users and API
>    implementors, and describe it as an index into used_atom[],
>    i.e. do not change anything.
>
> I'd say the latter is a more sensible way to go.  I think it is also
> OK to change this comment to "index into used_atom array (internal)"
> when ref-filter.h is created as an external API definition.

Like you said, the comment is still relevent to the code.
So I guess of the two options suggested by you the option of keeping the
comment and just adding "(internal)" while the code is moved to ref-filter.h
seems to be the best solution. This would eliminate the need for PATCH 03.

--
Regards,
Karthik

[PATCH v7 04/12] for-each-ref: rename 'refinfo' to 'ref_array_item'

[Karthik Nayak]

Rename 'refinfo' to 'ref_array_item' as a preparatory step for
introduction of new structures in the forthcoming patch.

Re-order the fields in 'ref_array_item' so that refname can be
eventually converted to a FLEX_ARRAY.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index bfad03f..8c44b49 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -31,12 +31,12 @@ struct ref_sort {
 	unsigned reverse : 1;
 };
 
-struct refinfo {
-	char *refname;
+struct ref_array_item {
 	unsigned char objectname[20];
 	int flag;
 	const char *symref;
 	struct atom_value *value;
+	char *refname;
 };
 
 static struct {
@@ -86,7 +86,7 @@ static struct {
  * a "*" to denote deref_tag().
  *
  * We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects.  refinfo
+ * of properties that we need to extract out of objects.  ref_array_item
  * structure will hold an array of values extracted that can be
  * indexed with the "atom number", which is an index into this
  * array.
@@ -623,7 +623,7 @@ static inline char *copy_advance(char *dst, const char *src)
 /*
  * Parse the object referred by ref, and grab needed value.
  */
-static void populate_value(struct refinfo *ref)
+static void populate_value(struct ref_array_item *ref)
 {
 	void *buf;
 	struct object *obj;
@@ -835,7 +835,7 @@ static void populate_value(struct refinfo *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 refinfo *ref, int atom, struct atom_value **v)
+static void get_value(struct ref_array_item *ref, int atom, struct atom_value **v)
 {
 	if (!ref->value) {
 		populate_value(ref);
@@ -845,7 +845,7 @@ static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
 }
 
 struct grab_ref_cbdata {
-	struct refinfo **grab_array;
+	struct ref_array_item **grab_array;
 	const char **grab_pattern;
 	int grab_cnt;
 };
@@ -875,12 +875,12 @@ static int match_name_as_path(const char **pattern, const char *refname)
 	return 0;
 }
 
-/* Allocate space for a new refinfo and copy the objectname and flag to it */
-static struct refinfo *new_refinfo(const char *refname,
-				   const unsigned char *objectname,
-				   int flag)
+/* Allocate space for a new ref_array_item and copy the objectname and flag to it */
+static struct ref_array_item *new_ref_array_item(const char *refname,
+						 const unsigned char *objectname,
+						 int flag)
 {
-	struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
 	ref->refname = xstrdup(refname);
 	hashcpy(ref->objectname, objectname);
 	ref->flag = flag;
@@ -896,7 +896,7 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 			   int flag, void *cb_data)
 {
 	struct grab_ref_cbdata *cb = cb_data;
-	struct refinfo *ref;
+	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
 		  warning("ignoring ref with broken name %s", refname);
@@ -911,14 +911,14 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	 * to do its job and the resulting list may yet to be pruned
 	 * by maxcount logic.
 	 */
-	ref = new_refinfo(refname, oid->hash, flag);
+	ref = new_ref_array_item(refname, oid->hash, flag);
 
 	REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
 	cb->grab_array[cb->grab_cnt++] = ref;
 	return 0;
 }
 
-static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b)
+static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
 	int cmp;
@@ -945,8 +945,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b
 static struct ref_sort *ref_sort;
 static int compare_refs(const void *a_, const void *b_)
 {
-	struct refinfo *a = *((struct refinfo **)a_);
-	struct refinfo *b = *((struct refinfo **)b_);
+	struct ref_array_item *a = *((struct ref_array_item **)a_);
+	struct ref_array_item *b = *((struct ref_array_item **)b_);
 	struct ref_sort *s;
 
 	for (s = ref_sort; s; s = s->next) {
@@ -957,10 +957,10 @@ static int compare_refs(const void *a_, const void *b_)
 	return 0;
 }
 
-static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs)
+static void sort_refs(struct ref_sort *sort, struct ref_array_item **refs, int num_refs)
 {
 	ref_sort = sort;
-	qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
+	qsort(refs, num_refs, sizeof(struct ref_array_item *), compare_refs);
 }
 
 static void print_value(struct atom_value *v, int quote_style)
@@ -1027,7 +1027,7 @@ static void emit(const char *cp, const char *ep)
 	}
 }
 
-static void show_ref(struct refinfo *info, const char *format, int quote_style)
+static void show_ref(struct ref_array_item *info, const char *format, int quote_style)
 {
 	const char *cp, *sp, *ep;
 
@@ -1100,7 +1100,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
-	struct refinfo **refs;
+	struct ref_array_item **refs;
 	struct grab_ref_cbdata cbdata;
 
 	struct option opts[] = {
-- 
2.4.2

[PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

[Karthik Nayak]

Introduce 'ref_filter_cbdata' which will hold 'ref_filter'
(conditions to filter the refs on) and 'ref_array' (the array
of ref_array_items). Modify the code to use these new structures.

This is a preparatory patch to eventually move code from 'for-each-ref'
to 'ref-filter' and make it publicly available.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 54 ++++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 8c44b49..913d757 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -39,6 +39,20 @@ struct ref_array_item {
 	char *refname;
 };
 
+struct ref_array {
+	int nr, alloc;
+	struct ref_array_item **items;
+};
+
+struct ref_filter {
+	const char **name_patterns;
+};
+
+struct ref_filter_cbdata {
+	struct ref_array array;
+	struct ref_filter filter;
+};
+
 static struct {
 	const char *name;
 	cmp_type cmp_type;
@@ -844,12 +858,6 @@ static void get_value(struct ref_array_item *ref, int atom, struct atom_value **
 	*v = &ref->value[atom];
 }
 
-struct grab_ref_cbdata {
-	struct ref_array_item **grab_array;
-	const char **grab_pattern;
-	int grab_cnt;
-};
-
 /*
  * Return 1 if the refname matches with one of the patterns,
  * otherwise 0.  The patterns can be literal prefix (e.g. a
@@ -895,7 +903,8 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 static int grab_single_ref(const char *refname, const struct object_id *oid,
 			   int flag, void *cb_data)
 {
-	struct grab_ref_cbdata *cb = cb_data;
+	struct ref_filter_cbdata *ref_cbdata = cb_data;
+	struct ref_filter *filter = &ref_cbdata->filter;
 	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
@@ -903,7 +912,7 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 		  return 0;
 	}
 
-	if (*cb->grab_pattern && !match_name_as_path(cb->grab_pattern, refname))
+	if (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))
 		return 0;
 
 	/*
@@ -913,8 +922,8 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	 */
 	ref = new_ref_array_item(refname, oid->hash, flag);
 
-	REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
-	cb->grab_array[cb->grab_cnt++] = ref;
+	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
+	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
 	return 0;
 }
 
@@ -957,10 +966,10 @@ static int compare_refs(const void *a_, const void *b_)
 	return 0;
 }
 
-static void sort_refs(struct ref_sort *sort, struct ref_array_item **refs, int num_refs)
+static void sort_refs(struct ref_sort *sort, struct ref_array *array)
 {
 	ref_sort = sort;
-	qsort(refs, num_refs, sizeof(struct ref_array_item *), compare_refs);
+	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
 }
 
 static void print_value(struct atom_value *v, int quote_style)
@@ -1096,12 +1105,11 @@ static char const * const for_each_ref_usage[] = {
 
 int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
-	int i, num_refs;
+	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
-	struct ref_array_item **refs;
-	struct grab_ref_cbdata cbdata;
+	struct ref_filter_cbdata ref_cbdata;
 
 	struct option opts[] = {
 		OPT_BIT('s', "shell", &quote_style,
@@ -1139,17 +1147,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
-	memset(&cbdata, 0, sizeof(cbdata));
-	cbdata.grab_pattern = argv;
-	for_each_rawref(grab_single_ref, &cbdata);
-	refs = cbdata.grab_array;
-	num_refs = cbdata.grab_cnt;
+	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
+	ref_cbdata.filter.name_patterns = argv;
+	for_each_rawref(grab_single_ref, &ref_cbdata);
 
-	sort_refs(sort, refs, num_refs);
+	sort_refs(sort, &ref_cbdata.array);
 
-	if (!maxcount || num_refs < maxcount)
-		maxcount = num_refs;
+	if (!maxcount || ref_cbdata.array.nr < maxcount)
+		maxcount = ref_cbdata.array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref(refs[i], format, quote_style);
+		show_ref(ref_cbdata.array.items[i], format, quote_style);
 	return 0;
 }
-- 
2.4.2

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

[Matthieu Moy]

Karthik Nayak <karthik.188@gmail.com> writes:

> +struct ref_filter_cbdata {
> +	struct ref_array array;
> +	struct ref_filter filter;
> +};

I didn't notice this at first, but why introduce the structure like this
when you are going to turn it into pointers later in PATCH 7:

Karthik Nayak <karthik.188@gmail.com> writes:

> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -42,12 +45,17 @@ struct ref_filter {
>  };
>  
>  struct ref_filter_cbdata {
> -	struct ref_array array;
> -	struct ref_filter filter;
> +	struct ref_array *array;
> +	struct ref_filter *filter;
>  };

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

[Karthik Nayak]

On 06/11/2015 11:11 PM, Matthieu Moy wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > +struct ref_filter_cbdata {
> > +    struct ref_array array;
> > +    struct ref_filter filter;
> > +};
>
> I didn't notice this at first, but why introduce the structure like this
> when you are going to turn it into pointers later in PATCH 7:
Here it is serving to for-each-ref, so I kept it this way so as to ensure
that currently as per this patch

struct ref_filter_cbdata ref_cbdata;

would be the only declaration needed in for-each-ref.c
If I made them pointers here I would need to have

struct ref_filter_cbdata ref_cbdata;
struct ref_filter filter;
struct ref_array array;
ref_cbdata.filter = &filter;
ref_cbdata.array = &array;

And this seemed like an overhead.
If you still think I need to change the code to reflect the change early on
I will gladly do a re-roll.



-- 
Regards,
Karthik

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

[Matthieu Moy]

Karthik Nayak <karthik.188@gmail.com> writes:

> On 06/11/2015 11:11 PM, Matthieu Moy wrote:
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>> > +struct ref_filter_cbdata {
>> > +    struct ref_array array;
>> > +    struct ref_filter filter;
>> > +};
>>
>> I didn't notice this at first, but why introduce the structure like this
>> when you are going to turn it into pointers later in PATCH 7:
> Here it is serving to for-each-ref, so I kept it this way so as to ensure
> that currently as per this patch
>
> struct ref_filter_cbdata ref_cbdata;
>
> would be the only declaration needed in for-each-ref.c
> If I made them pointers here I would need to have
>
> struct ref_filter_cbdata ref_cbdata;
> struct ref_filter filter;
> struct ref_array array;
> ref_cbdata.filter = &filter;
> ref_cbdata.array = &array;

... but after PATCH 7, filter and array are passed to ref_filter so you
don't have this overhead anyway. Makes sense.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

[Karthik Nayak]

On 06/12/2015 12:43 AM, Matthieu Moy wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > On 06/11/2015 11:11 PM, Matthieu Moy wrote:
> >> Karthik Nayak <karthik.188@gmail.com> writes:
> >>
> >>> +struct ref_filter_cbdata {
> >>> +    struct ref_array array;
> >>> +    struct ref_filter filter;
> >>> +};
> >>
> >> I didn't notice this at first, but why introduce the structure like this
> >> when you are going to turn it into pointers later in PATCH 7:
> > Here it is serving to for-each-ref, so I kept it this way so as to ensure
> > that currently as per this patch
> >
> > struct ref_filter_cbdata ref_cbdata;
> >
> > would be the only declaration needed in for-each-ref.c
> > If I made them pointers here I would need to have
> >
> > struct ref_filter_cbdata ref_cbdata;
> > struct ref_filter filter;
> > struct ref_array array;
> > ref_cbdata.filter = &filter;
> > ref_cbdata.array = &array;
>
> ... but after PATCH 7, filter and array are passed to ref_filter so you
> don't have this overhead anyway. Makes sense.
>
Yes, there we wouldn't have a ref_cbdata in 'for-each-ref'.
But this would be taken care of in 'filter_refs()'.
"Makes sense." Not sure if you're agreeing with me or you want me to re-roll.

-- 
Regards,
Karthik

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

[Matthieu Moy]

Karthik Nayak <karthik.188@gmail.com> writes:

>> ... but after PATCH 7, filter and array are passed to ref_filter so you
>> don't have this overhead anyway. Makes sense.
>>
> Yes, there we wouldn't have a ref_cbdata in 'for-each-ref'.
> But this would be taken care of in 'filter_refs()'.
> "Makes sense." Not sure if you're agreeing with me or you want me to
> re-roll.

I'm agreeing with you.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH v7 06/12] for-each-ref: introduce 'ref_array_clear()'

[Karthik Nayak]

Introduce and implement 'ref_array_clear()' which will free
all allocated memory for 'ref_array'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 913d757..58396b2 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -927,6 +927,26 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
+/*  Free memory allocated for a ref_array_item */
+static void free_array_item(struct ref_array_item *item)
+{
+	free((char *)item->symref);
+	free(item->refname);
+	free(item);
+}
+
+/* Free all memory allocated for ref_array */
+void ref_array_clear(struct ref_array *array)
+{
+	int i;
+
+	for (i = 0; i < array->nr; i++)
+		free_array_item(array->items[i]);
+	free(array->items);
+	array->items = NULL;
+	array->nr = array->alloc = 0;
+}
+
 static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
@@ -1157,5 +1177,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		maxcount = ref_cbdata.array.nr;
 	for (i = 0; i < maxcount; i++)
 		show_ref(ref_cbdata.array.items[i], format, quote_style);
+	ref_array_clear(&ref_cbdata.array);
 	return 0;
 }
-- 
2.4.2

[PATCH v7 07/12] for-each-ref: rename some functions and make them public

[Karthik Nayak]

Rename some of the functions and make them publicly 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.

Functions renamed are:
parse_atom()		-> 	parse_ref_filter_atom()
verify_format()		-> 	verify_ref_format()
get_value()		-> 	get_ref_atom_value()
grab_single_ref()	-> 	ref_filter_handler()
sort_refs()		->	ref_array_sort()
show_ref()		->	show_ref_array_item()
default_sort()		->	ref_default_sorting()
opt_parse_sort()	->	parse_opt_ref_sorting()
cmp_ref_sort()		->	cmp_ref_sorting()

Rename 'struct ref_sort' to 'struct ref_sorting' in this context.

Based-on-patch-by: Jeff King <peff@peff.net>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 68 +++++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 58396b2..4a6eaba 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -25,8 +25,8 @@ struct atom_value {
 	unsigned long ul; /* used for sorting when not FIELD_STR */
 };
 
-struct ref_sort {
-	struct ref_sort *next;
+struct ref_sorting {
+	struct ref_sorting *next;
 	int atom; /* index into 'struct atom_value *' array */
 	unsigned reverse : 1;
 };
@@ -113,7 +113,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;
@@ -190,7 +190,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;
 
@@ -202,7 +202,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))
@@ -409,7 +409,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, ':');
@@ -849,7 +849,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);
@@ -900,8 +900,7 @@ 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
  * later object processing.
  */
-static int grab_single_ref(const char *refname, const struct object_id *oid,
-			   int flag, void *cb_data)
+int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
 	struct ref_filter *filter = &ref_cbdata->filter;
@@ -947,14 +946,14 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
-static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref_array_item *b)
+static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
 	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);
@@ -971,24 +970,24 @@ static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref
 	return (s->reverse) ? -cmp : cmp;
 }
 
-static struct ref_sort *ref_sort;
+static struct ref_sorting *ref_sorting;
 static int compare_refs(const void *a_, const void *b_)
 {
 	struct ref_array_item *a = *((struct ref_array_item **)a_);
 	struct ref_array_item *b = *((struct ref_array_item **)b_);
-	struct ref_sort *s;
+	struct ref_sorting *s;
 
-	for (s = ref_sort; s; s = s->next) {
-		int cmp = cmp_ref_sort(s, a, b);
+	for (s = ref_sorting; s; s = s->next) {
+		int cmp = cmp_ref_sorting(s, a, b);
 		if (cmp)
 			return cmp;
 	}
 	return 0;
 }
 
-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void ref_array_sort(struct ref_sorting *sort, struct ref_array *array)
 {
-	ref_sort = sort;
+	ref_sorting = sort;
 	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
 }
 
@@ -1056,7 +1055,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;
 
@@ -1066,7 +1065,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) {
@@ -1085,21 +1084,22 @@ 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_sorting *ref_default_sorting(void)
 {
 	static const char cstr_name[] = "refname";
 
-	struct ref_sort *sort = xcalloc(1, sizeof(*sort));
+	struct ref_sorting *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 parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
 {
-	struct ref_sort **sort_tail = opt->value;
-	struct ref_sort *s;
+	struct ref_sorting **sort_tail = opt->value;
+	struct ref_sorting *s;
 	int len;
 
 	if (!arg) /* should --no-sort void the list ? */
@@ -1114,7 +1114,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;
 }
 
@@ -1127,7 +1127,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
-	struct ref_sort *sort = NULL, **sort_tail = &sort;
+	struct ref_sorting *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
 	struct ref_filter_cbdata ref_cbdata;
 
@@ -1145,7 +1145,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"), &parse_opt_ref_sorting),
 		OPT_END(),
 	};
 
@@ -1158,25 +1158,25 @@ 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_sorting();
 
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
 	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
 	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);
+	ref_array_sort(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_array_clear(&ref_cbdata.array);
 	return 0;
 }
-- 
2.4.2

[PATCH v7 08/12] for-each-ref: rename variables called sort to sorting

[Karthik Nayak]

Rename all the variables called sort to sorting to match the
function/structure name changes made in the previous patch.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4a6eaba..3cf456a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -985,9 +985,9 @@ static int compare_refs(const void *a_, const void *b_)
 	return 0;
 }
 
-void ref_array_sort(struct ref_sorting *sort, struct ref_array *array)
+void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
 {
-	ref_sorting = sort;
+	ref_sorting = sorting;
 	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
 }
 
@@ -1089,16 +1089,16 @@ struct ref_sorting *ref_default_sorting(void)
 {
 	static const char cstr_name[] = "refname";
 
-	struct ref_sorting *sort = xcalloc(1, sizeof(*sort));
+	struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));
 
-	sort->next = NULL;
-	sort->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
-	return sort;
+	sorting->next = NULL;
+	sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
+	return sorting;
 }
 
 int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
 {
-	struct ref_sorting **sort_tail = opt->value;
+	struct ref_sorting **sorting_tail = opt->value;
 	struct ref_sorting *s;
 	int len;
 
@@ -1106,8 +1106,8 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
 		return -1;
 
 	s = xcalloc(1, sizeof(*s));
-	s->next = *sort_tail;
-	*sort_tail = s;
+	s->next = *sorting_tail;
+	*sorting_tail = s;
 
 	if (*arg == '-') {
 		s->reverse = 1;
@@ -1127,7 +1127,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
-	struct ref_sorting *sort = NULL, **sort_tail = &sort;
+	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	int maxcount = 0, quote_style = 0;
 	struct ref_filter_cbdata ref_cbdata;
 
@@ -1144,7 +1144,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		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"),
+		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
 			    N_("field name to sort on"), &parse_opt_ref_sorting),
 		OPT_END(),
 	};
@@ -1161,8 +1161,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	if (verify_ref_format(format))
 		usage_with_options(for_each_ref_usage, opts);
 
-	if (!sort)
-		sort = ref_default_sorting();
+	if (!sorting)
+		sorting = ref_default_sorting();
 
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
@@ -1171,7 +1171,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	ref_cbdata.filter.name_patterns = argv;
 	for_each_rawref(ref_filter_handler, &ref_cbdata);
 
-	ref_array_sort(sort, &ref_cbdata.array);
+	ref_array_sort(sorting, &ref_cbdata.array);
 
 	if (!maxcount || ref_cbdata.array.nr < maxcount)
 		maxcount = ref_cbdata.array.nr;
-- 
2.4.2

[PATCH v7 09/12] ref-filter: add 'ref-filter.h'

[Karthik Nayak]

This is step one of creating a common library for 'for-each-ref',
'branch -l' and 'tag -l'. This creates a header file with the
functions and data structures that ref-filter will provide.
We move the data structures created in for-each-ref to this header
file.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 41 +------------------------------
 ref-filter.h           | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 40 deletions(-)
 create mode 100644 ref-filter.h

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3cf456a..2fbbc59 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -10,49 +10,10 @@
 #include "parse-options.h"
 #include "remote.h"
 #include "color.h"
-
-/* Quoting styles */
-#define QUOTE_NONE 0
-#define QUOTE_SHELL 1
-#define QUOTE_PERL 2
-#define QUOTE_PYTHON 4
-#define QUOTE_TCL 8
+#include "ref-filter.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
-struct atom_value {
-	const char *s;
-	unsigned long ul; /* used for sorting when not FIELD_STR */
-};
-
-struct ref_sorting {
-	struct ref_sorting *next;
-	int atom; /* index into 'struct atom_value *' array */
-	unsigned reverse : 1;
-};
-
-struct ref_array_item {
-	unsigned char objectname[20];
-	int flag;
-	const char *symref;
-	struct atom_value *value;
-	char *refname;
-};
-
-struct ref_array {
-	int nr, alloc;
-	struct ref_array_item **items;
-};
-
-struct ref_filter {
-	const char **name_patterns;
-};
-
-struct ref_filter_cbdata {
-	struct ref_array array;
-	struct ref_filter filter;
-};
-
 static struct {
 	const char *name;
 	cmp_type cmp_type;
diff --git a/ref-filter.h b/ref-filter.h
new file mode 100644
index 0000000..d9e043f
--- /dev/null
+++ b/ref-filter.h
@@ -0,0 +1,66 @@
+#ifndef REF_FILTER_H
+#define REF_FILTER_H
+
+#include "sha1-array.h"
+#include "refs.h"
+#include "commit.h"
+#include "parse-options.h"
+
+/* Quoting styles */
+#define QUOTE_NONE 0
+#define QUOTE_SHELL 1
+#define QUOTE_PERL 2
+#define QUOTE_PYTHON 4
+#define QUOTE_TCL 8
+
+struct atom_value {
+	const char *s;
+	unsigned long ul; /* used for sorting when not FIELD_STR */
+};
+
+struct ref_sorting {
+	struct ref_sorting *next;
+	int atom; /* index into 'struct atom_value *' array */
+	unsigned reverse : 1;
+};
+
+struct ref_array_item {
+	unsigned char objectname[20];
+	int flag;
+	const char *symref;
+	struct atom_value *value;
+	char *refname;
+};
+
+struct ref_array {
+	int nr, alloc;
+	struct ref_array_item **items;
+};
+
+struct ref_filter {
+	const char **name_patterns;
+};
+
+struct ref_filter_cbdata {
+	struct ref_array array;
+	struct ref_filter filter;
+};
+
+/*  Callback function for for_each_*ref(). This filters the refs based on the filters set */
+int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data);
+/*  Clear all memory allocated to ref_array */
+void ref_array_clear(struct ref_array *array);
+/*  Parse format string and sort specifiers */
+int parse_ref_filter_atom(const char *atom, const char *ep);
+/*  Used to verify if the given format is correct and to parse out the used atoms */
+int verify_ref_format(const char *format);
+/*  Sort the given ref_array as per the ref_sorting provided */
+void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/*  Print the ref using the given format and quote_style */
+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style);
+/*  Callback function for parsing the sort option */
+int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);
+/*  Default sort option based on refname */
+struct ref_sorting *ref_default_sorting(void);
+
+#endif /*  REF_FILTER_H  */
-- 
2.4.2

[PATCH v7 10/12] ref-filter: move code from 'for-each-ref'

[Karthik Nayak]

Move most of the code from 'for-each-ref' to 'ref-filter' to make
it publicly available to other commands, this is to unify the code
of 'tag -l', 'branch -l' and 'for-each-ref' so that they can share
their implementations with each other.

Add 'ref-filter' to the Makefile, this completes the movement of code
from 'for-each-ref' to 'ref-filter'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Makefile               |    1 +
 builtin/for-each-ref.c | 1074 -----------------------------------------------
 ref-filter.c           | 1078 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1079 insertions(+), 1074 deletions(-)
 create mode 100644 ref-filter.c

diff --git a/Makefile b/Makefile
index 54ec511..d715b66 100644
--- a/Makefile
+++ b/Makefile
@@ -762,6 +762,7 @@ LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs.o
+LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
 LIB_OBJS += rerere.o
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 2fbbc59..637fc4a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -2,1083 +2,9 @@
 #include "cache.h"
 #include "refs.h"
 #include "object.h"
-#include "tag.h"
-#include "commit.h"
-#include "tree.h"
-#include "blob.h"
-#include "quote.h"
 #include "parse-options.h"
-#include "remote.h"
-#include "color.h"
 #include "ref-filter.h"
 
-typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
-
-static struct {
-	const char *name;
-	cmp_type cmp_type;
-} valid_atom[] = {
-	{ "refname" },
-	{ "objecttype" },
-	{ "objectsize", FIELD_ULONG },
-	{ "objectname" },
-	{ "tree" },
-	{ "parent" },
-	{ "numparent", FIELD_ULONG },
-	{ "object" },
-	{ "type" },
-	{ "tag" },
-	{ "author" },
-	{ "authorname" },
-	{ "authoremail" },
-	{ "authordate", FIELD_TIME },
-	{ "committer" },
-	{ "committername" },
-	{ "committeremail" },
-	{ "committerdate", FIELD_TIME },
-	{ "tagger" },
-	{ "taggername" },
-	{ "taggeremail" },
-	{ "taggerdate", FIELD_TIME },
-	{ "creator" },
-	{ "creatordate", FIELD_TIME },
-	{ "subject" },
-	{ "body" },
-	{ "contents" },
-	{ "contents:subject" },
-	{ "contents:body" },
-	{ "contents:signature" },
-	{ "upstream" },
-	{ "push" },
-	{ "symref" },
-	{ "flag" },
-	{ "HEAD" },
-	{ "color" },
-};
-
-/*
- * An atom is a valid field atom listed above, possibly prefixed with
- * a "*" to denote deref_tag().
- *
- * We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects.  ref_array_item
- * structure will hold an array of values extracted that can be
- * indexed with the "atom number", which is an index into this
- * array.
- */
-static const char **used_atom;
-static cmp_type *used_atom_type;
-static int used_atom_cnt, need_tagged, need_symref;
-static int need_color_reset_at_eol;
-
-/*
- * Used to parse format string and sort specifiers
- */
-int parse_ref_filter_atom(const char *atom, const char *ep)
-{
-	const char *sp;
-	int i, at;
-
-	sp = atom;
-	if (*sp == '*' && sp < ep)
-		sp++; /* deref */
-	if (ep <= sp)
-		die("malformed field name: %.*s", (int)(ep-atom), atom);
-
-	/* Do we have the atom already used elsewhere? */
-	for (i = 0; i < used_atom_cnt; i++) {
-		int len = strlen(used_atom[i]);
-		if (len == ep - atom && !memcmp(used_atom[i], atom, len))
-			return i;
-	}
-
-	/* Is the atom a valid one? */
-	for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
-		int len = strlen(valid_atom[i].name);
-		/*
-		 * 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
-		 * table.
-		 */
-		const char *formatp = strchr(sp, ':');
-		if (!formatp || ep < formatp)
-			formatp = ep;
-		if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))
-			break;
-	}
-
-	if (ARRAY_SIZE(valid_atom) <= 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);
-	REALLOC_ARRAY(used_atom_type, used_atom_cnt);
-	used_atom[at] = xmemdupz(atom, ep - atom);
-	used_atom_type[at] = valid_atom[i].cmp_type;
-	if (*atom == '*')
-		need_tagged = 1;
-	if (!strcmp(used_atom[at], "symref"))
-		need_symref = 1;
-	return at;
-}
-
-/*
- * In a format string, find the next occurrence of %(atom).
- */
-static const char *find_next(const char *cp)
-{
-	while (*cp) {
-		if (*cp == '%') {
-			/*
-			 * %( is the start of an atom;
-			 * %% is a quoted per-cent.
-			 */
-			if (cp[1] == '(')
-				return cp;
-			else if (cp[1] == '%')
-				cp++; /* skip over two % */
-			/* otherwise this is a singleton, literal % */
-		}
-		cp++;
-	}
-	return NULL;
-}
-
-/*
- * Make sure the format string is well formed, and parse out
- * the used atoms.
- */
-int verify_ref_format(const char *format)
-{
-	const char *cp, *sp;
-
-	need_color_reset_at_eol = 0;
-	for (cp = format; *cp && (sp = find_next(cp)); ) {
-		const char *color, *ep = strchr(sp, ')');
-		int at;
-
-		if (!ep)
-			return error("malformed format string %s", sp);
-		/* sp points at "%(" and ep points at the closing ")" */
-		at = parse_ref_filter_atom(sp + 2, ep);
-		cp = ep + 1;
-
-		if (skip_prefix(used_atom[at], "color:", &color))
-			need_color_reset_at_eol = !!strcmp(color, "reset");
-	}
-	return 0;
-}
-
-/*
- * Given an object name, read the object data and size, and return a
- * "struct object".  If the object data we are returning is also borrowed
- * by the "struct object" representation, set *eaten as well---it is a
- * signal from parse_object_buffer to us not to free the buffer.
- */
-static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned long *sz, int *eaten)
-{
-	enum object_type type;
-	void *buf = read_sha1_file(sha1, &type, sz);
-
-	if (buf)
-		*obj = parse_object_buffer(sha1, type, *sz, buf, eaten);
-	else
-		*obj = NULL;
-	return buf;
-}
-
-static int grab_objectname(const char *name, const unsigned char *sha1,
-			    struct atom_value *v)
-{
-	if (!strcmp(name, "objectname")) {
-		char *s = xmalloc(41);
-		strcpy(s, sha1_to_hex(sha1));
-		v->s = s;
-		return 1;
-	}
-	if (!strcmp(name, "objectname:short")) {
-		v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
-		return 1;
-	}
-	return 0;
-}
-
-/* See grab_values */
-static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
-{
-	int i;
-
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i];
-		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
-			continue;
-		if (deref)
-			name++;
-		if (!strcmp(name, "objecttype"))
-			v->s = typename(obj->type);
-		else if (!strcmp(name, "objectsize")) {
-			char *s = xmalloc(40);
-			sprintf(s, "%lu", sz);
-			v->ul = sz;
-			v->s = s;
-		}
-		else if (deref)
-			grab_objectname(name, obj->sha1, v);
-	}
-}
-
-/* See grab_values */
-static void grab_tag_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
-{
-	int i;
-	struct tag *tag = (struct tag *) obj;
-
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i];
-		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
-			continue;
-		if (deref)
-			name++;
-		if (!strcmp(name, "tag"))
-			v->s = tag->tag;
-		else if (!strcmp(name, "type") && tag->tagged)
-			v->s = typename(tag->tagged->type);
-		else if (!strcmp(name, "object") && tag->tagged) {
-			char *s = xmalloc(41);
-			strcpy(s, sha1_to_hex(tag->tagged->sha1));
-			v->s = s;
-		}
-	}
-}
-
-/* See grab_values */
-static void grab_commit_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
-{
-	int i;
-	struct commit *commit = (struct commit *) obj;
-
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i];
-		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
-			continue;
-		if (deref)
-			name++;
-		if (!strcmp(name, "tree")) {
-			char *s = xmalloc(41);
-			strcpy(s, sha1_to_hex(commit->tree->object.sha1));
-			v->s = s;
-		}
-		if (!strcmp(name, "numparent")) {
-			char *s = xmalloc(40);
-			v->ul = commit_list_count(commit->parents);
-			sprintf(s, "%lu", v->ul);
-			v->s = s;
-		}
-		else if (!strcmp(name, "parent")) {
-			int num = commit_list_count(commit->parents);
-			int i;
-			struct commit_list *parents;
-			char *s = xmalloc(41 * num + 1);
-			v->s = s;
-			for (i = 0, parents = commit->parents;
-			     parents;
-			     parents = parents->next, i = i + 41) {
-				struct commit *parent = parents->item;
-				strcpy(s+i, sha1_to_hex(parent->object.sha1));
-				if (parents->next)
-					s[i+40] = ' ';
-			}
-			if (!i)
-				*s = '\0';
-		}
-	}
-}
-
-static const char *find_wholine(const char *who, int wholen, const char *buf, unsigned long sz)
-{
-	const char *eol;
-	while (*buf) {
-		if (!strncmp(buf, who, wholen) &&
-		    buf[wholen] == ' ')
-			return buf + wholen + 1;
-		eol = strchr(buf, '\n');
-		if (!eol)
-			return "";
-		eol++;
-		if (*eol == '\n')
-			return ""; /* end of header */
-		buf = eol;
-	}
-	return "";
-}
-
-static const char *copy_line(const char *buf)
-{
-	const char *eol = strchrnul(buf, '\n');
-	return xmemdupz(buf, eol - buf);
-}
-
-static const char *copy_name(const char *buf)
-{
-	const char *cp;
-	for (cp = buf; *cp && *cp != '\n'; cp++) {
-		if (!strncmp(cp, " <", 2))
-			return xmemdupz(buf, cp - buf);
-	}
-	return "";
-}
-
-static const char *copy_email(const char *buf)
-{
-	const char *email = strchr(buf, '<');
-	const char *eoemail;
-	if (!email)
-		return "";
-	eoemail = strchr(email, '>');
-	if (!eoemail)
-		return "";
-	return xmemdupz(email, eoemail + 1 - email);
-}
-
-static char *copy_subject(const char *buf, unsigned long len)
-{
-	char *r = xmemdupz(buf, len);
-	int i;
-
-	for (i = 0; i < len; i++)
-		if (r[i] == '\n')
-			r[i] = ' ';
-
-	return r;
-}
-
-static void grab_date(const char *buf, struct atom_value *v, const char *atomname)
-{
-	const char *eoemail = strstr(buf, "> ");
-	char *zone;
-	unsigned long timestamp;
-	long tz;
-	enum date_mode date_mode = DATE_NORMAL;
-	const char *formatp;
-
-	/*
-	 * We got here because atomname ends in "date" or "date<something>";
-	 * it's not possible that <something> is not ":<format>" because
-	 * 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, ':');
-	if (formatp != NULL) {
-		formatp++;
-		date_mode = parse_date_format(formatp);
-	}
-
-	if (!eoemail)
-		goto bad;
-	timestamp = strtoul(eoemail + 2, &zone, 10);
-	if (timestamp == ULONG_MAX)
-		goto bad;
-	tz = strtol(zone, NULL, 10);
-	if ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)
-		goto bad;
-	v->s = xstrdup(show_date(timestamp, tz, date_mode));
-	v->ul = timestamp;
-	return;
- bad:
-	v->s = "";
-	v->ul = 0;
-}
-
-/* See grab_values */
-static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
-{
-	int i;
-	int wholen = strlen(who);
-	const char *wholine = NULL;
-
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i];
-		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
-			continue;
-		if (deref)
-			name++;
-		if (strncmp(who, name, wholen))
-			continue;
-		if (name[wholen] != 0 &&
-		    strcmp(name + wholen, "name") &&
-		    strcmp(name + wholen, "email") &&
-		    !starts_with(name + wholen, "date"))
-			continue;
-		if (!wholine)
-			wholine = find_wholine(who, wholen, buf, sz);
-		if (!wholine)
-			return; /* no point looking for it */
-		if (name[wholen] == 0)
-			v->s = copy_line(wholine);
-		else if (!strcmp(name + wholen, "name"))
-			v->s = copy_name(wholine);
-		else if (!strcmp(name + wholen, "email"))
-			v->s = copy_email(wholine);
-		else if (starts_with(name + wholen, "date"))
-			grab_date(wholine, v, name);
-	}
-
-	/*
-	 * For a tag or a commit object, if "creator" or "creatordate" is
-	 * requested, do something special.
-	 */
-	if (strcmp(who, "tagger") && strcmp(who, "committer"))
-		return; /* "author" for commit object is not wanted */
-	if (!wholine)
-		wholine = find_wholine(who, wholen, buf, sz);
-	if (!wholine)
-		return;
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i];
-		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
-			continue;
-		if (deref)
-			name++;
-
-		if (starts_with(name, "creatordate"))
-			grab_date(wholine, v, name);
-		else if (!strcmp(name, "creator"))
-			v->s = copy_line(wholine);
-	}
-}
-
-static void find_subpos(const char *buf, unsigned long sz,
-			const char **sub, unsigned long *sublen,
-			const char **body, unsigned long *bodylen,
-			unsigned long *nonsiglen,
-			const char **sig, unsigned long *siglen)
-{
-	const char *eol;
-	/* skip past header until we hit empty line */
-	while (*buf && *buf != '\n') {
-		eol = strchrnul(buf, '\n');
-		if (*eol)
-			eol++;
-		buf = eol;
-	}
-	/* skip any empty lines */
-	while (*buf == '\n')
-		buf++;
-
-	/* parse signature first; we might not even have a subject line */
-	*sig = buf + parse_signature(buf, strlen(buf));
-	*siglen = strlen(*sig);
-
-	/* subject is first non-empty line */
-	*sub = buf;
-	/* subject goes to first empty line */
-	while (buf < *sig && *buf && *buf != '\n') {
-		eol = strchrnul(buf, '\n');
-		if (*eol)
-			eol++;
-		buf = eol;
-	}
-	*sublen = buf - *sub;
-	/* drop trailing newline, if present */
-	if (*sublen && (*sub)[*sublen - 1] == '\n')
-		*sublen -= 1;
-
-	/* skip any empty lines */
-	while (*buf == '\n')
-		buf++;
-	*body = buf;
-	*bodylen = strlen(buf);
-	*nonsiglen = *sig - buf;
-}
-
-/* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
-{
-	int i;
-	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
-	unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
-
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i];
-		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
-			continue;
-		if (deref)
-			name++;
-		if (strcmp(name, "subject") &&
-		    strcmp(name, "body") &&
-		    strcmp(name, "contents") &&
-		    strcmp(name, "contents:subject") &&
-		    strcmp(name, "contents:body") &&
-		    strcmp(name, "contents:signature"))
-			continue;
-		if (!subpos)
-			find_subpos(buf, sz,
-				    &subpos, &sublen,
-				    &bodypos, &bodylen, &nonsiglen,
-				    &sigpos, &siglen);
-
-		if (!strcmp(name, "subject"))
-			v->s = copy_subject(subpos, sublen);
-		else if (!strcmp(name, "contents:subject"))
-			v->s = copy_subject(subpos, sublen);
-		else if (!strcmp(name, "body"))
-			v->s = xmemdupz(bodypos, bodylen);
-		else if (!strcmp(name, "contents:body"))
-			v->s = xmemdupz(bodypos, nonsiglen);
-		else if (!strcmp(name, "contents:signature"))
-			v->s = xmemdupz(sigpos, siglen);
-		else if (!strcmp(name, "contents"))
-			v->s = xstrdup(subpos);
-	}
-}
-
-/*
- * We want to have empty print-string for field requests
- * that do not apply (e.g. "authordate" for a tag object)
- */
-static void fill_missing_values(struct atom_value *val)
-{
-	int i;
-	for (i = 0; i < used_atom_cnt; i++) {
-		struct atom_value *v = &val[i];
-		if (v->s == NULL)
-			v->s = "";
-	}
-}
-
-/*
- * 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).
- * 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.
- */
-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
-{
-	grab_common_values(val, deref, obj, buf, sz);
-	switch (obj->type) {
-	case OBJ_TAG:
-		grab_tag_values(val, deref, obj, buf, sz);
-		grab_sub_body_contents(val, deref, obj, buf, sz);
-		grab_person("tagger", val, deref, obj, buf, sz);
-		break;
-	case OBJ_COMMIT:
-		grab_commit_values(val, deref, obj, buf, sz);
-		grab_sub_body_contents(val, deref, obj, buf, sz);
-		grab_person("author", val, deref, obj, buf, sz);
-		grab_person("committer", val, deref, obj, buf, sz);
-		break;
-	case OBJ_TREE:
-		/* grab_tree_values(val, deref, obj, buf, sz); */
-		break;
-	case OBJ_BLOB:
-		/* grab_blob_values(val, deref, obj, buf, sz); */
-		break;
-	default:
-		die("Eh?  Object of type %d?", obj->type);
-	}
-}
-
-static inline char *copy_advance(char *dst, const char *src)
-{
-	while (*src)
-		*dst++ = *src++;
-	return dst;
-}
-
-/*
- * 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 unsigned char *tagged;
-
-	ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
-
-	if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
-		unsigned char unused1[20];
-		ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
-					     unused1, NULL);
-		if (!ref->symref)
-			ref->symref = "";
-	}
-
-	/* Fill in specials first */
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i];
-		struct atom_value *v = &ref->value[i];
-		int deref = 0;
-		const char *refname;
-		const char *formatp;
-		struct branch *branch = NULL;
-
-		if (*name == '*') {
-			deref = 1;
-			name++;
-		}
-
-		if (starts_with(name, "refname"))
-			refname = ref->refname;
-		else if (starts_with(name, "symref"))
-			refname = ref->symref ? ref->symref : "";
-		else if (starts_with(name, "upstream")) {
-			const char *branch_name;
-			/* only local branches may have an upstream */
-			if (!skip_prefix(ref->refname, "refs/heads/",
-					 &branch_name))
-				continue;
-			branch = branch_get(branch_name);
-
-			refname = branch_get_upstream(branch, NULL);
-			if (!refname)
-				continue;
-		} else if (starts_with(name, "push")) {
-			const char *branch_name;
-			if (!skip_prefix(ref->refname, "refs/heads/",
-					 &branch_name))
-				continue;
-			branch = branch_get(branch_name);
-
-			refname = branch_get_push(branch, NULL);
-			if (!refname)
-				continue;
-		} else if (starts_with(name, "color:")) {
-			char color[COLOR_MAXLEN] = "";
-
-			if (color_parse(name + 6, color) < 0)
-				die(_("unable to parse format"));
-			v->s = xstrdup(color);
-			continue;
-		} else if (!strcmp(name, "flag")) {
-			char buf[256], *cp = buf;
-			if (ref->flag & REF_ISSYMREF)
-				cp = copy_advance(cp, ",symref");
-			if (ref->flag & REF_ISPACKED)
-				cp = copy_advance(cp, ",packed");
-			if (cp == buf)
-				v->s = "";
-			else {
-				*cp = '\0';
-				v->s = xstrdup(buf + 1);
-			}
-			continue;
-		} else if (!deref && grab_objectname(name, ref->objectname, v)) {
-			continue;
-		} else if (!strcmp(name, "HEAD")) {
-			const char *head;
-			unsigned char sha1[20];
-
-			head = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
-						  sha1, NULL);
-			if (!strcmp(ref->refname, head))
-				v->s = "*";
-			else
-				v->s = " ";
-			continue;
-		} else
-			continue;
-
-		formatp = strchr(name, ':');
-		if (formatp) {
-			int num_ours, num_theirs;
-
-			formatp++;
-			if (!strcmp(formatp, "short"))
-				refname = shorten_unambiguous_ref(refname,
-						      warn_ambiguous_refs);
-			else if (!strcmp(formatp, "track") &&
-				 (starts_with(name, "upstream") ||
-				  starts_with(name, "push"))) {
-				char buf[40];
-
-				if (stat_tracking_info(branch, &num_ours,
-						       &num_theirs, NULL))
-					continue;
-
-				if (!num_ours && !num_theirs)
-					v->s = "";
-				else if (!num_ours) {
-					sprintf(buf, "[behind %d]", num_theirs);
-					v->s = xstrdup(buf);
-				} else if (!num_theirs) {
-					sprintf(buf, "[ahead %d]", num_ours);
-					v->s = xstrdup(buf);
-				} else {
-					sprintf(buf, "[ahead %d, behind %d]",
-						num_ours, num_theirs);
-					v->s = xstrdup(buf);
-				}
-				continue;
-			} else if (!strcmp(formatp, "trackshort") &&
-				   (starts_with(name, "upstream") ||
-				    starts_with(name, "push"))) {
-				assert(branch);
-
-				if (stat_tracking_info(branch, &num_ours,
-							&num_theirs, NULL))
-					continue;
-
-				if (!num_ours && !num_theirs)
-					v->s = "=";
-				else if (!num_ours)
-					v->s = "<";
-				else if (!num_theirs)
-					v->s = ">";
-				else
-					v->s = "<>";
-				continue;
-			} else
-				die("unknown %.*s format %s",
-				    (int)(formatp - name), name, formatp);
-		}
-
-		if (!deref)
-			v->s = refname;
-		else {
-			int len = strlen(refname);
-			char *s = xmalloc(len + 4);
-			sprintf(s, "%s^{}", refname);
-			v->s = s;
-		}
-	}
-
-	for (i = 0; i < used_atom_cnt; i++) {
-		struct atom_value *v = &ref->value[i];
-		if (v->s == NULL)
-			goto need_obj;
-	}
-	return;
-
- need_obj:
-	buf = get_obj(ref->objectname, &obj, &size, &eaten);
-	if (!buf)
-		die("missing object %s for %s",
-		    sha1_to_hex(ref->objectname), ref->refname);
-	if (!obj)
-		die("parse_object_buffer failed on %s for %s",
-		    sha1_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->sha1;
-
-	/*
-	 * 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",
-		    sha1_to_hex(tagged), ref->refname);
-	if (!obj)
-		die("parse_object_buffer failed on %s for %s",
-		    sha1_to_hex(tagged), ref->refname);
-	grab_values(ref->value, 1, obj, buf, size);
-	if (!eaten)
-		free(buf);
-}
-
-/*
- * Given a ref, return the value for the atom.  This lazily gets value
- * out of the object by calling populate value.
- */
-static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
-{
-	if (!ref->value) {
-		populate_value(ref);
-		fill_missing_values(ref->value);
-	}
-	*v = &ref->value[atom];
-}
-
-/*
- * Return 1 if the refname matches with one of the patterns,
- * otherwise 0.  The patterns can be literal prefix (e.g. a
- * refname "refs/heads/master" matches a pattern "refs/heads/")
- * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
- */
-static int match_name_as_path(const char **pattern, const char *refname)
-{
-	int namelen = strlen(refname);
-	for (; *pattern; pattern++) {
-		const char *p = *pattern;
-		int plen = strlen(p);
-
-		if ((plen <= namelen) &&
-		    !strncmp(refname, p, plen) &&
-		    (refname[plen] == '\0' ||
-		     refname[plen] == '/' ||
-		     p[plen-1] == '/'))
-			return 1;
-		if (!wildmatch(p, refname, WM_PATHNAME, NULL))
-			return 1;
-	}
-	return 0;
-}
-
-/* Allocate space for a new ref_array_item and copy the objectname and flag to it */
-static struct ref_array_item *new_ref_array_item(const char *refname,
-						 const unsigned char *objectname,
-						 int flag)
-{
-	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
-	ref->refname = xstrdup(refname);
-	hashcpy(ref->objectname, objectname);
-	ref->flag = flag;
-
-	return ref;
-}
-
-/*
- * A call-back given to for_each_ref().  Filter refs and keep them for
- * later object processing.
- */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
-{
-	struct ref_filter_cbdata *ref_cbdata = cb_data;
-	struct ref_filter *filter = &ref_cbdata->filter;
-	struct ref_array_item *ref;
-
-	if (flag & REF_BAD_NAME) {
-		  warning("ignoring ref with broken name %s", refname);
-		  return 0;
-	}
-
-	if (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))
-		return 0;
-
-	/*
-	 * We do not open the object yet; sort may only need refname
-	 * to do its job and the resulting list may yet to be pruned
-	 * by maxcount logic.
-	 */
-	ref = new_ref_array_item(refname, oid->hash, flag);
-
-	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
-	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
-	return 0;
-}
-
-/*  Free memory allocated for a ref_array_item */
-static void free_array_item(struct ref_array_item *item)
-{
-	free((char *)item->symref);
-	free(item->refname);
-	free(item);
-}
-
-/* Free all memory allocated for ref_array */
-void ref_array_clear(struct ref_array *array)
-{
-	int i;
-
-	for (i = 0; i < array->nr; i++)
-		free_array_item(array->items[i]);
-	free(array->items);
-	array->items = NULL;
-	array->nr = array->alloc = 0;
-}
-
-static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
-{
-	struct atom_value *va, *vb;
-	int cmp;
-	cmp_type cmp_type = used_atom_type[s->atom];
-
-	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);
-		break;
-	default:
-		if (va->ul < vb->ul)
-			cmp = -1;
-		else if (va->ul == vb->ul)
-			cmp = 0;
-		else
-			cmp = 1;
-		break;
-	}
-	return (s->reverse) ? -cmp : cmp;
-}
-
-static struct ref_sorting *ref_sorting;
-static int compare_refs(const void *a_, const void *b_)
-{
-	struct ref_array_item *a = *((struct ref_array_item **)a_);
-	struct ref_array_item *b = *((struct ref_array_item **)b_);
-	struct ref_sorting *s;
-
-	for (s = ref_sorting; s; s = s->next) {
-		int cmp = cmp_ref_sorting(s, a, b);
-		if (cmp)
-			return cmp;
-	}
-	return 0;
-}
-
-void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
-{
-	ref_sorting = sorting;
-	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
-}
-
-static void print_value(struct atom_value *v, int quote_style)
-{
-	struct strbuf sb = STRBUF_INIT;
-	switch (quote_style) {
-	case QUOTE_NONE:
-		fputs(v->s, stdout);
-		break;
-	case QUOTE_SHELL:
-		sq_quote_buf(&sb, v->s);
-		break;
-	case QUOTE_PERL:
-		perl_quote_buf(&sb, v->s);
-		break;
-	case QUOTE_PYTHON:
-		python_quote_buf(&sb, v->s);
-		break;
-	case QUOTE_TCL:
-		tcl_quote_buf(&sb, v->s);
-		break;
-	}
-	if (quote_style != QUOTE_NONE) {
-		fputs(sb.buf, stdout);
-		strbuf_release(&sb);
-	}
-}
-
-static int hex1(char ch)
-{
-	if ('0' <= ch && ch <= '9')
-		return ch - '0';
-	else if ('a' <= ch && ch <= 'f')
-		return ch - 'a' + 10;
-	else if ('A' <= ch && ch <= 'F')
-		return ch - 'A' + 10;
-	return -1;
-}
-static int hex2(const char *cp)
-{
-	if (cp[0] && cp[1])
-		return (hex1(cp[0]) << 4) | hex1(cp[1]);
-	else
-		return -1;
-}
-
-static void emit(const char *cp, const char *ep)
-{
-	while (*cp && (!ep || cp < ep)) {
-		if (*cp == '%') {
-			if (cp[1] == '%')
-				cp++;
-			else {
-				int ch = hex2(cp + 1);
-				if (0 <= ch) {
-					putchar(ch);
-					cp += 3;
-					continue;
-				}
-			}
-		}
-		putchar(*cp);
-		cp++;
-	}
-}
-
-void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
-{
-	const char *cp, *sp, *ep;
-
-	for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
-		struct atom_value *atomv;
-
-		ep = strchr(sp, ')');
-		if (cp < sp)
-			emit(cp, sp);
-		get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
-		print_value(atomv, quote_style);
-	}
-	if (*cp) {
-		sp = cp + strlen(cp);
-		emit(cp, sp);
-	}
-	if (need_color_reset_at_eol) {
-		struct atom_value resetv;
-		char color[COLOR_MAXLEN] = "";
-
-		if (color_parse("reset", color) < 0)
-			die("BUG: couldn't parse 'reset' as a color");
-		resetv.s = color;
-		print_value(&resetv, quote_style);
-	}
-	putchar('\n');
-}
-
-/*  If no sorting option is given, use refname to sort as default */
-struct ref_sorting *ref_default_sorting(void)
-{
-	static const char cstr_name[] = "refname";
-
-	struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));
-
-	sorting->next = NULL;
-	sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
-	return sorting;
-}
-
-int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
-{
-	struct ref_sorting **sorting_tail = opt->value;
-	struct ref_sorting *s;
-	int len;
-
-	if (!arg) /* should --no-sort void the list ? */
-		return -1;
-
-	s = xcalloc(1, sizeof(*s));
-	s->next = *sorting_tail;
-	*sorting_tail = s;
-
-	if (*arg == '-') {
-		s->reverse = 1;
-		arg++;
-	}
-	len = strlen(arg);
-	s->atom = parse_ref_filter_atom(arg, arg+len);
-	return 0;
-}
-
 static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
 	NULL
diff --git a/ref-filter.c b/ref-filter.c
new file mode 100644
index 0000000..74fe295
--- /dev/null
+++ b/ref-filter.c
@@ -0,0 +1,1078 @@
+#include "builtin.h"
+#include "cache.h"
+#include "parse-options.h"
+#include "refs.h"
+#include "wildmatch.h"
+#include "commit.h"
+#include "remote.h"
+#include "color.h"
+#include "tag.h"
+#include "quote.h"
+#include "ref-filter.h"
+
+typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
+
+static struct {
+	const char *name;
+	cmp_type cmp_type;
+} valid_atom[] = {
+	{ "refname" },
+	{ "objecttype" },
+	{ "objectsize", FIELD_ULONG },
+	{ "objectname" },
+	{ "tree" },
+	{ "parent" },
+	{ "numparent", FIELD_ULONG },
+	{ "object" },
+	{ "type" },
+	{ "tag" },
+	{ "author" },
+	{ "authorname" },
+	{ "authoremail" },
+	{ "authordate", FIELD_TIME },
+	{ "committer" },
+	{ "committername" },
+	{ "committeremail" },
+	{ "committerdate", FIELD_TIME },
+	{ "tagger" },
+	{ "taggername" },
+	{ "taggeremail" },
+	{ "taggerdate", FIELD_TIME },
+	{ "creator" },
+	{ "creatordate", FIELD_TIME },
+	{ "subject" },
+	{ "body" },
+	{ "contents" },
+	{ "contents:subject" },
+	{ "contents:body" },
+	{ "contents:signature" },
+	{ "upstream" },
+	{ "push" },
+	{ "symref" },
+	{ "flag" },
+	{ "HEAD" },
+	{ "color" },
+};
+
+/*
+ * An atom is a valid field atom listed above, possibly prefixed with
+ * a "*" to denote deref_tag().
+ *
+ * We parse given format string and sort specifiers, and make a list
+ * of properties that we need to extract out of objects.  ref_array_item
+ * structure will hold an array of values extracted that can be
+ * indexed with the "atom number", which is an index into this
+ * array.
+ */
+static const char **used_atom;
+static cmp_type *used_atom_type;
+static int used_atom_cnt, need_tagged, need_symref;
+static int need_color_reset_at_eol;
+
+/*
+ * Used to parse format string and sort specifiers
+ */
+int parse_ref_filter_atom(const char *atom, const char *ep)
+{
+	const char *sp;
+	int i, at;
+
+	sp = atom;
+	if (*sp == '*' && sp < ep)
+		sp++; /* deref */
+	if (ep <= sp)
+		die("malformed field name: %.*s", (int)(ep-atom), atom);
+
+	/* Do we have the atom already used elsewhere? */
+	for (i = 0; i < used_atom_cnt; i++) {
+		int len = strlen(used_atom[i]);
+		if (len == ep - atom && !memcmp(used_atom[i], atom, len))
+			return i;
+	}
+
+	/* Is the atom a valid one? */
+	for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
+		int len = strlen(valid_atom[i].name);
+		/*
+		 * 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
+		 * table.
+		 */
+		const char *formatp = strchr(sp, ':');
+		if (!formatp || ep < formatp)
+			formatp = ep;
+		if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))
+			break;
+	}
+
+	if (ARRAY_SIZE(valid_atom) <= 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);
+	REALLOC_ARRAY(used_atom_type, used_atom_cnt);
+	used_atom[at] = xmemdupz(atom, ep - atom);
+	used_atom_type[at] = valid_atom[i].cmp_type;
+	if (*atom == '*')
+		need_tagged = 1;
+	if (!strcmp(used_atom[at], "symref"))
+		need_symref = 1;
+	return at;
+}
+
+/*
+ * In a format string, find the next occurrence of %(atom).
+ */
+static const char *find_next(const char *cp)
+{
+	while (*cp) {
+		if (*cp == '%') {
+			/*
+			 * %( is the start of an atom;
+			 * %% is a quoted per-cent.
+			 */
+			if (cp[1] == '(')
+				return cp;
+			else if (cp[1] == '%')
+				cp++; /* skip over two % */
+			/* otherwise this is a singleton, literal % */
+		}
+		cp++;
+	}
+	return NULL;
+}
+
+/*
+ * Make sure the format string is well formed, and parse out
+ * the used atoms.
+ */
+int verify_ref_format(const char *format)
+{
+	const char *cp, *sp;
+
+	need_color_reset_at_eol = 0;
+	for (cp = format; *cp && (sp = find_next(cp)); ) {
+		const char *color, *ep = strchr(sp, ')');
+		int at;
+
+		if (!ep)
+			return error("malformed format string %s", sp);
+		/* sp points at "%(" and ep points at the closing ")" */
+		at = parse_ref_filter_atom(sp + 2, ep);
+		cp = ep + 1;
+
+		if (skip_prefix(used_atom[at], "color:", &color))
+			need_color_reset_at_eol = !!strcmp(color, "reset");
+	}
+	return 0;
+}
+
+/*
+ * Given an object name, read the object data and size, and return a
+ * "struct object".  If the object data we are returning is also borrowed
+ * by the "struct object" representation, set *eaten as well---it is a
+ * signal from parse_object_buffer to us not to free the buffer.
+ */
+static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned long *sz, int *eaten)
+{
+	enum object_type type;
+	void *buf = read_sha1_file(sha1, &type, sz);
+
+	if (buf)
+		*obj = parse_object_buffer(sha1, type, *sz, buf, eaten);
+	else
+		*obj = NULL;
+	return buf;
+}
+
+static int grab_objectname(const char *name, const unsigned char *sha1,
+			    struct atom_value *v)
+{
+	if (!strcmp(name, "objectname")) {
+		char *s = xmalloc(41);
+		strcpy(s, sha1_to_hex(sha1));
+		v->s = s;
+		return 1;
+	}
+	if (!strcmp(name, "objectname:short")) {
+		v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
+		return 1;
+	}
+	return 0;
+}
+
+/* See grab_values */
+static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+{
+	int i;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+		if (!strcmp(name, "objecttype"))
+			v->s = typename(obj->type);
+		else if (!strcmp(name, "objectsize")) {
+			char *s = xmalloc(40);
+			sprintf(s, "%lu", sz);
+			v->ul = sz;
+			v->s = s;
+		}
+		else if (deref)
+			grab_objectname(name, obj->sha1, v);
+	}
+}
+
+/* See grab_values */
+static void grab_tag_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+{
+	int i;
+	struct tag *tag = (struct tag *) obj;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+		if (!strcmp(name, "tag"))
+			v->s = tag->tag;
+		else if (!strcmp(name, "type") && tag->tagged)
+			v->s = typename(tag->tagged->type);
+		else if (!strcmp(name, "object") && tag->tagged) {
+			char *s = xmalloc(41);
+			strcpy(s, sha1_to_hex(tag->tagged->sha1));
+			v->s = s;
+		}
+	}
+}
+
+/* See grab_values */
+static void grab_commit_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+{
+	int i;
+	struct commit *commit = (struct commit *) obj;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+		if (!strcmp(name, "tree")) {
+			char *s = xmalloc(41);
+			strcpy(s, sha1_to_hex(commit->tree->object.sha1));
+			v->s = s;
+		}
+		if (!strcmp(name, "numparent")) {
+			char *s = xmalloc(40);
+			v->ul = commit_list_count(commit->parents);
+			sprintf(s, "%lu", v->ul);
+			v->s = s;
+		}
+		else if (!strcmp(name, "parent")) {
+			int num = commit_list_count(commit->parents);
+			int i;
+			struct commit_list *parents;
+			char *s = xmalloc(41 * num + 1);
+			v->s = s;
+			for (i = 0, parents = commit->parents;
+			     parents;
+			     parents = parents->next, i = i + 41) {
+				struct commit *parent = parents->item;
+				strcpy(s+i, sha1_to_hex(parent->object.sha1));
+				if (parents->next)
+					s[i+40] = ' ';
+			}
+			if (!i)
+				*s = '\0';
+		}
+	}
+}
+
+static const char *find_wholine(const char *who, int wholen, const char *buf, unsigned long sz)
+{
+	const char *eol;
+	while (*buf) {
+		if (!strncmp(buf, who, wholen) &&
+		    buf[wholen] == ' ')
+			return buf + wholen + 1;
+		eol = strchr(buf, '\n');
+		if (!eol)
+			return "";
+		eol++;
+		if (*eol == '\n')
+			return ""; /* end of header */
+		buf = eol;
+	}
+	return "";
+}
+
+static const char *copy_line(const char *buf)
+{
+	const char *eol = strchrnul(buf, '\n');
+	return xmemdupz(buf, eol - buf);
+}
+
+static const char *copy_name(const char *buf)
+{
+	const char *cp;
+	for (cp = buf; *cp && *cp != '\n'; cp++) {
+		if (!strncmp(cp, " <", 2))
+			return xmemdupz(buf, cp - buf);
+	}
+	return "";
+}
+
+static const char *copy_email(const char *buf)
+{
+	const char *email = strchr(buf, '<');
+	const char *eoemail;
+	if (!email)
+		return "";
+	eoemail = strchr(email, '>');
+	if (!eoemail)
+		return "";
+	return xmemdupz(email, eoemail + 1 - email);
+}
+
+static char *copy_subject(const char *buf, unsigned long len)
+{
+	char *r = xmemdupz(buf, len);
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (r[i] == '\n')
+			r[i] = ' ';
+
+	return r;
+}
+
+static void grab_date(const char *buf, struct atom_value *v, const char *atomname)
+{
+	const char *eoemail = strstr(buf, "> ");
+	char *zone;
+	unsigned long timestamp;
+	long tz;
+	enum date_mode date_mode = DATE_NORMAL;
+	const char *formatp;
+
+	/*
+	 * We got here because atomname ends in "date" or "date<something>";
+	 * it's not possible that <something> is not ":<format>" because
+	 * 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, ':');
+	if (formatp != NULL) {
+		formatp++;
+		date_mode = parse_date_format(formatp);
+	}
+
+	if (!eoemail)
+		goto bad;
+	timestamp = strtoul(eoemail + 2, &zone, 10);
+	if (timestamp == ULONG_MAX)
+		goto bad;
+	tz = strtol(zone, NULL, 10);
+	if ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)
+		goto bad;
+	v->s = xstrdup(show_date(timestamp, tz, date_mode));
+	v->ul = timestamp;
+	return;
+ bad:
+	v->s = "";
+	v->ul = 0;
+}
+
+/* See grab_values */
+static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+{
+	int i;
+	int wholen = strlen(who);
+	const char *wholine = NULL;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+		if (strncmp(who, name, wholen))
+			continue;
+		if (name[wholen] != 0 &&
+		    strcmp(name + wholen, "name") &&
+		    strcmp(name + wholen, "email") &&
+		    !starts_with(name + wholen, "date"))
+			continue;
+		if (!wholine)
+			wholine = find_wholine(who, wholen, buf, sz);
+		if (!wholine)
+			return; /* no point looking for it */
+		if (name[wholen] == 0)
+			v->s = copy_line(wholine);
+		else if (!strcmp(name + wholen, "name"))
+			v->s = copy_name(wholine);
+		else if (!strcmp(name + wholen, "email"))
+			v->s = copy_email(wholine);
+		else if (starts_with(name + wholen, "date"))
+			grab_date(wholine, v, name);
+	}
+
+	/*
+	 * For a tag or a commit object, if "creator" or "creatordate" is
+	 * requested, do something special.
+	 */
+	if (strcmp(who, "tagger") && strcmp(who, "committer"))
+		return; /* "author" for commit object is not wanted */
+	if (!wholine)
+		wholine = find_wholine(who, wholen, buf, sz);
+	if (!wholine)
+		return;
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (starts_with(name, "creatordate"))
+			grab_date(wholine, v, name);
+		else if (!strcmp(name, "creator"))
+			v->s = copy_line(wholine);
+	}
+}
+
+static void find_subpos(const char *buf, unsigned long sz,
+			const char **sub, unsigned long *sublen,
+			const char **body, unsigned long *bodylen,
+			unsigned long *nonsiglen,
+			const char **sig, unsigned long *siglen)
+{
+	const char *eol;
+	/* skip past header until we hit empty line */
+	while (*buf && *buf != '\n') {
+		eol = strchrnul(buf, '\n');
+		if (*eol)
+			eol++;
+		buf = eol;
+	}
+	/* skip any empty lines */
+	while (*buf == '\n')
+		buf++;
+
+	/* parse signature first; we might not even have a subject line */
+	*sig = buf + parse_signature(buf, strlen(buf));
+	*siglen = strlen(*sig);
+
+	/* subject is first non-empty line */
+	*sub = buf;
+	/* subject goes to first empty line */
+	while (buf < *sig && *buf && *buf != '\n') {
+		eol = strchrnul(buf, '\n');
+		if (*eol)
+			eol++;
+		buf = eol;
+	}
+	*sublen = buf - *sub;
+	/* drop trailing newline, if present */
+	if (*sublen && (*sub)[*sublen - 1] == '\n')
+		*sublen -= 1;
+
+	/* skip any empty lines */
+	while (*buf == '\n')
+		buf++;
+	*body = buf;
+	*bodylen = strlen(buf);
+	*nonsiglen = *sig - buf;
+}
+
+/* See grab_values */
+static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+{
+	int i;
+	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
+	unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+		if (strcmp(name, "subject") &&
+		    strcmp(name, "body") &&
+		    strcmp(name, "contents") &&
+		    strcmp(name, "contents:subject") &&
+		    strcmp(name, "contents:body") &&
+		    strcmp(name, "contents:signature"))
+			continue;
+		if (!subpos)
+			find_subpos(buf, sz,
+				    &subpos, &sublen,
+				    &bodypos, &bodylen, &nonsiglen,
+				    &sigpos, &siglen);
+
+		if (!strcmp(name, "subject"))
+			v->s = copy_subject(subpos, sublen);
+		else if (!strcmp(name, "contents:subject"))
+			v->s = copy_subject(subpos, sublen);
+		else if (!strcmp(name, "body"))
+			v->s = xmemdupz(bodypos, bodylen);
+		else if (!strcmp(name, "contents:body"))
+			v->s = xmemdupz(bodypos, nonsiglen);
+		else if (!strcmp(name, "contents:signature"))
+			v->s = xmemdupz(sigpos, siglen);
+		else if (!strcmp(name, "contents"))
+			v->s = xstrdup(subpos);
+	}
+}
+
+/*
+ * We want to have empty print-string for field requests
+ * that do not apply (e.g. "authordate" for a tag object)
+ */
+static void fill_missing_values(struct atom_value *val)
+{
+	int i;
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct atom_value *v = &val[i];
+		if (v->s == NULL)
+			v->s = "";
+	}
+}
+
+/*
+ * 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).
+ * 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.
+ */
+static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+{
+	grab_common_values(val, deref, obj, buf, sz);
+	switch (obj->type) {
+	case OBJ_TAG:
+		grab_tag_values(val, deref, obj, buf, sz);
+		grab_sub_body_contents(val, deref, obj, buf, sz);
+		grab_person("tagger", val, deref, obj, buf, sz);
+		break;
+	case OBJ_COMMIT:
+		grab_commit_values(val, deref, obj, buf, sz);
+		grab_sub_body_contents(val, deref, obj, buf, sz);
+		grab_person("author", val, deref, obj, buf, sz);
+		grab_person("committer", val, deref, obj, buf, sz);
+		break;
+	case OBJ_TREE:
+		/* grab_tree_values(val, deref, obj, buf, sz); */
+		break;
+	case OBJ_BLOB:
+		/* grab_blob_values(val, deref, obj, buf, sz); */
+		break;
+	default:
+		die("Eh?  Object of type %d?", obj->type);
+	}
+}
+
+static inline char *copy_advance(char *dst, const char *src)
+{
+	while (*src)
+		*dst++ = *src++;
+	return dst;
+}
+
+/*
+ * 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 unsigned char *tagged;
+
+	ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
+
+	if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
+		unsigned char unused1[20];
+		ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
+					     unused1, NULL);
+		if (!ref->symref)
+			ref->symref = "";
+	}
+
+	/* Fill in specials first */
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &ref->value[i];
+		int deref = 0;
+		const char *refname;
+		const char *formatp;
+		struct branch *branch = NULL;
+
+		if (*name == '*') {
+			deref = 1;
+			name++;
+		}
+
+		if (starts_with(name, "refname"))
+			refname = ref->refname;
+		else if (starts_with(name, "symref"))
+			refname = ref->symref ? ref->symref : "";
+		else if (starts_with(name, "upstream")) {
+			const char *branch_name;
+			/* only local branches may have an upstream */
+			if (!skip_prefix(ref->refname, "refs/heads/",
+					 &branch_name))
+				continue;
+			branch = branch_get(branch_name);
+
+			refname = branch_get_upstream(branch, NULL);
+			if (!refname)
+				continue;
+		} else if (starts_with(name, "push")) {
+			const char *branch_name;
+			if (!skip_prefix(ref->refname, "refs/heads/",
+					 &branch_name))
+				continue;
+			branch = branch_get(branch_name);
+
+			refname = branch_get_push(branch, NULL);
+			if (!refname)
+				continue;
+		} else if (starts_with(name, "color:")) {
+			char color[COLOR_MAXLEN] = "";
+
+			if (color_parse(name + 6, color) < 0)
+				die(_("unable to parse format"));
+			v->s = xstrdup(color);
+			continue;
+		} else if (!strcmp(name, "flag")) {
+			char buf[256], *cp = buf;
+			if (ref->flag & REF_ISSYMREF)
+				cp = copy_advance(cp, ",symref");
+			if (ref->flag & REF_ISPACKED)
+				cp = copy_advance(cp, ",packed");
+			if (cp == buf)
+				v->s = "";
+			else {
+				*cp = '\0';
+				v->s = xstrdup(buf + 1);
+			}
+			continue;
+		} else if (!deref && grab_objectname(name, ref->objectname, v)) {
+			continue;
+		} else if (!strcmp(name, "HEAD")) {
+			const char *head;
+			unsigned char sha1[20];
+
+			head = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
+						  sha1, NULL);
+			if (!strcmp(ref->refname, head))
+				v->s = "*";
+			else
+				v->s = " ";
+			continue;
+		} else
+			continue;
+
+		formatp = strchr(name, ':');
+		if (formatp) {
+			int num_ours, num_theirs;
+
+			formatp++;
+			if (!strcmp(formatp, "short"))
+				refname = shorten_unambiguous_ref(refname,
+						      warn_ambiguous_refs);
+			else if (!strcmp(formatp, "track") &&
+				 (starts_with(name, "upstream") ||
+				  starts_with(name, "push"))) {
+				char buf[40];
+
+				if (stat_tracking_info(branch, &num_ours,
+						       &num_theirs, NULL))
+					continue;
+
+				if (!num_ours && !num_theirs)
+					v->s = "";
+				else if (!num_ours) {
+					sprintf(buf, "[behind %d]", num_theirs);
+					v->s = xstrdup(buf);
+				} else if (!num_theirs) {
+					sprintf(buf, "[ahead %d]", num_ours);
+					v->s = xstrdup(buf);
+				} else {
+					sprintf(buf, "[ahead %d, behind %d]",
+						num_ours, num_theirs);
+					v->s = xstrdup(buf);
+				}
+				continue;
+			} else if (!strcmp(formatp, "trackshort") &&
+				   (starts_with(name, "upstream") ||
+				    starts_with(name, "push"))) {
+				assert(branch);
+
+				if (stat_tracking_info(branch, &num_ours,
+							&num_theirs, NULL))
+					continue;
+
+				if (!num_ours && !num_theirs)
+					v->s = "=";
+				else if (!num_ours)
+					v->s = "<";
+				else if (!num_theirs)
+					v->s = ">";
+				else
+					v->s = "<>";
+				continue;
+			} else
+				die("unknown %.*s format %s",
+				    (int)(formatp - name), name, formatp);
+		}
+
+		if (!deref)
+			v->s = refname;
+		else {
+			int len = strlen(refname);
+			char *s = xmalloc(len + 4);
+			sprintf(s, "%s^{}", refname);
+			v->s = s;
+		}
+	}
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct atom_value *v = &ref->value[i];
+		if (v->s == NULL)
+			goto need_obj;
+	}
+	return;
+
+ need_obj:
+	buf = get_obj(ref->objectname, &obj, &size, &eaten);
+	if (!buf)
+		die("missing object %s for %s",
+		    sha1_to_hex(ref->objectname), ref->refname);
+	if (!obj)
+		die("parse_object_buffer failed on %s for %s",
+		    sha1_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->sha1;
+
+	/*
+	 * 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",
+		    sha1_to_hex(tagged), ref->refname);
+	if (!obj)
+		die("parse_object_buffer failed on %s for %s",
+		    sha1_to_hex(tagged), ref->refname);
+	grab_values(ref->value, 1, obj, buf, size);
+	if (!eaten)
+		free(buf);
+}
+
+/*
+ * Given a ref, return the value for the atom.  This lazily gets value
+ * out of the object by calling populate value.
+ */
+static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
+{
+	if (!ref->value) {
+		populate_value(ref);
+		fill_missing_values(ref->value);
+	}
+	*v = &ref->value[atom];
+}
+
+/*
+ * Return 1 if the refname matches with one of the patterns,
+ * otherwise 0.  The patterns can be literal prefix (e.g. a
+ * refname "refs/heads/master" matches a pattern "refs/heads/")
+ * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
+ */
+static int match_name_as_path(const char **pattern, const char *refname)
+{
+	int namelen = strlen(refname);
+	for (; *pattern; pattern++) {
+		const char *p = *pattern;
+		int plen = strlen(p);
+
+		if ((plen <= namelen) &&
+		    !strncmp(refname, p, plen) &&
+		    (refname[plen] == '\0' ||
+		     refname[plen] == '/' ||
+		     p[plen-1] == '/'))
+			return 1;
+		if (!wildmatch(p, refname, WM_PATHNAME, NULL))
+			return 1;
+	}
+	return 0;
+}
+
+/* Allocate space for a new ref_array_item and copy the objectname and flag to it */
+static struct ref_array_item *new_ref_array_item(const char *refname,
+						 const unsigned char *objectname,
+						 int flag)
+{
+	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
+	ref->refname = xstrdup(refname);
+	hashcpy(ref->objectname, objectname);
+	ref->flag = flag;
+
+	return ref;
+}
+
+/*
+ * A call-back given to for_each_ref().  Filter refs and keep them for
+ * later object processing.
+ */
+int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+{
+	struct ref_filter_cbdata *ref_cbdata = cb_data;
+	struct ref_filter *filter = &ref_cbdata->filter;
+	struct ref_array_item *ref;
+
+	if (flag & REF_BAD_NAME) {
+		  warning("ignoring ref with broken name %s", refname);
+		  return 0;
+	}
+
+	if (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))
+		return 0;
+
+	/*
+	 * We do not open the object yet; sort may only need refname
+	 * to do its job and the resulting list may yet to be pruned
+	 * by maxcount logic.
+	 */
+	ref = new_ref_array_item(refname, oid->hash, flag);
+
+	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
+	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
+	return 0;
+}
+
+/*  Free memory allocated for a ref_array_item */
+static void free_array_item(struct ref_array_item *item)
+{
+	free((char *)item->symref);
+	free(item->refname);
+	free(item);
+}
+
+/* Free all memory allocated for ref_array */
+void ref_array_clear(struct ref_array *array)
+{
+	int i;
+
+	for (i = 0; i < array->nr; i++)
+		free_array_item(array->items[i]);
+	free(array->items);
+	array->items = NULL;
+	array->nr = array->alloc = 0;
+}
+
+static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
+{
+	struct atom_value *va, *vb;
+	int cmp;
+	cmp_type cmp_type = used_atom_type[s->atom];
+
+	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);
+		break;
+	default:
+		if (va->ul < vb->ul)
+			cmp = -1;
+		else if (va->ul == vb->ul)
+			cmp = 0;
+		else
+			cmp = 1;
+		break;
+	}
+	return (s->reverse) ? -cmp : cmp;
+}
+
+static struct ref_sorting *ref_sorting;
+static int compare_refs(const void *a_, const void *b_)
+{
+	struct ref_array_item *a = *((struct ref_array_item **)a_);
+	struct ref_array_item *b = *((struct ref_array_item **)b_);
+	struct ref_sorting *s;
+
+	for (s = ref_sorting; s; s = s->next) {
+		int cmp = cmp_ref_sorting(s, a, b);
+		if (cmp)
+			return cmp;
+	}
+	return 0;
+}
+
+void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
+{
+	ref_sorting = sorting;
+	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
+}
+
+static void print_value(struct atom_value *v, int quote_style)
+{
+	struct strbuf sb = STRBUF_INIT;
+	switch (quote_style) {
+	case QUOTE_NONE:
+		fputs(v->s, stdout);
+		break;
+	case QUOTE_SHELL:
+		sq_quote_buf(&sb, v->s);
+		break;
+	case QUOTE_PERL:
+		perl_quote_buf(&sb, v->s);
+		break;
+	case QUOTE_PYTHON:
+		python_quote_buf(&sb, v->s);
+		break;
+	case QUOTE_TCL:
+		tcl_quote_buf(&sb, v->s);
+		break;
+	}
+	if (quote_style != QUOTE_NONE) {
+		fputs(sb.buf, stdout);
+		strbuf_release(&sb);
+	}
+}
+
+static int hex1(char ch)
+{
+	if ('0' <= ch && ch <= '9')
+		return ch - '0';
+	else if ('a' <= ch && ch <= 'f')
+		return ch - 'a' + 10;
+	else if ('A' <= ch && ch <= 'F')
+		return ch - 'A' + 10;
+	return -1;
+}
+static int hex2(const char *cp)
+{
+	if (cp[0] && cp[1])
+		return (hex1(cp[0]) << 4) | hex1(cp[1]);
+	else
+		return -1;
+}
+
+static void emit(const char *cp, const char *ep)
+{
+	while (*cp && (!ep || cp < ep)) {
+		if (*cp == '%') {
+			if (cp[1] == '%')
+				cp++;
+			else {
+				int ch = hex2(cp + 1);
+				if (0 <= ch) {
+					putchar(ch);
+					cp += 3;
+					continue;
+				}
+			}
+		}
+		putchar(*cp);
+		cp++;
+	}
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
+{
+	const char *cp, *sp, *ep;
+
+	for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
+		struct atom_value *atomv;
+
+		ep = strchr(sp, ')');
+		if (cp < sp)
+			emit(cp, sp);
+		get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
+		print_value(atomv, quote_style);
+	}
+	if (*cp) {
+		sp = cp + strlen(cp);
+		emit(cp, sp);
+	}
+	if (need_color_reset_at_eol) {
+		struct atom_value resetv;
+		char color[COLOR_MAXLEN] = "";
+
+		if (color_parse("reset", color) < 0)
+			die("BUG: couldn't parse 'reset' as a color");
+		resetv.s = color;
+		print_value(&resetv, quote_style);
+	}
+	putchar('\n');
+}
+
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sorting *ref_default_sorting(void)
+{
+	static const char cstr_name[] = "refname";
+
+	struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));
+
+	sorting->next = NULL;
+	sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
+	return sorting;
+}
+
+int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
+{
+	struct ref_sorting **sorting_tail = opt->value;
+	struct ref_sorting *s;
+	int len;
+
+	if (!arg) /* should --no-sort void the list ? */
+		return -1;
+
+	s = xcalloc(1, sizeof(*s));
+	s->next = *sorting_tail;
+	*sorting_tail = s;
+
+	if (*arg == '-') {
+		s->reverse = 1;
+		arg++;
+	}
+	len = strlen(arg);
+	s->atom = parse_ref_filter_atom(arg, arg+len);
+	return 0;
+}
-- 
2.4.2

[PATCH v7 11/12] for-each-ref: introduce filter_refs()

[Karthik Nayak]

Introduce filter_refs() which will act as an API for filtering
a set of refs. Based on the type of refs the user has requested,
we iterate through those refs and apply filters as per the
given ref_filter structure and finally store the filtered refs
in the ref_array structure.

Currently this will wrap around ref_filter_handler(). Hence,
ref_filter_handler is made file scope static.

As users of this API will no longer send a ref_filter_cbdata
structure directly, we make the elements of ref_filter_cbdata
pointers. We can now use the information given by the users
to obtain our own ref_filter_cbdata structure. Changes are made to
support the change in ref_filter_cbdata structure.

Make 'for-each-ref' use this API.

Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 21 +++++++++++----------
 ref-filter.c           | 30 ++++++++++++++++++++++++++----
 ref-filter.h           | 16 ++++++++++++----
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 637fc4a..e2f15e6 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -16,7 +16,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	int maxcount = 0, quote_style = 0;
-	struct ref_filter_cbdata ref_cbdata;
+	struct ref_array array;
+	struct ref_filter filter;
 
 	struct option opts[] = {
 		OPT_BIT('s', "shell", &quote_style,
@@ -54,16 +55,16 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
-	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
-	ref_cbdata.filter.name_patterns = argv;
-	for_each_rawref(ref_filter_handler, &ref_cbdata);
+	memset(&array, 0, sizeof(array));
+	memset(&filter, 0, sizeof(filter));
+	filter.name_patterns = argv;
+	filter_refs(&array, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN, &filter);
+	ref_array_sort(sorting, &array);
 
-	ref_array_sort(sorting, &ref_cbdata.array);
-
-	if (!maxcount || ref_cbdata.array.nr < maxcount)
-		maxcount = ref_cbdata.array.nr;
+	if (!maxcount || array.nr < maxcount)
+		maxcount = array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref_array_item(ref_cbdata.array.items[i], format, quote_style);
-	ref_array_clear(&ref_cbdata.array);
+		show_ref_array_item(array.items[i], format, quote_style);
+	ref_array_clear(&array);
 	return 0;
 }
diff --git a/ref-filter.c b/ref-filter.c
index 74fe295..6f7defc 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -859,10 +859,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
  * later object processing.
  */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
-	struct ref_filter *filter = &ref_cbdata->filter;
+	struct ref_filter *filter = ref_cbdata->filter;
 	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
@@ -880,8 +880,8 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	 */
 	ref = new_ref_array_item(refname, oid->hash, flag);
 
-	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
-	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
+	REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
+	ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
 	return 0;
 }
 
@@ -905,6 +905,28 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, unsigned int type, struct ref_filter *filter)
+{
+	struct ref_filter_cbdata ref_cbdata;
+
+	ref_cbdata.array = array;
+	ref_cbdata.filter = filter;
+
+	if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
+		return for_each_rawref(ref_filter_handler, &ref_cbdata);
+	else if (type & FILTER_REFS_ALL)
+		return for_each_ref(ref_filter_handler, &ref_cbdata);
+	else
+		die("filter_refs: invalid type");
+	return 0;
+}
+
 static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
diff --git a/ref-filter.h b/ref-filter.h
index d9e043f..f917ebc 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -13,6 +13,9 @@
 #define QUOTE_PYTHON 4
 #define QUOTE_TCL 8
 
+#define FILTER_REFS_INCLUDE_BROKEN 0x1
+#define FILTER_REFS_ALL 0x2
+
 struct atom_value {
 	const char *s;
 	unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -42,12 +45,17 @@ struct ref_filter {
 };
 
 struct ref_filter_cbdata {
-	struct ref_array array;
-	struct ref_filter filter;
+	struct ref_array *array;
+	struct ref_filter *filter;
 };
 
-/*  Callback function for for_each_*ref(). This filters the refs based on the filters set */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data);
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, unsigned int type, struct ref_filter *filter);
 /*  Clear all memory allocated to ref_array */
 void ref_array_clear(struct ref_array *array);
 /*  Parse format string and sort specifiers */
-- 
2.4.2

Re: [PATCH v7 11/12] for-each-ref: introduce filter_refs()

[Matthieu Moy]

Karthik Nayak <karthik.188@gmail.com> writes:

> +	filter_refs(&array, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN, &filter);

I think it is more common to have options at the end, so I'd write it as

filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);

(changing the declaration too, obviously)

I really like the way cmd_for_each_ref looks like now.

> @@ -905,6 +905,28 @@ void ref_array_clear(struct ref_array *array)
>  	array->nr = array->alloc = 0;
>  }
>  
> +/*
> + * API for filtering a set of refs. Based on the type of refs the user
> + * has requested, we iterate through those refs and apply filters
> + * as per the given ref_filter structure and finally store the
> + * filtered refs in the ref_array structure.
> + */
> +int filter_refs(struct ref_array *array, unsigned int type, struct ref_filter *filter)
> +{
> +	struct ref_filter_cbdata ref_cbdata;
> +
> +	ref_cbdata.array = array;
> +	ref_cbdata.filter = filter;
> +
> +	if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
> +		return for_each_rawref(ref_filter_handler, &ref_cbdata);
> +	else if (type & FILTER_REFS_ALL)
> +		return for_each_ref(ref_filter_handler, &ref_cbdata);
> +	else
> +		die("filter_refs: invalid type");
> +	return 0;
> +}

I thought you would make a helper function that would return a pointer
to either for_each_rawref or for_each_ref (or another later), but that
would probably be overkill.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v7 11/12] for-each-ref: introduce filter_refs()

[Karthik Nayak]

On 06/11/2015 10:30 PM, Matthieu Moy wrote:
>
> I think it is more common to have options at the end, so I'd write it as
>
> filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);
>
> (changing the declaration too, obviously)
>
> I really like the way cmd_for_each_ref looks like now.
>

Ok! will reply to this mail with a small changed patch!

>
> I thought you would make a helper function that would return a pointer
> to either for_each_rawref or for_each_ref (or another later), but that
> would probably be overkill.
>

Yea that does seem like an overkill!

-- 
Regards,
Karthik

Re: [PATCH v7 11/12] for-each-ref: introduce filter_refs()

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +	filter_refs(&array, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN, &filter);
>
> I think it is more common to have options at the end, so I'd write it as
>
> filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);
>
> (changing the declaration too, obviously)

Good point.

>
> I really like the way cmd_for_each_ref looks like now.

Likewise.

[PATCH v7 11/12] for-each-ref: introduce filter_refs()

[Karthik Nayak]

Introduce filter_refs() which will act as an API for filtering
a set of refs. Based on the type of refs the user has requested,
we iterate through those refs and apply filters as per the
given ref_filter structure and finally store the filtered refs
in the ref_array structure.

Currently this will wrap around ref_filter_handler(). Hence,
ref_filter_handler is made file scope static.

As users of this API will no longer send a ref_filter_cbdata
structure directly, we make the elements of ref_filter_cbdata
pointers. We can now use the information given by the users
to obtain our own ref_filter_cbdata structure. Changes are made to
support the change in ref_filter_cbdata structure.

Make 'for-each-ref' use this API.

Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/for-each-ref.c | 21 +++++++++++----------
 ref-filter.c           | 30 ++++++++++++++++++++++++++----
 ref-filter.h           | 16 ++++++++++++----
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 637fc4a..7919206 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -16,7 +16,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	int maxcount = 0, quote_style = 0;
-	struct ref_filter_cbdata ref_cbdata;
+	struct ref_array array;
+	struct ref_filter filter;
 
 	struct option opts[] = {
 		OPT_BIT('s', "shell", &quote_style,
@@ -54,16 +55,16 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
-	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
-	ref_cbdata.filter.name_patterns = argv;
-	for_each_rawref(ref_filter_handler, &ref_cbdata);
+	memset(&array, 0, sizeof(array));
+	memset(&filter, 0, sizeof(filter));
+	filter.name_patterns = argv;
+	filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);
+	ref_array_sort(sorting, &array);
 
-	ref_array_sort(sorting, &ref_cbdata.array);
-
-	if (!maxcount || ref_cbdata.array.nr < maxcount)
-		maxcount = ref_cbdata.array.nr;
+	if (!maxcount || array.nr < maxcount)
+		maxcount = array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref_array_item(ref_cbdata.array.items[i], format, quote_style);
-	ref_array_clear(&ref_cbdata.array);
+		show_ref_array_item(array.items[i], format, quote_style);
+	ref_array_clear(&array);
 	return 0;
 }
diff --git a/ref-filter.c b/ref-filter.c
index 74fe295..9371091 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -859,10 +859,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
  * later object processing.
  */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
-	struct ref_filter *filter = &ref_cbdata->filter;
+	struct ref_filter *filter = ref_cbdata->filter;
 	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
@@ -880,8 +880,8 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	 */
 	ref = new_ref_array_item(refname, oid->hash, flag);
 
-	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
-	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
+	REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
+	ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
 	return 0;
 }
 
@@ -905,6 +905,28 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type)
+{
+	struct ref_filter_cbdata ref_cbdata;
+
+	ref_cbdata.array = array;
+	ref_cbdata.filter = filter;
+
+	if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
+		return for_each_rawref(ref_filter_handler, &ref_cbdata);
+	else if (type & FILTER_REFS_ALL)
+		return for_each_ref(ref_filter_handler, &ref_cbdata);
+	else
+		die("filter_refs: invalid type");
+	return 0;
+}
+
 static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
diff --git a/ref-filter.h b/ref-filter.h
index d9e043f..3999340 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -13,6 +13,9 @@
 #define QUOTE_PYTHON 4
 #define QUOTE_TCL 8
 
+#define FILTER_REFS_INCLUDE_BROKEN 0x1
+#define FILTER_REFS_ALL 0x2
+
 struct atom_value {
 	const char *s;
 	unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -42,12 +45,17 @@ struct ref_filter {
 };
 
 struct ref_filter_cbdata {
-	struct ref_array array;
-	struct ref_filter filter;
+	struct ref_array *array;
+	struct ref_filter *filter;
 };
 
-/*  Callback function for for_each_*ref(). This filters the refs based on the filters set */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data);
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type);
 /*  Clear all memory allocated to ref_array */
 void ref_array_clear(struct ref_array *array);
 /*  Parse format string and sort specifiers */
-- 
2.4.2

[PATCH v7 12/12] ref-filter: make 'ref_array_item' use a FLEX_ARRAY for refname

[Karthik Nayak]

This would remove the need of using a pointer to store refname.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 ref-filter.c | 7 ++++---
 ref-filter.h | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 6f7defc..310ecd6 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -847,8 +847,10 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 						 const unsigned char *objectname,
 						 int flag)
 {
-	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
-	ref->refname = xstrdup(refname);
+	size_t len = strlen(refname);
+	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);
+	memcpy(ref->refname, refname, len);
+	ref->refname[len] = '\0';
 	hashcpy(ref->objectname, objectname);
 	ref->flag = flag;
 
@@ -889,7 +891,6 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 static void free_array_item(struct ref_array_item *item)
 {
 	free((char *)item->symref);
-	free(item->refname);
 	free(item);
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index f917ebc..6ab2a75 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -32,7 +32,7 @@ struct ref_array_item {
 	int flag;
 	const char *symref;
 	struct atom_value *value;
-	char *refname;
+	char refname[FLEX_ARRAY];
 };
 
 struct ref_array {
-- 
2.4.2

Re: [PATCH v7 01/12] for-each-ref: extract helper functions out of grab_single_ref()

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Extract two helper functions out of grab_single_ref(). Firstly,
> new_refinfo() which is used to allocate memory for a new refinfo
> structure and copy the objectname, refname and flag to it.
> Secondly, match_name_as_path() which when given an array of patterns
> and the refname checks if the refname matches any of the patterns
> given while the pattern is a pathname, also supports wildcard
> characters.
>
> This is a preperatory patch for restructuring 'for-each-ref' and
> eventually moving most of it to 'ref-filter' to provide the
> functionality to similar commands via public API's.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  builtin/for-each-ref.c | 64 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index f7e51a7..67c8b62 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -851,6 +851,44 @@ struct grab_ref_cbdata {
>  };
>  
>  /*
> + * Return 1 if the refname matches with one of the patterns,

s/with //;

> + * otherwise 0.  The patterns can be literal prefix (e.g. a
> + * refname "refs/heads/master" matches a pattern "refs/heads/")
> + * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
> + */

I know this was my bad suggestion, but "refs/heads/m" can be thought
of as a "literal prefix" that may match "refs/heads/master"; we do
not want to make that match, so perhaps "literal" is a bad way to
say this.  "A pattern can be a path prefix or a worldcard"?

Re: [PATCH v7 01/12] for-each-ref: extract helper functions out of grab_single_ref()

[Karthik Nayak]

On 06/12/2015 11:00 PM, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Extract two helper functions out of grab_single_ref(). Firstly,
>> new_refinfo() which is used to allocate memory for a new refinfo
>> structure and copy the objectname, refname and flag to it.
>> Secondly, match_name_as_path() which when given an array of patterns
>> and the refname checks if the refname matches any of the patterns
>> given while the pattern is a pathname, also supports wildcard
>> characters.
>>
>> This is a preperatory patch for restructuring 'for-each-ref' and
>> eventually moving most of it to 'ref-filter' to provide the
>> functionality to similar commands via public API's.
>>
>> Helped-by: Junio C Hamano <gitster@pobox.com>
>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>   builtin/for-each-ref.c | 64 ++++++++++++++++++++++++++++++++------------------
>>   1 file changed, 41 insertions(+), 23 deletions(-)
>>
>> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
>> index f7e51a7..67c8b62 100644
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -851,6 +851,44 @@ struct grab_ref_cbdata {
>>   };
>>
>>   /*
>> + * Return 1 if the refname matches with one of the patterns,
>
> s/with //;

Thanks! will change :)

>
>> + * otherwise 0.  The patterns can be literal prefix (e.g. a
>> + * refname "refs/heads/master" matches a pattern "refs/heads/")
>> + * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
>> + */
>
> I know this was my bad suggestion, but "refs/heads/m" can be thought
> of as a "literal prefix" that may match "refs/heads/master"; we do
> not want to make that match, so perhaps "literal" is a bad way to
> say this.  "A pattern can be a path prefix or a worldcard"?
>

Yes! that sounds right, after all its doing a path match.

-- 
Regards,
Karthik

Re: [PATCH v7 0/12] Create ref-filter from for-each-ref

[Matthieu Moy]

Karthik Nayak <karthik.188@gmail.com> writes:

> The previous version of this patch can be found here:
> http://thread.gmane.org/gmane.comp.version-control.git/270922
>
> Changes found in this version:
> *    Various changes to the 'filter_refs()' function.
> *    Split 'for-each-ref: clean up code' into two commits.
> *    Other small changes.

My previous comments are all taken into account. The whole series looks
good to me now.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/