Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> @@ -687,6 +690,17 @@ static void populate_value(struct ref_array_item *ref)
> else
> v->s = " ";
> continue;
> + } else if (starts_with(name, "align:")) {
> + const char *valp = NULL;
> +
> + skip_prefix(name, "align:", &valp);
> + if (!valp[0])
> + die(_("No value given with 'align='"));
> + strtoul_ui(valp, 10, &ref->align_value);
> + if (ref->align_value < 1)
> + die(_("Value should be greater than zero"));
> + v->s = "";
> + continue;
> } else
> continue;
>
> @@ -1254,17 +1268,38 @@ static void emit(const char *cp, const char *ep)
> }
> }
>
> +static void assign_formating(struct ref_array_item *ref, int parsed_atom, struct atom_value *v)
> +{
> + if (v->s[0] && ref->align_value) {
> + unsigned int len = 0;
> +
> + len = utf8_strwidth(v->s);
> + if (ref->align_value > len) {
> + struct strbuf buf = STRBUF_INIT;
> + strbuf_addstr(&buf, v->s);
> + if (!v->s[0])
> + free((char *)v->s);
> + strbuf_addchars(&buf, ' ', ref->align_value - len);
> + v->s = strbuf_detach(&buf, NULL);
> + }
> + ref->align_value = 0;
> + }
> +}
What is your plan for this function? Is it envisioned that this
will gain more variations of formatting options over time?
Otherwise it seems misnamed (it is not "assign formatting" but
merely "pad to the right").
I also doubt that the logic is sane. More specifically, what does
that "if (v->s[0])" buy you?
Your caller is iterating over the elements in a format string,
e.g. 'A %(align:20)%(foo) B %(bar) C', and its caller is iterating
over a list of refs, e.g. 'maint', 'master' branches. With that
format string, as long as %(foo) does not expand to something that
exceeds 20 display places or so, I'd expect literal 'B' for all refs
to align, but I do not think this code gives me that; what happens
if '%(foo)' happens to be an empty string for 'maint' but is a
string, say 'x', for 'master'?
--
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