Am 19.04.2017 um 11:08 schrieb Jacob Keller:
From: Jacob Keller <jacob.keller@xxxxxxxxx>
Many options can be negated by prefixing the option with "no-", for
example "--3way" can be prefixed with "--no-3way" to disable it. Since
0f1930c58754 ("parse-options: allow positivation of options
starting, with no-", 2012-02-25) we have also had support to negate
options which start with "no-" by using the positive wording.
This leads to the confusing (and non-documented) case that you can still
prefix options beginning with "no-" by a second "no-" to negate them.
That is, we allow "no-no-hardlinks" to negate the "no-hardlinks" option.
This can be confusing to the user so lets just disallow the
double-negative forms. If the long_name begins with "no-" then we simply
don't allow the regular negation format, and only allow the option to be
negated by the positive form.
Your patch is a modernized version of my old one, so I'm fine with it.
But I wonder how --no-no-x being treated the same as --x can be
confusing. https://en.wikipedia.org/wiki/Double_negative explains it, I
think -- in some languages and dialects multiple negatives increase
negativity instead of cancelling themselves out pairwise. So users
would expect to get no x with --no-x and even less of it with --no-no-x?
Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx>
---
I started going about implementing an OPT_NEGBOOL as suggested by Peff,
but realized this might just be simpler, and we already support the
positive format for the negation, so we don't lose expressiveness. We
*might* want to tie this to an option flag instead so that it only kicks
in if the option specifically requests it. Thoughts?
Do you mean that there should be a flag for allowing double negation?
In which situation would it be useful?
Or do you mean that negation should be disabled by default and would
have to be enabled explicitly, unlike the current situation where it is
enabled by default and can be turned off with PARSE_OPT_NONEG? That
depends on how often we'd want to disable negation, I guess. For
boolean flags it probably makes sense to allow it by default.
parse-options.c | 3 +++
t/t0040-parse-options.sh | 5 ++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/parse-options.c b/parse-options.c
index a23a1e67f04f..8e024c569f52 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -299,6 +299,9 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
}
continue;
}
+ /* avoid double-negate on long_name */
+ if (starts_with(long_name, "no-"))
+ continue;
flags |= OPT_UNSET;
if (!skip_prefix(arg + 3, long_name, &rest)) {
/* abbreviated and negated? */
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 74d2cd76fe56..abccfa5f265f 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -88,7 +88,6 @@ test_expect_success 'OPT_BOOL() is idempotent #1' 'check boolean: 1 --yes --yes'
test_expect_success 'OPT_BOOL() is idempotent #2' 'check boolean: 1 -DB'
test_expect_success 'OPT_BOOL() negation #1' 'check boolean: 0 -D --no-yes'
-test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'
test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown_i18n --fear'
test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown_i18n --no-no-fear'
@@ -392,4 +391,8 @@ test_expect_success '--no-verbose resets multiple verbose to 0' '
test-parse-options --expect="verbose: 0" -v -v -v --no-verbose
'
+test_expect_success 'double negation not accepted' '
+ test_must_fail test-parse-options --expect="boolean: 0" --no-no-doubt
+'
+
test_done
Using check_unknown_i18n like in the test for --no-no-fear would be
shorter and more consistent.
René