Re: [PATCH] parse-opt: migrate builtin-checkout-index.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:30
Pierre Habouzit [off-list ref] writes:
On Sun, Oct 19, 2008 at 08:11:31PM +0000, Junio C Hamano wrote:quoted
Miklos Vajna [off-list ref] writes:quoted
quoted
quoted
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.