git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Pierre Habouzit <madcoder@debian.org>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: [PATCH] parse-options: abbreviation engine fix.
Date: Mon, 5 Nov 2007 12:34:00 +0000 (GMT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0711051230020.4362@racer.site> (raw)
In-Reply-To: <1194264204-3475-2-git-send-email-madcoder@debian.org>


When an option could be an ambiguous abbreviation of two options, the code 
used to error out.  Even if an exact match would have occured later.

Test and original patch by Pierre Habouzit.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	On Mon, 5 Nov 2007, Pierre Habouzit wrote:

	> 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.

	Yeah, I assumed that you would never do such a thing, but I agree 
	that with recursing option parsing it is much more likely.

	It took me some time to understand your patch, and that the moving 
	of the UNSET handling was unnecessary.

	IMHO this patch is easier to read.

 parse-options.c          |   33 +++++++++++++++++++++------------
 t/t0040-parse-options.sh |   13 +++++++++++++
 test-parse-options.c     |    1 +
 3 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index cc09c98..15b32f7 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, *ambiguous_option = NULL;
+	int abbrev_flags = 0, ambiguous_flags = 0;
 
 	if (!arg_end)
 		arg_end = arg + strlen(arg);
@@ -137,16 +137,16 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
 			/* 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 (abbrev_option) {
+					/*
+					 * If this is abbreviated, it is
+					 * ambiguous. So when there is no
+					 * exact match later, we need to
+					 * error out.
+					 */
+					ambiguous_option = abbrev_option;
+					ambiguous_flags = abbrev_flags;
+				}
 				if (!(flags & OPT_UNSET) && *arg_end)
 					p->opt = arg_end + 1;
 				abbrev_option = options;
@@ -176,6 +176,15 @@ is_abbreviated:
 		}
 		return get_value(p, options, flags);
 	}
+
+	if (ambiguous_option)
+		return error("Ambiguous option: %s "
+			"(could be --%s%s or --%s%s)",
+			arg,
+			(ambiguous_flags & OPT_UNSET) ?  "no-" : "",
+			ambiguous_option->long_name,
+			(abbrev_flags & OPT_UNSET) ?  "no-" : "",
+			abbrev_option->long_name);
 	if (abbrev_option)
 		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.1549.g91a3

  parent reply	other threads:[~2007-11-05 12:35 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 ` [PATCH 1/4] parse-options: abbreviation engine fix Pierre Habouzit
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   ` Johannes Schindelin [this message]
2007-11-05 12:38     ` [PATCH] parse-options: abbreviation engine fix 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=Pine.LNX.4.64.0711051230020.4362@racer.site \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=madcoder@debian.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).