Re: [PATCH/RFC 08/10] ref-filter: introduce remote_ref_atom_parser()
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:26
On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Introduce remote_ref_atom_parser() which will parse the '%(upstream)' and '%(push)' atoms and store information into the 'used_atom' structure based on the modifiers used along with the corresponding atom. Signed-off-by: Karthik Nayak <redacted> ---diff --git a/ref-filter.c b/ref-filter.c@@ -37,6 +37,11 @@ static struct used_atom { union { const char *color; struct align align; + struct { + unsigned int shorten : 1, + track : 1, + trackshort : 1; + } remote_ref;
Are 'shorten', 'track', and 'trackshort' mutually exclusive? If so, a
simple enum would be clearer than bitfields:
union {
const char *color;
struct align align;
enum { RR_PLAIN, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
remote_ref;
};
Or something.
quoted hunk ↗ jump to hunk
} u; } *used_atom; static int used_atom_cnt, need_tagged, need_symref;@@ -69,6 +74,24 @@ void color_atom_parser(struct used_atom *atom) +void remote_ref_atom_parser(struct used_atom *atom) +{ + const char *buf; + + buf = strchr(atom->str, ':'); + if (!buf) + return; + buf++; + if (!strcmp(buf, "short")) + atom->u.remote_ref.shorten = 1; + else if (!strcmp(buf, "track")) + atom->u.remote_ref.track = 1; + else if (!strcmp(buf, "trackshort")) + atom->u.remote_ref.trackshort = 1; + else + die(_("improper format entered align:%s"), buf);
"align:"? Also, how about a more grammatically-friendly error message?
quoted hunk ↗ jump to hunk
+} +@@ -838,6 +861,42 @@ static inline char *copy_advance(char *dst, const char *src) +static void fill_remote_ref_details(struct used_atom *atom, const char *refname, + struct branch *branch, const char **s) +{ + int num_ours, num_theirs; + if (atom->u.remote_ref.shorten) + *s = shorten_unambiguous_ref(refname, warn_ambiguous_refs); + else if (atom->u.remote_ref.track) { + if (stat_tracking_info(branch, &num_ours, + &num_theirs, NULL)) + return; + if (!num_ours && !num_theirs) + *s = ""; + else if (!num_ours) + *s = xstrfmt("[behind %d]", num_theirs); + else if (!num_theirs) + *s = xstrfmt("[ahead %d]", num_ours); + else + *s = xstrfmt("[ahead %d, behind %d]", + num_ours, num_theirs);
Tangent: These xstrfmt()'d strings are getting leaked, right? Is that something that we need to worry about (if, for instance, a repository contains a lot of tracking refs)? Should there be a NEEDSWORK comment here regarding the issue?
quoted hunk ↗ jump to hunk
+ } else if (atom->u.remote_ref.trackshort) { + if (stat_tracking_info(branch, &num_ours, + &num_theirs, NULL)) + return; + + if (!num_ours && !num_theirs) + *s = "="; + else if (!num_ours) + *s = "<"; + else if (!num_theirs) + *s = ">"; + else + *s = "<>"; + } else + *s = refname; +} + /* * Parse the object referred by ref, and grab needed value. */@@ -948,49 +1011,11 @@ static void populate_value(struct ref_array_item *ref) formatp = strchr(name, ':'); if (formatp) { - int num_ours, num_theirs; - formatp++; if (!strcmp(formatp, "short")) refname = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
Is this duplicating work already done by fill_remote_ref_details()?
- else if (!strcmp(formatp, "track") &&
- (starts_with(name, "upstream") ||
- starts_with(name, "push"))) {
-
- if (stat_tracking_info(branch, &num_ours,
- &num_theirs, NULL))
- continue;
-
- if (!num_ours && !num_theirs)
- v->s = "";
- else if (!num_ours)
- v->s = xstrfmt("[behind %d]", num_theirs);
- else if (!num_theirs)
- v->s = xstrfmt("[ahead %d]", num_ours);
- else
- v->s = xstrfmt("[ahead %d, behind %d]",
- num_ours, num_theirs);
- continue;
- } else if (!strcmp(formatp, "trackshort") &&
- (starts_with(name, "upstream") ||
- starts_with(name, "push"))) {
- assert(branch);
-
- if (stat_tracking_info(branch, &num_ours,
- &num_theirs, NULL))
- continue;
-
- if (!num_ours && !num_theirs)
- v->s = "=";
- else if (!num_ours)
- v->s = "<";
- else if (!num_theirs)
- v->s = ">";
- else
- v->s = "<>";
- continue;
- } else
+ else
die("unknown %.*s format %s",
(int)(formatp - name), name, formatp);
}
--
2.6.2