All of lore.kernel.org
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johan Herland <johan@herland.net>,
	git@vger.kernel.org, Pierre Habouzit <madcoder@debian.org>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH] parse-options: detect attempt to add a duplicate short option name
Date: Thu, 04 Sep 2014 08:13:09 +0200	[thread overview]
Message-ID: <540802F5.1070708@web.de> (raw)
In-Reply-To: <xmqqbnqwwds2.fsf@gitster.dls.corp.google.com>

Am 04.09.2014 um 00:16 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> 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

  reply	other threads:[~2014-09-04  6:13 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-03 14:03 [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option Johan Herland
2014-09-03 14:03 ` [RFC/PATCH 1/3] t7503/4: Add failing testcases for revert/cherry-pick --no-verify Johan Herland
2014-09-03 19:28   ` Junio C Hamano
2014-09-03 14:03 ` [RFC/PATCH 2/3] revert/cherry-pick: Add --no-verify option, and pass it on to commit Johan Herland
2014-09-03 19:32   ` Junio C Hamano
2014-09-03 19:42     ` [PATCH] parse-options: detect attempt to add a duplicate short option name Junio C Hamano
2014-09-03 20:29       ` René Scharfe
2014-09-03 21:05         ` Junio C Hamano
2014-09-03 21:46           ` René Scharfe
2014-09-03 22:16             ` Junio C Hamano
2014-09-04  6:13               ` René Scharfe [this message]
2014-09-04 17:24                 ` Junio C Hamano
2014-09-04 18:07                   ` Junio C Hamano
2014-09-03 21:46           ` Jonathan Nieder
2014-09-03 21:58             ` Jonathan Nieder
2014-09-04  8:34     ` [RFC/PATCH 2/3] revert/cherry-pick: Add --no-verify option, and pass it on to commit Johan Herland
2014-09-03 14:03 ` [RFC/PATCH 3/3] revert/cherry-pick --no-verify: Update documentation Johan Herland
2014-09-03 19:21 ` [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option Junio C Hamano
2014-09-05 21:05 ` Fabian Ruch
2014-09-08 15:13   ` Johan Herland

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=540802F5.1070708@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johan@herland.net \
    --cc=jrnieder@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.