On Sun, Dec 13, 2015 at 6:23 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>> 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 <Karthik.188@xxxxxxxxx>
>> ---
>> 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.
>
Sure, will do that.
>> } 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?
>
Bad copy-paste, will change that.
>> +}
>> +
>> @@ -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?
>
This is sort of a problem with most of the values in ref-filter, we dynamically
allocate memory and do not free it, since the program exits soon after and
we leave it to the Operating System to do the garbage collection.
Not sure if we'd want to work on this though.
>> + } 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()?
>
No, this is only activated when using "refname". fill_remote_ref_details()
is used by the %(upstream) and %(push) atoms, both of which skip the
loop using "continue" in populate_value().
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html