From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Pierre Habouzit <madcoder@debian.org>
Subject: Re: [PATCH] parse-options: deprecate OPT_BOOLEAN
Date: Tue, 27 Sep 2011 21:22:55 -0700 [thread overview]
Message-ID: <7vbou5s3c0.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20110928035809.GC10662@sigill.intra.peff.net> (Jeff King's message of "Tue, 27 Sep 2011 23:58:09 -0400")
Jeff King <peff@peff.net> writes:
> On Tue, Sep 27, 2011 at 04:56:49PM -0700, Junio C Hamano wrote:
>
>> It is natural to expect that an option defined with OPT_BOOLEAN() could be
>> used in this way:
>> [...]
>> to easily tell three cases apart:
>>
>> - There is no mention of the `--option` on the command line;
>> - The variable is positively set with `--option`; or
>> - The variable is explicitly negated with `--no-option`.
>>
>> Unfortunately, this is not the case. OPT_BOOLEAN() increments the variable
>> every time `--option` is given, and resets it to zero when `--no-option`
>> is given.
>
> Yes, please. I remember being bitten by this at one point. Your
> transition plan makes sense to me.
The best part of it is that this leaves many small bite-sized tasks, one
"struct option" per patch, most of which can be done by people who are not
uber experts in Git internals ;-).
> Can OPT_UYN be folded into this, as well?
Perhaps, but I would prefer to keep this simple at least during the first
pass of eradicating OPT_BOOLEAN and OPTION_BOOLEAN. After that is done, we
may want to tackle OPT_UYN() that uses 2 for "unset", which feels a tad
unnatural. It has only one user---even if it turns out that it was a
mistake, the damage would be relatively limited to fix it.
next prev parent reply other threads:[~2011-09-28 4:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-27 23:56 [PATCH] parse-options: deprecate OPT_BOOLEAN Junio C Hamano
2011-09-27 23:59 ` [PATCH] archive.c: use OPT_BOOL() Junio C Hamano
2011-09-28 3:58 ` [PATCH] parse-options: deprecate OPT_BOOLEAN Jeff King
2011-09-28 4:22 ` Junio C Hamano [this message]
2011-09-28 9:30 ` Miles Bader
2011-09-28 17:44 ` [PATCH 1/3] parseopt: add OPT_NOOP_NOARG René Scharfe
2011-09-28 17:47 ` [PATCH 2/3] revert: use OPT_NOOP_NOARG René Scharfe
2011-09-28 17:47 ` [PATCH 3/3] apply: " René Scharfe
2011-09-28 19:47 ` [PATCH 1/3] parseopt: add OPT_NOOP_NOARG Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vbou5s3c0.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=madcoder@debian.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.