On Mon, Jul 20, 2015 at 11:31 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Mon, Jul 20, 2015 at 1:22 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>> Add a new atom "align" and support %(align:X) where X is a number.
>> This will align the preceeding atom value to the left followed by
>> spaces for a total length of X characters. If X is less than the item
>> size, the entire atom value is printed.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
>> ---
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 7561727..93f59aa 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -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: align:%u"), ref->align_value);
>> + v->s = "";
>
> Mental note: v->s points at literal zero-length string ("").
>
>> + continue;
>> } else
>> continue;
>>
>> @@ -1254,17 +1268,40 @@ 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 (ref->align_value && !starts_with(used_atom[parsed_atom], "align")) {
>> + unsigned int len = 0;
>> +
>> + if (*v->s)
>> + len = utf8_strwidth(v->s);
>> + if (ref->align_value > len) {
>> + struct strbuf buf = STRBUF_INIT;
>> + if (*v->s)
>> + strbuf_addstr(&buf, v->s);
>> + if (*v->s && v->s[0] == '\0')
>> + free((char *)v->s);
>
> Is the "v->s[0] == '\0'" checking for the same literal zero-length
> string assigned above? If so, attempting to free() that string doesn't
> make sense, since it's not heap-allocated. Maybe you meant != '\0'?
>
> Overall, this code is getting rather complex and difficult to follow
> (especially with all the 'v->s' checks thrown in). Junio's proposed
> 'pseudo_atom' and 'ref_formatting_state' would go a long way toward
> simplifying it.
>
You're right, thats what I meant.
Having a look at Junio's proposed idea.
--
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