[PATCH 1/2] do not mangle short options which take arguments

[Clemens Buchacher <drizzd@aon.at>]

Instead of

  $ git commit -a -ammend
  [work ce38944] mend
   1 files changed, 2 insertions(+), 0 deletions(-)

we now get

  $ git commit -a -ammend
  error: switch `m' must not be mangled with other options
  usage: git commit [options] [--] <filepattern>...
  [...]

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
On Fri, Sep 25, 2009 at 04:32:26PM -0700, Shawn O. Pearce wrote:
> I wonder, should the -m flag on commit not allow cuddling its
> value against the switch when its combined in short form with
> other switches?

Here we go.

Clemens

 parse-options.c            |   16 ++++++++++++----
 t/t0040-parse-options.sh   |   12 ++++++++++++
 t/t3701-add-interactive.sh |    2 +-
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index a64a4d6..4f16f37 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -5,6 +5,7 @@
 
 #define OPT_SHORT 1
 #define OPT_UNSET 2
+#define OPT_MANY 4
 
 static int opterror(const struct option *opt, const char *reason, int flags)
 {
@@ -43,9 +44,12 @@ static int get_value(struct parse_opt_ctx_t *p,
 		     const struct option *opt, int flags)
 {
 	const char *s, *arg;
+	const int many = flags & OPT_MANY;
 	const int unset = flags & OPT_UNSET;
 	int err;
 
+	if (many && !(opt->flags & PARSE_OPT_NOARG))
+		return opterror(opt, "must not be mangled with other options", flags);
 	if (unset && p->opt)
 		return opterror(opt, "takes no value", flags);
 	if (unset && (opt->flags & PARSE_OPT_NONEG))
@@ -149,14 +153,18 @@ static int get_value(struct parse_opt_ctx_t *p,
 	}
 }
 
-static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
+static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option
+		*options, int many)
 {
 	const struct option *numopt = NULL;
+	int flags = OPT_SHORT;
+	if (many)
+		flags |= OPT_MANY;
 
 	for (; options->type != OPTION_END; options++) {
 		if (options->short_name == *p->opt) {
 			p->opt = p->opt[1] ? p->opt + 1 : NULL;
-			return get_value(p, options, OPT_SHORT);
+			return get_value(p, options, flags);
 		}
 
 		/*
@@ -374,7 +382,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			ctx->opt = arg + 1;
 			if (internal_help && *ctx->opt == 'h')
 				return parse_options_usage(usagestr, options);
-			switch (parse_short_opt(ctx, options)) {
+			switch (parse_short_opt(ctx, options, 0)) {
 			case -1:
 				return parse_options_usage(usagestr, options);
 			case -2:
@@ -385,7 +393,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			while (ctx->opt) {
 				if (internal_help && *ctx->opt == 'h')
 					return parse_options_usage(usagestr, options);
-				switch (parse_short_opt(ctx, options)) {
+				switch (parse_short_opt(ctx, options, 1)) {
 				case -1:
 					return parse_options_usage(usagestr, options);
 				case -2:
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index bbc821e..86eb350 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -315,4 +315,16 @@ test_expect_success 'OPT_NUMBER_CALLBACK() works' '
 	test_cmp expect output
 '
 
+cat > mangle.err << EOF
+error: switch \`s' must not be mangled with other options
+EOF
+
+cat mangle.err expect.err > expect-mangle.err
+
+test_expect_success 'do not mangle options which require arguments' '
+	test_must_fail test-parse-options -bs123 > output 2> output.err &&
+	! test -s output &&
+	test_cmp expect-mangle.err output.err
+'
+
 test_done
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 62fd65e..208a134 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -207,7 +207,7 @@ index b6f2c08..61b9053 100755
 EOF
 # Test splitting the first patch, then adding both
 test_expect_success 'add first line works' '
-	git commit -am "clear local changes" &&
+	git commit -a -m "clear local changes" &&
 	git apply patch &&
 	(echo s; echo y; echo y) | git add -p file &&
 	git diff --cached > diff &&
-- 
1.6.5.rc1.214.g13c5a