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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 19, 2008, 20:59 UTC
Message-ID: <7v4p387146.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20081019201934.GO16610@artemis.corp>

Pierre Habouzit <madcoder@debian.org> writes:

Show 24 quoted lines

> On Sun, Oct 19, 2008 at 08:11:31PM +0000, Junio C Hamano wrote:
>> Miklos Vajna <vmiklos@frugalware.org> 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.