Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> 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.
I said "not a show-stopper" without giving a suggestion exactly
because I didn't (and I still don't) think either you or I can come
up with a good wording ;-). That is why the message was Cc'ed to
the list for others to comment.
>>> 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?
One potential issue you have with prefix matching with "align" is
that you can never have a different atom whose name happens to begin
with that substring. I think the best behaviour is for the higher
level parser to recognize %(ATOM) and %(ATOM:anything) and nothing
else for all ATOM in the valid atoms registry. That would prevent
%(ATOMfoo) from being handled as part of handling ATOM.
And make the code consider %(ATOM) form as a short-hand for %(ATOM:)
that does not have customizations. Conceptually %(refname) is a
%(refname:default).
For an atom like 'align' that does not have a reasonable default,
the parser for 'align' can notice that a required customization is
missing and give an error that is specific to 'align'.
So all calls in your code of the from
... else if (skip_prefix(name, "align", &val)) {
...
should become a call to helper that does more than skip_prefix().
int match_atom_name(const char *name, const char *atom_name, char **val)
{
char *body
if (!skip_prefix(name, atom_name, &body))
return 0; /* doesn't even begin with "align" */
if (!body[0]) {
*val = NULL; /* %(align) and no customization */
return 1;
}
if (body[0] != ':')
return 0; /* "alignfoo" is not "align" or "align:..." */
*val = body + 1; /* "align:val" */
return 1;
}
--
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