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

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

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