# 'git branch --no-merge' is ambiguous 4 messages from 2009-09-25 to 2009-09-27. Participants: Johannes Sixt, Andreas Schwab, Jeff King. Thread: https://gitlist.dev/t/21052 ## Johannes Sixt, 2009-09-25 10:28 Subject: 'git branch --no-merge' is ambiguous Message-ID: <4ABC9B69.5010205@viscovery.net> URL: https://gitlist.dev/e/4ABC9B69.5010205%40viscovery.net ``` Look here: $ git branch --merge * master $ git branch --no-merge error: Ambiguous option: no-merge (could be --no-merged or --no-merged) usage: ... --no-merged print only not merged branches --merged print only merged branches I tried to debug it, but parse_long_opt() is such awful spaghetti code that I don't grok it. Please help. -- Hannes ``` ## Andreas Schwab, 2009-09-25 18:44 Subject: Re: 'git branch --no-merge' is ambiguous Message-ID: URL: https://gitlist.dev/e/m2ljk250f7.fsf%40igel.home In-Reply-To: <4ABC9B69.5010205@viscovery.net> ``` Johannes Sixt writes: > Look here: > > $ git branch --merge > * master > $ git branch --no-merge > error: Ambiguous option: no-merge (could be --no-merged or --no-merged) > usage: ... > --no-merged print only not merged branches > --merged print only merged branches > > I tried to debug it, but parse_long_opt() is such awful spaghetti code > that I don't grok it. Please help. parse_long_opt always matches both --opt and --no-opt for any option "opt", and only get_value checks whether --no-opt is actually valid. Since the options for git branch contains both "no-merged" and "merged" there are two matches for --no-merge, but no exact match. With this patch the negation of a NONEG option is rejected earlier, but it changes the error message from "option `no-opt' isn't available" to "unknown option `no-opt'". diff --git a/parse-options.c b/parse-options.c index a64a4d6..f559411 100644 --- a/parse-options.c +++ b/parse-options.c @@ -230,6 +230,9 @@ is_abbreviated: abbrev_flags = flags; continue; } + /* negation allowed? */ + if (options->flags & PARSE_OPT_NONEG) + continue; /* negated and abbreviated very much? */ if (!prefixcmp("no-", arg)) { flags |= OPT_UNSET; Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ``` ## Jeff King, 2009-09-27 07:33 Subject: Re: 'git branch --no-merge' is ambiguous Message-ID: <20090927073305.GA15393@coredump.intra.peff.net> URL: https://gitlist.dev/e/20090927073305.GA15393%40coredump.intra.peff.net In-Reply-To: ``` On Fri, Sep 25, 2009 at 08:44:44PM +0200, Andreas Schwab wrote: > parse_long_opt always matches both --opt and --no-opt for any option > "opt", and only get_value checks whether --no-opt is actually valid. > Since the options for git branch contains both "no-merged" and "merged" > there are two matches for --no-merge, but no exact match. With this > patch the negation of a NONEG option is rejected earlier, but it changes > the error message from "option `no-opt' isn't available" to "unknown > option `no-opt'". Thanks. Reading through the code, I came to the same conclusion: we shouldn't be looking at --no-* at all if we are NONEG. I think the change in error message is acceptable. It is a little bit annoying that builtin-branch needs to have this as two separate options in the first place. But it wants to be able to do --no-merged with an argument, which is not currently possible with just a negation of --merged. I don't know if it is worth adding an OPT_NEGARG. Below is what I'm going to commit. Can I get your Signed-off-by? -- >8 -- From: Andreas Schwab Subject: [PATCH] parse-opt: ignore negation of OPT_NONEG for ambiguity checks parse_long_opt always matches both --opt and --no-opt for any option "opt", and only get_value checks whether --no-opt is actually valid. Since the options for git branch contains both "no-merged" and "merged" there are two matches for --no-merge, but no exact match. With this patch the negation of a NONEG option is rejected earlier, but it changes the error message from "option `no-opt' isn't available" to "unknown option `no-opt'". [jk: added test] Signed-off-by: Jeff King --- parse-options.c | 3 +++ t/t0040-parse-options.sh | 20 ++++++++++++++++++++ test-parse-options.c | 5 +++++ 3 files changed, 28 insertions(+), 0 deletions(-) diff --git a/parse-options.c b/parse-options.c index a64a4d6..f559411 100644 --- a/parse-options.c +++ b/parse-options.c @@ -230,6 +230,9 @@ is_abbreviated: abbrev_flags = flags; continue; } + /* negation allowed? */ + if (options->flags & PARSE_OPT_NONEG) + continue; /* negated and abbreviated very much? */ if (!prefixcmp("no-", arg)) { flags |= OPT_UNSET; diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index bbc821e..3d450ed 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -33,6 +33,8 @@ Magic arguments --quux means --quux -NUM set integer to NUM + same as -b + --ambiguous positive ambiguity + --no-ambiguous negative ambiguity Standard options --abbrev[=] use digits to display SHA-1s @@ -315,4 +317,22 @@ test_expect_success 'OPT_NUMBER_CALLBACK() works' ' test_cmp expect output ' +cat >expect <output 2>output.err && + test ! -s output.err && + test_cmp expect output +' + test_done diff --git a/test-parse-options.c b/test-parse-options.c index efa734b..acd1a2b 100644 --- a/test-parse-options.c +++ b/test-parse-options.c @@ -8,6 +8,7 @@ static int abbrev = 7; static int verbose = 0, dry_run = 0, quiet = 0; static char *string = NULL; static char *file = NULL; +static int ambiguous; static int length_callback(const struct option *opt, const char *arg, int unset) { @@ -59,6 +60,10 @@ int main(int argc, const char **argv) number_callback), { OPTION_BOOLEAN, '+', NULL, &boolean, NULL, "same as -b", PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH }, + { OPTION_BOOLEAN, 0, "ambiguous", &ambiguous, NULL, + "positive ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG }, + { OPTION_BOOLEAN, 0, "no-ambiguous", &ambiguous, NULL, + "negative ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG }, OPT_GROUP("Standard options"), OPT__ABBREV(&abbrev), OPT__VERBOSE(&verbose), -- 1.6.5.rc2.197.g25cf3 ``` ## Andreas Schwab, 2009-09-27 08:01 Subject: Re: 'git branch --no-merge' is ambiguous Message-ID: URL: https://gitlist.dev/e/m24oqo3jg1.fsf%40whitebox.home In-Reply-To: <20090927073305.GA15393@coredump.intra.peff.net> ``` Jeff King writes: > -- >8 -- > From: Andreas Schwab > Subject: [PATCH] parse-opt: ignore negation of OPT_NONEG for ambiguity checks > > parse_long_opt always matches both --opt and --no-opt for any option > "opt", and only get_value checks whether --no-opt is actually valid. > Since the options for git branch contains both "no-merged" and "merged" > there are two matches for --no-merge, but no exact match. With this > patch the negation of a NONEG option is rejected earlier, but it changes > the error message from "option `no-opt' isn't available" to "unknown > option `no-opt'". > > [jk: added test] > > Signed-off-by: Jeff King Signed-off-by: Andreas Schwab Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ```