Re: [PATCH] parse-opt: migrate builtin-checkout-index.

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

 



Pierre Habouzit <madcoder@xxxxxxxxxx> writes:

> On Sun, Oct 19, 2008 at 08:11:31PM +0000, Junio C Hamano wrote:
>> Miklos Vajna <vmiklos@xxxxxxxxxxxxxx> writes:
>
>> >> Right, I fixed this in option_parse_z(). --no-z should set
>> >> line_termination to \n instead of 1.
>> >
>> > Originally in option_parse_z() I had
>> >
>> >         line_termination = unset;
>> >
>> > which is in fact right, because (as Pierre pointed out) unset for short
>> > options are always false, but I changed it to
>> >
>> >         line_termination = 0;
>> >
>> > to make it more readable.
>> 
>> I think Pierre's comment is short-sighted.  Think of what would happen
>> when somebody adds "--nul" as a longer equivalent to "-z", since it is
>> extremely easy to do things like that with the use of parse-opt API?
>
> Err I was only pointing out that --no-z would no nothing, I actually
> didn't really read the argument :)  I didn't say having --null was a bad
> idea,...

It is a good practice to anticipate potential future breakages, assess the
cost to avoid them, avoid the ones that can be avoided with minimum cost
(and document the others you know will be broken).  That's the key to
produce maintainable piece of code.  The change necessary to Miklos's
original code to do this is quite simple (i.e. flip between '\0' and
'\n'), and I didn't see any room for argument like "short option won't let
you say --no-z".  That's where my comment about "short-sighted"-ness comes
from.

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

  Powered by Linux