Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 10, 2015 at 10:58 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> The check was for checking if there is anything after the colon,
>
> Why do you even care?  If %(color) expects more specific
> customization by adding colon followed by specific data after it,
> i.e. %(color:something), %(color:) should clearly be that "%(color)"
> thing with no customization---if it is "not enough customization" or
> "leaving everything default" depends on each atom, match_atom_name()
> is not a good place to make that policy decision (i.e. Mattheiu's
> rewrite to clear *valp to NULL when %(color:) is seen).  Instead,
> point *val to body + 1 just as usual, and let the caller tell
> between *val being NULL (not even any colon) and *val pointing at a
> NUL byte (nothing after colon) if it cares.

It is one thing that the user can actually do the check themselves,
but doesn't it make more sense that when we're using colon we expect a
value after it, and something like %(color:) makes no sense when color
specifically needs a value after the colon.

Hence rather than assuming that the no value is given and fallback to
the default
%(color), wouldn't it be better to warn the user that there is no value?

But then again we can leave this decision to the user like you said.

-- 
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]