Re: [PATCH 1/2] clone: do not ignore --no-hardlinks

From: Jeff King <peff@peff.net>
Date: Feb 27, 2009, 11:35 UTC
Message-ID: <20090227113558.GA19756@coredump.intra.peff.net>
In-Reply-To: <7vr61kmht9.fsf@gitster.siamese.dyndns.org>

On Thu, Feb 26, 2009 at 11:59:14PM -0800, Junio C Hamano wrote:

Show 5 quoted lines

> Except that it does not work.
> 
> Initializing option_local to -1 and then feeding it to parse_options() via
> OPT_BOOLEAN() is a cute idea, but when parse_options() sees a command line
> like this:

Ah, sorry. I didn't really test it thoroughly (though I did try --no-local, I obviously did not test --local with a hardlink failure).

> Damn it, it is called BOOLEAN, and naïvely I would think --option would
> set it to 1, --no-option would set it to 0, and not having --option nor
> --no-option would leave the associated variable alone, but apparently that
> is not what is happening.

That is exactly what I expected as a caller (and it sounds like you did, too), which perhaps argues in favor of changing it.

Upon seeing this, I guessed (as you did in the next email) that it was about bumping up levels of a boolean for things like verbosity. And I agree that sould not be OPT_BOOLEAN, but rather OPT_INCREMENT or whatever.

However, I did some grepping and it doesn't look like anybody is actually making use of the incrementing behavior (I was just looking for "verbose", though, so it's possible some unrelated boolean is using it). So it may be that the behavior can be changed now while no callers care, and we can get away without OPT_INCREMENT until somebody actually needs it.

-Peff