On Wed, Sep 2, 2015 at 8:31 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>>>> + die(_("format: `end` atom used without a supporting atom"));
>>>
>>> Not a show-stopper, but we may need some wordsmithing for "a
>>> supporting atom" here; an end-user would not know what it is.
>>
>> Probably something like "format: `end` atom should only be
>> used with modifier atoms".
>
> Between "supporting" and "modifier" I do not see much difference,
> though.
>
I don't see how we could provide a better message, as %(end) atom
would be common to various atoms eventually.
>>>> + } else if (skip_prefix(name, "align", &valp)) {
>>>
>>> This looked as if you are willing to take %(align) in addition to
>>> %(align:...), but...
>>>
>>>> + struct align *align = &v->align;
>>>> + struct strbuf **s;
>>>> +
>>>> + if (valp[0] != ':')
>>>> + die(_("format: usage %%(align:<width>,<position>)"));
>>>
>>> ... apparently that is not what is happening. Why not skip "align:"
>>> with colon as the prefix, then?
>>
>> Cause we wanted to provide an error for usage of "%(ailgn)" without any
>> subvalues as such.
>
> Wouldn't it be something that would be caught in the same codepath
> as what catches %(unrecognized) in the format string?
No, since "align" is defined as an atom, in the valid_atom struct.
Changing it to "align:" would work, but that seems a little inconsistent
with the other atoms.
Hence --format="%(align)foo%(end)" would just result in the
"format: `end` atom used without a supporting atom" error being
displayed, hence confusing the user even more.
--
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