On Wed, Sep 2, 2015 at 2:15 PM, Matthieu Moy
<Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>
>> @@ -163,9 +174,28 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style)
>> }
>> }
>>
>> +static void align_handler(struct ref_formatting_stack *stack)
>
> Perhaps name it end_align_handler, to make the difference with
> align_atom_handler clear.
>
> Also, I think moving this function to be right next to
> align_atom_handler in the code would make this more readable.
>
will do.
>> @@ -754,6 +816,42 @@ static void populate_value(struct ref_array_item *ref)
>> else
>> v->s = " ";
>> continue;
>> + } else if (skip_prefix(name, "align", &valp)) {
>> + struct align *align = &v->align;
>> + struct strbuf **s;
>> +
>> + if (valp[0] != ':')
>> + die(_("format: usage %%(align:<width>,<position>)"));
>> + else
>> + valp++;
>
> I agree with Junio that skip_prefix(name, "align:" ...) is simpler for
> the same thing.
Correct me if mistaken, but Junio wanted the first skip_prefix to
check for "align:" rather than "align", but that would mean we don't have
the error printing.
Are you sure we want to skip that?
>
>> + if (state.stack->prev)
>> + die(_("format: `end` atom missing"));
>
> Perhaps spell it %(end) instead of just end.
>
Yes, will do
--
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