* [PATCH/WIP v3 1/4] for-each-ref: re-structure code for moving to 'ref-filter'
@ 2015-05-28 17:38 Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref() Karthik Nayak
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 17:38 UTC (permalink / raw)
To: Git List; +Cc: Christian Couder, Matthieu Moy
Hello,
After the second version of the WIP/PATCH ($gmane/269836), This is a
follow up.
Changes:
* Broke down the first commit into 3 separate commits, cause they were
logically different and needed to built up incrementally.
* renamed some functions which were made public so that they don't seem
vague.
* other small changes.
--
Regards,
Karthik
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref()
2015-05-28 17:38 [PATCH/WIP v3 1/4] for-each-ref: re-structure code for moving to 'ref-filter' Karthik Nayak
@ 2015-05-28 17:43 ` Karthik Nayak
2015-05-28 20:13 ` Matthieu Moy
2015-05-28 17:43 ` [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation Karthik Nayak
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 17:43 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, 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 while supporting wild
characters.
This is a preperatory patch for restructuring 'for-each-ref' and
evntually 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 | 76 ++++++++++++++++++++++++++++----------------------
1 file changed, 43 insertions(+), 33 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..919d45e 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -837,6 +837,43 @@ struct grab_ref_cbdata {
};
/*
+ * Given a refname, return 1 if the refname matches with one of the patterns
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
+ */
+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.
*/
@@ -844,47 +881,20 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
{
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);
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, sha1);
- ref->flag = flag;
+ ref = new_refinfo(refname, sha1, flag);
+
+ REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
+ cb->grab_array[cb->grab_cnt++] = ref;
- cnt = cb->grab_cnt;
- REALLOC_ARRAY(cb->grab_array, cnt + 1);
- cb->grab_array[cnt++] = ref;
- cb->grab_cnt = cnt;
return 0;
}
--
2.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
2015-05-28 17:38 [PATCH/WIP v3 1/4] for-each-ref: re-structure code for moving to 'ref-filter' Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref() Karthik Nayak
@ 2015-05-28 17:43 ` Karthik Nayak
2015-05-28 20:26 ` Matthieu Moy
2015-05-28 17:43 ` [PATCH/WIP v3 3/4] for-each-ref: rename some functions and make them public Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref' Karthik Nayak
3 siblings, 1 reply; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 17:43 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, Karthik Nayak
Rename 'refinfo' to 'ref_array_item' and intoduce '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.
Re-order the fields in 'ref_array_item' so that refname can be
eventually converted to a FLEX_ARRAY.
Introduce 'ref_filter_clear_data' to clear all allocated memory.
This is a preparatory patch to eventually move code from 'for-each-ref'
to 'ref-filter' and making it publically 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 | 103 +++++++++++++++++++++++++++++--------------------
1 file changed, 61 insertions(+), 42 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 919d45e..d9fd512 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -31,12 +31,26 @@ 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;
+};
+
+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 {
@@ -85,7 +99,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.
@@ -622,7 +636,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;
@@ -821,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);
@@ -830,12 +844,6 @@ static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
*v = &ref->value[atom];
}
-struct grab_ref_cbdata {
- struct refinfo **grab_array;
- const char **grab_pattern;
- int grab_cnt;
-};
-
/*
* Given a refname, return 1 if the refname matches with one of the patterns
* while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
@@ -860,12 +868,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;
@@ -879,26 +887,39 @@ static struct refinfo *new_refinfo(const char *refname,
*/
static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
{
- struct grab_ref_cbdata *cb = cb_data;
- struct refinfo *ref;
+ 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;
+ warning("ignoring ref with broken name %s", refname);
+ 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;
- ref = new_refinfo(refname, sha1, flag);
+ ref = new_ref_array_item(refname, sha1, 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;
}
-static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b)
+/* Free all memory allocated for ref_filter_cbdata */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
+{
+ int i;
+
+ for (i = 0; i < ref_cbdata->array.nr; i++)
+ free(ref_cbdata->array.items[i]);
+ free(ref_cbdata->array.items);
+ ref_cbdata->array.items = NULL;
+ ref_cbdata->array.nr = ref_cbdata->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;
int cmp;
@@ -925,8 +946,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) {
@@ -937,10 +958,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 *array)
{
ref_sort = sort;
- qsort(refs, num_refs, sizeof(struct refinfo *), 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)
@@ -1007,7 +1028,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;
@@ -1076,12 +1097,12 @@ 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 refinfo **refs;
- struct grab_ref_cbdata cbdata;
+ struct ref_filter_cbdata ref_cbdata;
+ memset(&ref_cbdata, 0, sizeof(ref_cbdata));
struct option opts[] = {
OPT_BIT('s', "shell", "e_style,
@@ -1119,17 +1140,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;
+ 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);
+ ref_filter_clear_data(&ref_cbdata);
return 0;
}
--
2.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH/WIP v3 3/4] for-each-ref: rename some functions and make them public
2015-05-28 17:38 [PATCH/WIP v3 1/4] for-each-ref: re-structure code for moving to 'ref-filter' Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref() Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation Karthik Nayak
@ 2015-05-28 17:43 ` Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref' Karthik Nayak
3 siblings, 0 replies; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 17:43 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, Karthik Nayak
Rename some of the functions and make them publically available.
This is a preparatory step for moving code from 'for-each-ref'
to 'ref-filter' to make meaningful, targeted services available to
other commands via public APIs.
Based-on-patch-by: Jeff King <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 | 45 +++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index d9fd512..f045def 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -112,7 +112,7 @@ static int need_color_reset_at_eol;
/*
* Used to parse format string and sort specifiers
*/
-static int parse_atom(const char *atom, const char *ep)
+int parse_ref_filter_atom(const char *atom, const char *ep)
{
const char *sp;
int i, at;
@@ -189,7 +189,7 @@ static const char *find_next(const char *cp)
* Make sure the format string is well formed, and parse out
* the used atoms.
*/
-static int verify_format(const char *format)
+int verify_ref_format(const char *format)
{
const char *cp, *sp;
@@ -201,7 +201,7 @@ static int verify_format(const char *format)
if (!ep)
return error("malformed format string %s", sp);
/* sp points at "%(" and ep points at the closing ")" */
- at = parse_atom(sp + 2, ep);
+ at = parse_ref_filter_atom(sp + 2, ep);
cp = ep + 1;
if (skip_prefix(used_atom[at], "color:", &color))
@@ -408,7 +408,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
/*
* We got here because atomname ends in "date" or "date<something>";
* it's not possible that <something> is not ":<format>" because
- * parse_atom() wouldn't have allowed it, so we can assume that no
+ * parse_ref_filter_atom() wouldn't have allowed it, so we can assume that no
* ":" means no format is specified, and use the default.
*/
formatp = strchr(atomname, ':');
@@ -835,7 +835,7 @@ static void populate_value(struct ref_array_item *ref)
* Given a ref, return the value for the atom. This lazily gets value
* out of the object by calling populate value.
*/
-static void get_value(struct ref_array_item *ref, int atom, struct atom_value **v)
+static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
{
if (!ref->value) {
populate_value(ref);
@@ -882,10 +882,10 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
}
/*
- * A call-back given to for_each_ref(). Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
{
struct ref_filter_cbdata *ref_cbdata = cb_data;
struct ref_filter *filter = &ref_cbdata->filter;
@@ -925,8 +925,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref
int cmp;
cmp_type cmp_type = used_atom_type[s->atom];
- get_value(a, s->atom, &va);
- get_value(b, s->atom, &vb);
+ get_ref_atom_value(a, s->atom, &va);
+ get_ref_atom_value(b, s->atom, &vb);
switch (cmp_type) {
case FIELD_STR:
cmp = strcmp(va->s, vb->s);
@@ -958,7 +958,7 @@ static int compare_refs(const void *a_, const void *b_)
return 0;
}
-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)
{
ref_sort = sort;
qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
@@ -1028,7 +1028,7 @@ static void emit(const char *cp, const char *ep)
}
}
-static void show_ref(struct ref_array_item *info, const char *format, int quote_style)
+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
{
const char *cp, *sp, *ep;
@@ -1038,7 +1038,7 @@ static void show_ref(struct ref_array_item *info, const char *format, int quote_
ep = strchr(sp, ')');
if (cp < sp)
emit(cp, sp);
- get_value(info, parse_atom(sp + 2, ep), &atomv);
+ get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
print_value(atomv, quote_style);
}
if (*cp) {
@@ -1057,18 +1057,19 @@ static void show_ref(struct ref_array_item *info, const char *format, int quote_
putchar('\n');
}
-static struct ref_sort *default_sort(void)
+/* If no sorting option is given, use refname to sort as default */
+struct ref_sort *ref_default_sort(void)
{
static const char cstr_name[] = "refname";
struct ref_sort *sort = xcalloc(1, sizeof(*sort));
sort->next = NULL;
- sort->atom = parse_atom(cstr_name, cstr_name + strlen(cstr_name));
+ sort->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
return sort;
}
-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)
{
struct ref_sort **sort_tail = opt->value;
struct ref_sort *s;
@@ -1086,7 +1087,7 @@ static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
arg++;
}
len = strlen(arg);
- s->atom = parse_atom(arg, arg+len);
+ s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
}
@@ -1118,7 +1119,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
OPT_STRING( 0 , "format", &format, N_("format"), N_("format to use for the output")),
OPT_CALLBACK(0 , "sort", sort_tail, N_("key"),
- N_("field name to sort on"), &opt_parse_sort),
+ N_("field name to sort on"), &opt_parse_ref_sort),
OPT_END(),
};
@@ -1131,24 +1132,24 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
error("more than one quoting style?");
usage_with_options(for_each_ref_usage, opts);
}
- if (verify_format(format))
+ if (verify_ref_format(format))
usage_with_options(for_each_ref_usage, opts);
if (!sort)
- sort = default_sort();
+ sort = ref_default_sort();
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
ref_cbdata.filter.name_patterns = argv;
- for_each_rawref(grab_single_ref, &ref_cbdata);
+ for_each_rawref(ref_filter_handler, &ref_cbdata);
- sort_refs(sort, &ref_cbdata.array);
+ sort_ref_array(sort, &ref_cbdata.array);
if (!maxcount || ref_cbdata.array.nr < maxcount)
maxcount = ref_cbdata.array.nr;
for (i = 0; i < maxcount; i++)
- show_ref(ref_cbdata.array.items[i], format, quote_style);
+ show_ref_array_item(ref_cbdata.array.items[i], format, quote_style);
ref_filter_clear_data(&ref_cbdata);
return 0;
}
--
2.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref'
2015-05-28 17:38 [PATCH/WIP v3 1/4] for-each-ref: re-structure code for moving to 'ref-filter' Karthik Nayak
` (2 preceding siblings ...)
2015-05-28 17:43 ` [PATCH/WIP v3 3/4] for-each-ref: rename some functions and make them public Karthik Nayak
@ 2015-05-28 17:43 ` Karthik Nayak
2015-05-28 20:35 ` Matthieu Moy
3 siblings, 1 reply; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 17:43 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, Karthik Nayak
Move most of the code from 'for-each-ref' to 'ref-filter' to make
it publically 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.
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 | 1089 +-----------------------------------------------
ref-filter.c | 1051 ++++++++++++++++++++++++++++++++++++++++++++++
ref-filter.h | 66 +++
4 files changed, 1119 insertions(+), 1088 deletions(-)
create mode 100644 ref-filter.c
create mode 100644 ref-filter.h
diff --git a/Makefile b/Makefile
index 36655d5..068d313 100644
--- a/Makefile
+++ b/Makefile
@@ -761,6 +761,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 f045def..2f11c01 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -1,1095 +1,8 @@
#include "builtin.h"
#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"
-
-/* Quoting styles */
-#define QUOTE_NONE 0
-#define QUOTE_SHELL 1
-#define QUOTE_PERL 2
-#define QUOTE_PYTHON 4
-#define QUOTE_TCL 8
-
-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_sort {
- struct ref_sort *next;
- int atom; /* index into used_atom 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;
-} 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" },
- { "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")) {
- /* only local branches may have an upstream */
- if (!starts_with(ref->refname, "refs/heads/"))
- continue;
- branch = branch_get(ref->refname + 11);
-
- if (!branch || !branch->merge || !branch->merge[0] ||
- !branch->merge[0]->dst)
- continue;
- refname = branch->merge[0]->dst;
- } 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")) {
- char buf[40];
-
- if (stat_tracking_info(branch, &num_ours,
- &num_theirs) != 1)
- 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")) {
- assert(branch);
-
- if (stat_tracking_info(branch, &num_ours,
- &num_theirs) != 1)
- 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];
-}
-
-/*
- * Given a refname, return 1 if the refname matches with one of the patterns
- * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
- * and so on, else return 0. Supports use of wild characters.
- */
-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 unsigned char *sha1, 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;
-
- ref = new_ref_array_item(refname, sha1, flag);
-
- REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
- ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
-
- return 0;
-}
-
-/* Free all memory allocated for ref_filter_cbdata */
-void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
-{
- int i;
-
- for (i = 0; i < ref_cbdata->array.nr; i++)
- free(ref_cbdata->array.items[i]);
- free(ref_cbdata->array.items);
- ref_cbdata->array.items = NULL;
- ref_cbdata->array.nr = ref_cbdata->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;
- 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_sort *ref_sort;
-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;
-
- for (s = ref_sort; s; s = s->next) {
- int cmp = cmp_ref_sort(s, a, b);
- if (cmp)
- return cmp;
- }
- return 0;
-}
-
-void sort_ref_array(struct ref_sort *sort, struct ref_array *array)
-{
- ref_sort = sort;
- qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
-}
-
-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_sort *ref_default_sort(void)
-{
- static const char cstr_name[] = "refname";
-
- struct ref_sort *sort = xcalloc(1, sizeof(*sort));
-
- sort->next = NULL;
- sort->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
- return sort;
-}
-
-int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)
-{
- struct ref_sort **sort_tail = opt->value;
- struct ref_sort *s;
- int len;
-
- if (!arg) /* should --no-sort void the list ? */
- return -1;
-
- s = xcalloc(1, sizeof(*s));
- s->next = *sort_tail;
- *sort_tail = s;
-
- if (*arg == '-') {
- s->reverse = 1;
- arg++;
- }
- len = strlen(arg);
- s->atom = parse_ref_filter_atom(arg, arg+len);
- return 0;
-}
+#include "ref-filter.h"
static char const * const for_each_ref_usage[] = {
N_("git for-each-ref [<options>] [<pattern>]"),
diff --git a/ref-filter.c b/ref-filter.c
new file mode 100644
index 0000000..cdef681
--- /dev/null
+++ b/ref-filter.c
@@ -0,0 +1,1051 @@
+#include "builtin.h"
+#include "cache.h"
+#include "parse-options.h"
+#include "refs.h"
+#include "ref-filter.h"
+#include "wildmatch.h"
+#include "commit.h"
+#include "remote.h"
+#include "color.h"
+#include "tag.h"
+#include "quote.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" },
+ { "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")) {
+ /* only local branches may have an upstream */
+ if (!starts_with(ref->refname, "refs/heads/"))
+ continue;
+ branch = branch_get(ref->refname + 11);
+
+ if (!branch || !branch->merge || !branch->merge[0] ||
+ !branch->merge[0]->dst)
+ continue;
+ refname = branch->merge[0]->dst;
+ } 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")) {
+ char buf[40];
+
+ if (stat_tracking_info(branch, &num_ours,
+ &num_theirs) != 1)
+ 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")) {
+ assert(branch);
+
+ if (stat_tracking_info(branch, &num_ours,
+ &num_theirs) != 1)
+ 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];
+}
+
+/*
+ * Given a refname, return 1 if the refname matches with one of the patterns
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
+ */
+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 unsigned char *sha1, 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;
+
+ ref = new_ref_array_item(refname, sha1, flag);
+
+ REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
+ ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
+
+ return 0;
+}
+
+/* Free all memory allocated for ref_filter_cbdata */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
+{
+ int i;
+
+ for (i = 0; i < ref_cbdata->array.nr; i++)
+ free(ref_cbdata->array.items[i]);
+ free(ref_cbdata->array.items);
+ ref_cbdata->array.items = NULL;
+ ref_cbdata->array.nr = ref_cbdata->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;
+ 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_sort *ref_sort;
+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;
+
+ for (s = ref_sort; s; s = s->next) {
+ int cmp = cmp_ref_sort(s, a, b);
+ if (cmp)
+ return cmp;
+ }
+ return 0;
+}
+
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)
+{
+ ref_sort = sort;
+ qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
+}
+
+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_sort *ref_default_sort(void)
+{
+ static const char cstr_name[] = "refname";
+
+ struct ref_sort *sort = xcalloc(1, sizeof(*sort));
+
+ sort->next = NULL;
+ sort->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
+ return sort;
+}
+
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)
+{
+ struct ref_sort **sort_tail = opt->value;
+ struct ref_sort *s;
+ int len;
+
+ if (!arg) /* should --no-sort void the list ? */
+ return -1;
+
+ s = xcalloc(1, sizeof(*s));
+ s->next = *sort_tail;
+ *sort_tail = s;
+
+ if (*arg == '-') {
+ s->reverse = 1;
+ arg++;
+ }
+ len = strlen(arg);
+ s->atom = parse_ref_filter_atom(arg, arg+len);
+ return 0;
+}
diff --git a/ref-filter.h b/ref-filter.h
new file mode 100644
index 0000000..dacae59
--- /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_sort {
+ struct ref_sort *next;
+ int atom; /* index into used_atom 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 unsigned char *sha1, int flag, void *cb_data);
+/* Clear all memory allocated to ref_filter_data */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata);
+/* 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_sort provided */
+void sort_ref_array(struct ref_sort *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 opt_parse_ref_sort(const struct option *opt, const char *arg, int unset);
+/* Default sort option based on refname */
+struct ref_sort *ref_default_sort(void);
+
+#endif /* REF_FILTER_H */
--
2.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref()
2015-05-28 17:43 ` [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref() Karthik Nayak
@ 2015-05-28 20:13 ` Matthieu Moy
2015-05-28 20:33 ` Karthik Nayak
0 siblings, 1 reply; 15+ messages in thread
From: Matthieu Moy @ 2015-05-28 20:13 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, christian.couder
Karthik Nayak <karthik.188@gmail.com> writes:
> This is a preperatory patch for restructuring 'for-each-ref' and
> evntually moving most of it to 'ref-filter' to provide the
s/evntually/eventually/
The patch looks much nicer now.
> - int cnt;
> [...]
> + REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
> + cb->grab_array[cb->grab_cnt++] = ref;
>
> - cnt = cb->grab_cnt;
> - REALLOC_ARRAY(cb->grab_array, cnt + 1);
> - cb->grab_array[cnt++] = ref;
> - cb->grab_cnt = cnt;
This hunk is not advertized in the commit message, and I had to fight a
bit to understand what it does with "we're extracting helper functions"
in mind. It would have been much easier to review in a separate patch
entitled "for-each-ref: simplify code" for example.
But I agree that your version is simpler indeed.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
2015-05-28 17:43 ` [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation Karthik Nayak
@ 2015-05-28 20:26 ` Matthieu Moy
2015-05-28 20:37 ` Karthik Nayak
2015-05-28 21:18 ` Junio C Hamano
0 siblings, 2 replies; 15+ messages in thread
From: Matthieu Moy @ 2015-05-28 20:26 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, christian.couder
Karthik Nayak <karthik.188@gmail.com> writes:
> Rename 'refinfo' to 'ref_array_item' and intoduce 'ref_filter_cbdata'
The fact that you need to use "and" to describe your changes is a hint
that you can split better.
The patch looks much better, but I think you still need to split more to
make it easier to review.
> - * of properties that we need to extract out of objects. refinfo
> + * of properties that we need to extract out of objects. ref_array_item
Not very important, but two spaces after a period is what one is
supposed to do in English. Not everybody follow the rule, but it seems
backward to change the code to break it.
> if (flag & REF_BAD_NAME) {
> - warning("ignoring ref with broken name %s", refname);
> - return 0;
> + warning("ignoring ref with broken name %s", refname);
> + return 0;
Whitespace fix mixed with actual code.
> -static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b)
> +/* Free all memory allocated for ref_filter_cbdata */
> +void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
> +{
> + int i;
> +
> + for (i = 0; i < ref_cbdata->array.nr; i++)
> + free(ref_cbdata->array.items[i]);
> + free(ref_cbdata->array.items);
> + ref_cbdata->array.items = NULL;
> + ref_cbdata->array.nr = ref_cbdata->array.alloc = 0;
> +}
And this one is a real behavior change, which would be much better
documented in its own patch with a proper commit message (we had a
memory leak before, we didn't care because it happened right before
exiting, but we can't accept that in a clean library).
Reviewing is not just about having a look and seeing if there's
something stupid. Reviewers are actually taking a lot of time to see if
the patch does not introduce new bugs, or looking for better ways to do
the same thing. Be nice with them!
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref()
2015-05-28 20:13 ` Matthieu Moy
@ 2015-05-28 20:33 ` Karthik Nayak
0 siblings, 0 replies; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 20:33 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, christian.couder
On 05/29/2015 01:43 AM, Matthieu Moy wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> This is a preperatory patch for restructuring 'for-each-ref' and
>> evntually moving most of it to 'ref-filter' to provide the
>
> s/evntually/eventually/
Noted.
>
> The patch looks much nicer now.
>
>> - int cnt;
>> [...]
>> + REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
>> + cb->grab_array[cb->grab_cnt++] = ref;
>>
>> - cnt = cb->grab_cnt;
>> - REALLOC_ARRAY(cb->grab_array, cnt + 1);
>> - cb->grab_array[cnt++] = ref;
>> - cb->grab_cnt = cnt;
>
> This hunk is not advertized in the commit message, and I had to fight a
> bit to understand what it does with "we're extracting helper functions"
> in mind. It would have been much easier to review in a separate patch
> entitled "for-each-ref: simplify code" for example.
>
> But I agree that your version is simpler indeed.
>
Will put it in a separate patch. Thanks for the review.
--
Regards,
Karthik
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref'
2015-05-28 17:43 ` [PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref' Karthik Nayak
@ 2015-05-28 20:35 ` Matthieu Moy
2015-05-28 20:46 ` Karthik Nayak
0 siblings, 1 reply; 15+ messages in thread
From: Matthieu Moy @ 2015-05-28 20:35 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, christian.couder
Karthik Nayak <karthik.188@gmail.com> writes:
> Move most of the code from 'for-each-ref' to 'ref-filter' to make
> it publically available to other commands, this is to unify the code
s/publically/publicly/
> --- /dev/null
> +++ b/ref-filter.h
Moving file to the .h file should be done in a separate patch. I don't
want to review the 1050 lines of cut-and-paste other than by doing the
cut-and-paste myself and see if I get the same result, but this part is
not as straightforward and needs proper thinking and review.
In short: the big code movement should be *only* a cut-and-paste, alone
in its own patch.
On overall, we're getting close to an acceptable version for these 4
patches. My advice: prioritize polishing these few patches, so that we
can do a detailed review and then consider this part "done" (either
merged in git.git or not, but I'd like to avoid having to review the
code movement & refactoring again when we move to the next patches).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
2015-05-28 20:26 ` Matthieu Moy
@ 2015-05-28 20:37 ` Karthik Nayak
2015-05-28 20:41 ` Matthieu Moy
2015-05-28 21:18 ` Junio C Hamano
1 sibling, 1 reply; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 20:37 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, christian.couder
On 05/29/2015 01:56 AM, Matthieu Moy wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Rename 'refinfo' to 'ref_array_item' and intoduce 'ref_filter_cbdata'
>
> The fact that you need to use "and" to describe your changes is a hint
> that you can split better.
>
But the patch alone wouldn't make much sense here, as the whole idea is
the introduction of the new structures and renaming 'refinfo' to
'ref_array_item' was more of a byproduct to go along with the new
structures introduced.
>
> The patch looks much better, but I think you still need to split more to
> make it easier to review.
>
>> - * of properties that we need to extract out of objects. refinfo
>> + * of properties that we need to extract out of objects. ref_array_item
>
> Not very important, but two spaces after a period is what one is
> supposed to do in English. Not everybody follow the rule, but it seems
> backward to change the code to break it.
>
I'm just so used to single spacing, Will change it.
>> if (flag & REF_BAD_NAME) {
>> - warning("ignoring ref with broken name %s", refname);
>> - return 0;
>> + warning("ignoring ref with broken name %s", refname);
>> + return 0;
>
> Whitespace fix mixed with actual code.
>
Noted.
>> -static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b)
>> +/* Free all memory allocated for ref_filter_cbdata */
>> +void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ref_cbdata->array.nr; i++)
>> + free(ref_cbdata->array.items[i]);
>> + free(ref_cbdata->array.items);
>> + ref_cbdata->array.items = NULL;
>> + ref_cbdata->array.nr = ref_cbdata->array.alloc = 0;
>> +}
>
> And this one is a real behavior change, which would be much better
> documented in its own patch with a proper commit message (we had a
> memory leak before, we didn't care because it happened right before
> exiting, but we can't accept that in a clean library).
>
Ok will put that into a separate commit.
> Reviewing is not just about having a look and seeing if there's
> something stupid. Reviewers are actually taking a lot of time to see if
> the patch does not introduce new bugs, or looking for better ways to do
> the same thing. Be nice with them!
>
Thanks for the effort from your side, will try to split things as much
as possible and make it easier for Reviewers.
--
Regards,
Karthik
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
2015-05-28 20:37 ` Karthik Nayak
@ 2015-05-28 20:41 ` Matthieu Moy
2015-05-28 20:43 ` Karthik Nayak
0 siblings, 1 reply; 15+ messages in thread
From: Matthieu Moy @ 2015-05-28 20:41 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, christian.couder
Karthik Nayak <karthik.188@gmail.com> writes:
> On 05/29/2015 01:56 AM, Matthieu Moy wrote:
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> Rename 'refinfo' to 'ref_array_item' and intoduce 'ref_filter_cbdata'
>>
>> The fact that you need to use "and" to describe your changes is a hint
>> that you can split better.
>>
>
> But the patch alone wouldn't make much sense here, as the whole idea
> is the introduction of the new structures and renaming 'refinfo' to
> ref_array_item' was more of a byproduct to go along with the new
> structures introduced.
The point of separating them is that the rename implies a relatively
long patch (you have 17 occurences of 'refinfo' in the deletion part of
your patch), but it is straightforward to review (apply, run "git diff
--color-words" and press space a few times). But it is no longer so
simple once you mix it with anything else.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
2015-05-28 20:41 ` Matthieu Moy
@ 2015-05-28 20:43 ` Karthik Nayak
0 siblings, 0 replies; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 20:43 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, christian.couder
On 05/29/2015 02:11 AM, Matthieu Moy wrote:
> The point of separating them is that the rename implies a relatively
> long patch (you have 17 occurences of 'refinfo' in the deletion part of
> your patch), but it is straightforward to review (apply, run "git diff
> --color-words" and press space a few times). But it is no longer so
> simple once you mix it with anything else.
>
Makes sense, thanks for putting it out.
--
Regards,
Karthik
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref'
2015-05-28 20:35 ` Matthieu Moy
@ 2015-05-28 20:46 ` Karthik Nayak
0 siblings, 0 replies; 15+ messages in thread
From: Karthik Nayak @ 2015-05-28 20:46 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, christian.couder
On 05/29/2015 02:05 AM, Matthieu Moy wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Move most of the code from 'for-each-ref' to 'ref-filter' to make
>> it publically available to other commands, this is to unify the code
>
> s/publically/publicly/
>
>> --- /dev/null
>> +++ b/ref-filter.h
>
> Moving file to the .h file should be done in a separate patch. I don't
> want to review the 1050 lines of cut-and-paste other than by doing the
> cut-and-paste myself and see if I get the same result, but this part is
> not as straightforward and needs proper thinking and review.
>
> In short: the big code movement should be *only* a cut-and-paste, alone
> in its own patch.
>
Ok, will separate it into two patches.
>
> On overall, we're getting close to an acceptable version for these 4
> patches. My advice: prioritize polishing these few patches, so that we
> can do a detailed review and then consider this part "done" (either
> merged in git.git or not, but I'd like to avoid having to review the
> code movement & refactoring again when we move to the next patches).
>
Sure I'll fix changes suggested by you and push to Github probably by
tonight (IST), will wait for a day or two to see if there are more
suggestions by others on the mailing list before sending a new patch
series here.
--
Regards,
Karthik
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
2015-05-28 20:26 ` Matthieu Moy
2015-05-28 20:37 ` Karthik Nayak
@ 2015-05-28 21:18 ` Junio C Hamano
2015-05-28 21:28 ` Junio C Hamano
1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2015-05-28 21:18 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Karthik Nayak, git, christian.couder
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Not very important, but two spaces after a period is what one is
> supposed to do in English. Not everybody follow the rule, but it seems
> backward to change the code to break it.
I am old fashioned enough to personally agree, but this practice
that originates in the typewriter era is frowned upon in many
circles, including those who work with professional typesetters, I
think.
Sent off-list to reduce noise level; this is an issue that attracts
a lot of useless comments.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
2015-05-28 21:18 ` Junio C Hamano
@ 2015-05-28 21:28 ` Junio C Hamano
0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2015-05-28 21:28 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Karthik Nayak, git, christian.couder
Junio C Hamano <gitster@pobox.com> writes:
> Sent off-list to reduce noise level; this is an issue that attracts
> a lot of useless comments.
Heh, somehow I screwed up and sent on-list X-<.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-05-28 21:28 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-28 17:38 [PATCH/WIP v3 1/4] for-each-ref: re-structure code for moving to 'ref-filter' Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref() Karthik Nayak
2015-05-28 20:13 ` Matthieu Moy
2015-05-28 20:33 ` Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation Karthik Nayak
2015-05-28 20:26 ` Matthieu Moy
2015-05-28 20:37 ` Karthik Nayak
2015-05-28 20:41 ` Matthieu Moy
2015-05-28 20:43 ` Karthik Nayak
2015-05-28 21:18 ` Junio C Hamano
2015-05-28 21:28 ` Junio C Hamano
2015-05-28 17:43 ` [PATCH/WIP v3 3/4] for-each-ref: rename some functions and make them public Karthik Nayak
2015-05-28 17:43 ` [PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref' Karthik Nayak
2015-05-28 20:35 ` Matthieu Moy
2015-05-28 20:46 ` Karthik Nayak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.