From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] parse-options: warn developers on negated options
Date: Wed, 06 Sep 2017 10:52:46 +0900 [thread overview]
Message-ID: <xmqq8thsa901.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170905230845.17108-1-sbeller@google.com> (Stefan Beller's message of "Tue, 5 Sep 2017 16:08:45 -0700")
Stefan Beller <sbeller@google.com> writes:
> This patch disallows all no- options, but we could be more open and allow
> --no-options that have the NO_NEG bit set.
"--no-foo" that does not take "--foo" is perhaps OK so should not
trigger an error.
A ("--no-foo", "--foo") pair is better spelled as ("--foo",
"--no-foo") pair whose default is "--foo", but making it an error is
probably a bit too much.
Compared to that, ("--no-foo", "--no-no-foo") pair feels nonsense.
Having said that, because the existing parse_options_check() is all
about catching the programming mistake (the end user cannot fix an
error from it by tweaking the command line option s/he gives to the
program), I do not think a conditional compilation like you added
mixes well. Either make the whole thing, not just your new test,
conditional to -DDEVELOPER (which would make it possible for you to
build and ship a binary with broken options[] array to the end-users
that does not die in this function), which is undesirable, or add a
new test that catches a definite error unconditionally.
next prev parent reply other threads:[~2017-09-06 1:52 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-05 21:01 [PATCH] builtin/merge: honor commit-msg hook for merges Stefan Beller
2017-09-05 21:38 ` Junio C Hamano
2017-09-05 23:08 ` [PATCH] parse-options: warn developers on negated options Stefan Beller
2017-09-06 1:52 ` Junio C Hamano [this message]
2017-09-06 3:16 ` Junio C Hamano
2017-09-06 21:36 ` Stefan Beller
2017-09-06 23:41 ` Junio C Hamano
2017-09-05 23:29 ` [PATCHv2] builtin/merge: honor commit-msg hook for merges Stefan Beller
2017-09-06 1:57 ` Junio C Hamano
2017-09-06 22:11 ` Stefan Beller
2017-09-06 23:43 ` Junio C Hamano
2017-09-07 22:04 ` [PATCHv3] " Stefan Beller
2017-09-08 1:13 ` Junio C Hamano
2017-09-11 17:12 ` Stefan Beller
2017-09-16 6:22 ` Kaartic Sivaraam
2017-09-20 19:55 ` Stefan Beller
2017-09-21 1:10 ` Junio C Hamano
2017-09-21 20:29 ` [PATCH] Documentation/githooks: mention merge in commit-msg hook Stefan Beller
2017-09-22 1:58 ` 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=xmqq8thsa901.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=sbeller@google.com \
/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.