public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Pierre Habouzit <madcoder@debian•org>
To: gitster@pobox•com
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, Pierre Habouzit <madcoder@debian•org>
Subject: [PATCH 1/4] parse-options: abbreviation engine fix.
Date: Mon,  5 Nov 2007 13:03:21 +0100	[thread overview]
Message-ID: <1194264204-3475-2-git-send-email-madcoder@debian.org> (raw)
In-Reply-To: <1194264204-3475-1-git-send-email-madcoder@debian.org>

When we had at least two long option then followed by another one that was a
prefix of both of them, then the abbreviation detector failed.

Fix the issue, add a test.

Signed-off-by: Pierre Habouzit <madcoder@debian•org>
---
 parse-options.c          |   48 +++++++++++++++++++++++-----------------------
 t/t0040-parse-options.sh |   13 ++++++++++++
 test-parse-options.c     |    1 +
 3 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index cc09c98..d2e32c1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -119,8 +119,8 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
                           const struct option *options)
 {
 	const char *arg_end = strchr(arg, '=');
-	const struct option *abbrev_option = NULL;
-	int abbrev_flags = 0;
+	const struct option *abbrev_option = NULL, *conflict_option = NULL;
+	int abbrev_flags = 0, conflict_flags = 0;
 
 	if (!arg_end)
 		arg_end = arg + strlen(arg);
@@ -132,42 +132,33 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
 		if (!options->long_name)
 			continue;
 
+		/* special case {n,no,no-} that always conflict */
+		if (!prefixcmp("no-", arg))
+			die("`--{n,no,no-}' cannot be abbreviated forms of options");
+
 		rest = skip_prefix(arg, options->long_name);
 		if (!rest) {
 			/* abbreviated? */
 			if (!strncmp(options->long_name, arg, arg_end - arg)) {
 is_abbreviated:
-				if (abbrev_option)
-					return error("Ambiguous option: %s "
-						"(could be --%s%s or --%s%s)",
-						arg,
-						(flags & OPT_UNSET) ?
-							"no-" : "",
-						options->long_name,
-						(abbrev_flags & OPT_UNSET) ?
-							"no-" : "",
-						abbrev_option->long_name);
-				if (!(flags & OPT_UNSET) && *arg_end)
-					p->opt = arg_end + 1;
+				conflict_option = abbrev_option;
+				conflict_flags = abbrev_flags;
 				abbrev_option = options;
 				abbrev_flags = flags;
 				continue;
 			}
-			/* negated and abbreviated very much? */
-			if (!prefixcmp("no-", arg)) {
-				flags |= OPT_UNSET;
-				goto is_abbreviated;
-			}
 			/* negated? */
 			if (strncmp(arg, "no-", 3))
 				continue;
 			flags |= OPT_UNSET;
-			rest = skip_prefix(arg + 3, options->long_name);
+			arg += 3;
+			rest = skip_prefix(arg, options->long_name);
 			/* abbreviated and negated? */
-			if (!rest && !prefixcmp(options->long_name, arg + 3))
-				goto is_abbreviated;
-			if (!rest)
+			if (!rest) {
+				if (!strncmp(options->long_name, arg, arg_end - arg))
+					goto is_abbreviated;
 				continue;
+			}
 		}
 		if (*rest) {
 			if (*rest != '=')
@@ -176,8 +167,17 @@ is_abbreviated:
 		}
 		return get_value(p, options, flags);
 	}
-	if (abbrev_option)
+	if (conflict_option)
+		return error("Ambiguous option: %s (could be --%s%s or --%s%s)",
+			arg, (conflict_flags & OPT_UNSET) ?  "no-" : "",
+			conflict_option->long_name,
+			(abbrev_flags & OPT_UNSET) ?  "no-" : "",
+			abbrev_option->long_name);
+	if (abbrev_option) {
+		if (!(abbrev_flags & OPT_UNSET) && *arg_end)
+			p->opt = arg_end + 1;
 		return get_value(p, abbrev_option, abbrev_flags);
+	}
 	return error("unknown option `%s'", arg);
 }
 
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ae49424..ee758e5 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -18,6 +18,7 @@ string options
     -s, --string <string>
                           get a string
     --string2 <str>       get another string
+    --st <st>             get another string (pervert ordering)
 
 EOF
 
@@ -90,4 +91,16 @@ test_expect_failure 'ambiguously abbreviated option' '
         test $? != 129
 '
 
+cat > expect << EOF
+boolean: 0
+integer: 2
+string: 123
+EOF
+
+test_expect_failure 'non ambiguous option (after two options it abbreviates)' '
+	test-parse-options --st 123 &&
+	test ! -s output.err &&
+	git diff expect output
+'
+
 test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 277cfe4..4d3e2ec 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -18,6 +18,7 @@ int main(int argc, const char **argv)
 		OPT_GROUP("string options"),
 		OPT_STRING('s', "string", &string, "string", "get a string"),
 		OPT_STRING(0, "string2", &string, "str", "get another string"),
+		OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
 		OPT_END(),
 	};
 	int i;
-- 
1.5.3.5.1531.g59008


  reply	other threads:[~2007-11-05 12:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-05 12:03 proposal for an OPTION_SUBARRAY (recursive parser) Pierre Habouzit
2007-11-05 12:03 ` Pierre Habouzit [this message]
2007-11-05 12:03   ` [PATCH 2/4] Some better parse-options documentation Pierre Habouzit
2007-11-05 12:03     ` [PATCH 3/4] Add OPTION_BASEOFFSET/OPTION_SUBARRAY Pierre Habouzit
2007-11-05 12:03       ` [PATCH 4/4] Implement OPTION_SUBARRAY handling Pierre Habouzit
2007-11-05 12:34   ` [PATCH] parse-options: abbreviation engine fix Johannes Schindelin
2007-11-05 12:38     ` Johannes Schindelin
2007-11-05 13:15       ` [PATCH 1/3] " Johannes Schindelin
2007-11-05 13:15       ` [PATCH 2/3] parseopt: introduce OPT_RECURSE to specify shared options Johannes Schindelin
2007-11-05 13:46         ` Johannes Schindelin
2007-11-05 16:15           ` Linus Torvalds
2007-11-05 16:29             ` Johannes Schindelin
2007-11-05 16:53               ` Pierre Habouzit
2007-11-05 21:48           ` Junio C Hamano
2007-11-05 22:14             ` Pierre Habouzit
2007-11-05 13:15       ` [PATCH 3/3] parseopt: do not list options with the same name twice Johannes Schindelin
2007-11-05 12:59     ` [PATCH] parse-options: abbreviation engine fix Pierre Habouzit

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=1194264204-3475-2-git-send-email-madcoder@debian.org \
    --to=madcoder@debian$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox