Subject: Re: [PATCH] parse-options: detect attempt to add a duplicate short option name

From: René Scharfe <[email-masked]>
Date: Thu, 04 Sep 2014 08:13:09 +0200

Am 04.09.2014 um 00:16 schrieb Junio C Hamano:
> René Scharfe <[email-masked]> writes:
> 
>>> It bugged me enough that we didn't identify which short option
>>> letter we were complaining about
>>
>> The old code did report the short option.  E.g. for t1502 it said:
>>
>> 	error: BUG: switch 'b' short name already used
>>
>> You can leave that to optbug(), no need for the strbuf.
> 
> Not quite, as an opt with long name is reported with the long name
> only, which is not very nice when the problem we are reporting is
> about its short variant.

Perhaps something like the patch below helps, here and in general?

>> Space is allowed as a short option by the code; intentionally?
> 
> I didn't think of a strong reason to declare either way, so, yes it
> was deliberate that I didn't tighten to disallow.

OK.  I don't think it's easy to come up with a usable way for having
space as a short option, but maybe it's possible.

---
 parse-options.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/parse-options.c b/parse-options.c
index b7925c5..f1c0b5d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -14,8 +14,12 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,
 
 int optbug(const struct option *opt, const char *reason)
 {
-	if (opt->long_name)
+	if (opt->long_name) {
+		if (opt->short_name)
+			return error("BUG: switch '%c' (--%s) %s",
+				     opt->short_name, opt->long_name, reason);
 		return error("BUG: option '%s' %s", opt->long_name, reason);
+	}
 	return error("BUG: switch '%c' %s", opt->short_name, reason);
 }
 
-- 
2.1.0