{"thread":{"id":"21052","subject":"'git branch --no-merge' is ambiguous","startedAt":"2009-09-25T10:28:57Z","lastAt":"2009-09-27T08:01:18Z","messageCount":4,"participants":["Johannes Sixt","Andreas Schwab","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"123765","messageId":"4ABC9B69.5010205@viscovery.net","threadId":"21052","inReplyTo":null,"subject":"'git branch --no-merge' is ambiguous","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-09-25T10:28:57Z","receivedAt":"2009-09-25T10:28:57Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Look here:\n\n $ git branch --merge\n * master\n $ git branch --no-merge\n error: Ambiguous option: no-merge (could be --no-merged or --no-merged)\n usage: ...\n     --no-merged <commit>  print only not merged branches\n     --merged <commit>     print only merged branches\n\nI tried to debug it, but parse_long_opt() is such awful spaghetti code\nthat I don't grok it. Please help.\n\n-- Hannes\n"},{"id":"123794","messageId":"m2ljk250f7.fsf@igel.home","threadId":"21052","inReplyTo":"4ABC9B69.5010205@viscovery.net","subject":"Re: 'git branch --no-merge' is ambiguous","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2009-09-25T18:44:44Z","receivedAt":"2009-09-25T18:44:44Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Look here:\n>\n>  $ git branch --merge\n>  * master\n>  $ git branch --no-merge\n>  error: Ambiguous option: no-merge (could be --no-merged or --no-merged)\n>  usage: ...\n>      --no-merged <commit>  print only not merged branches\n>      --merged <commit>     print only merged branches\n>\n> I tried to debug it, but parse_long_opt() is such awful spaghetti code\n> that I don't grok it. Please help.\n\nparse_long_opt always matches both --opt and --no-opt for any option\n\"opt\", and only get_value checks whether --no-opt is actually valid.\nSince the options for git branch contains both \"no-merged\" and \"merged\"\nthere are two matches for --no-merge, but no exact match.  With this\npatch the negation of a NONEG option is rejected earlier, but it changes\nthe error message from \"option `no-opt' isn't available\" to \"unknown\noption `no-opt'\".\n\ndiff --git a/parse-options.c b/parse-options.c\nindex a64a4d6..f559411 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -230,6 +230,9 @@ is_abbreviated:\n \t\t\t\tabbrev_flags = flags;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\t/* negation allowed? */\n+\t\t\tif (options->flags & PARSE_OPT_NONEG)\n+\t\t\t\tcontinue;\n \t\t\t/* negated and abbreviated very much? */\n \t\t\tif (!prefixcmp(\"no-\", arg)) {\n \t\t\t\tflags |= OPT_UNSET;\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"123884","messageId":"20090927073305.GA15393@coredump.intra.peff.net","threadId":"21052","inReplyTo":"m2ljk250f7.fsf@igel.home","subject":"Re: 'git branch --no-merge' is ambiguous","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-27T07:33:06Z","receivedAt":"2009-09-27T07:33:06Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 25, 2009 at 08:44:44PM +0200, Andreas Schwab wrote:\n\n> parse_long_opt always matches both --opt and --no-opt for any option\n> \"opt\", and only get_value checks whether --no-opt is actually valid.\n> Since the options for git branch contains both \"no-merged\" and \"merged\"\n> there are two matches for --no-merge, but no exact match.  With this\n> patch the negation of a NONEG option is rejected earlier, but it changes\n> the error message from \"option `no-opt' isn't available\" to \"unknown\n> option `no-opt'\".\n\nThanks. Reading through the code, I came to the same conclusion: we\nshouldn't be looking at --no-* at all if we are NONEG. I think the\nchange in error message is acceptable.\n\nIt is a little bit annoying that builtin-branch needs to have this as\ntwo separate options in the first place. But it wants to be able to do\n--no-merged with an argument, which is not currently possible with just\na negation of --merged. I don't know if it is worth adding an\nOPT_NEGARG.\n\nBelow is what I'm going to commit.  Can I get your Signed-off-by?\n\n-- >8 --\nFrom: Andreas Schwab <schwab@linux-m68k.org>\nSubject: [PATCH] parse-opt: ignore negation of OPT_NONEG for ambiguity checks\n\nparse_long_opt always matches both --opt and --no-opt for any option\n\"opt\", and only get_value checks whether --no-opt is actually valid.\nSince the options for git branch contains both \"no-merged\" and \"merged\"\nthere are two matches for --no-merge, but no exact match.  With this\npatch the negation of a NONEG option is rejected earlier, but it changes\nthe error message from \"option `no-opt' isn't available\" to \"unknown\noption `no-opt'\".\n\n[jk: added test]\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n parse-options.c          |    3 +++\n t/t0040-parse-options.sh |   20 ++++++++++++++++++++\n test-parse-options.c     |    5 +++++\n 3 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex a64a4d6..f559411 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -230,6 +230,9 @@ is_abbreviated:\n \t\t\t\tabbrev_flags = flags;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\t/* negation allowed? */\n+\t\t\tif (options->flags & PARSE_OPT_NONEG)\n+\t\t\t\tcontinue;\n \t\t\t/* negated and abbreviated very much? */\n \t\t\tif (!prefixcmp(\"no-\", arg)) {\n \t\t\t\tflags |= OPT_UNSET;\ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex bbc821e..3d450ed 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -33,6 +33,8 @@ Magic arguments\n     --quux                means --quux\n     -NUM                  set integer to NUM\n     +                     same as -b\n+    --ambiguous           positive ambiguity\n+    --no-ambiguous        negative ambiguity\n \n Standard options\n     --abbrev[=<n>]        use <n> digits to display SHA-1s\n@@ -315,4 +317,22 @@ test_expect_success 'OPT_NUMBER_CALLBACK() works' '\n \ttest_cmp expect output\n '\n \n+cat >expect <<EOF\n+boolean: 0\n+integer: 0\n+timestamp: 0\n+string: (not set)\n+abbrev: 7\n+verbose: 0\n+quiet: no\n+dry run: no\n+file: (not set)\n+EOF\n+\n+test_expect_success 'negation of OPT_NONEG flags is not ambiguous' '\n+\ttest-parse-options --no-ambig >output 2>output.err &&\n+\ttest ! -s output.err &&\n+\ttest_cmp expect output\n+'\n+\n test_done\ndiff --git a/test-parse-options.c b/test-parse-options.c\nindex efa734b..acd1a2b 100644\n--- a/test-parse-options.c\n+++ b/test-parse-options.c\n@@ -8,6 +8,7 @@ static int abbrev = 7;\n static int verbose = 0, dry_run = 0, quiet = 0;\n static char *string = NULL;\n static char *file = NULL;\n+static int ambiguous;\n \n static int length_callback(const struct option *opt, const char *arg, int unset)\n {\n@@ -59,6 +60,10 @@ int main(int argc, const char **argv)\n \t\t\tnumber_callback),\n \t\t{ OPTION_BOOLEAN, '+', NULL, &boolean, NULL, \"same as -b\",\n \t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH },\n+\t\t{ OPTION_BOOLEAN, 0, \"ambiguous\", &ambiguous, NULL,\n+\t\t  \"positive ambiguity\", PARSE_OPT_NOARG | PARSE_OPT_NONEG },\n+\t\t{ OPTION_BOOLEAN, 0, \"no-ambiguous\", &ambiguous, NULL,\n+\t\t  \"negative ambiguity\", PARSE_OPT_NOARG | PARSE_OPT_NONEG },\n \t\tOPT_GROUP(\"Standard options\"),\n \t\tOPT__ABBREV(&abbrev),\n \t\tOPT__VERBOSE(&verbose),\n-- \n1.6.5.rc2.197.g25cf3\n"},{"id":"123887","messageId":"m24oqo3jg1.fsf@whitebox.home","threadId":"21052","inReplyTo":"20090927073305.GA15393@coredump.intra.peff.net","subject":"Re: 'git branch --no-merge' is ambiguous","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2009-09-27T08:01:18Z","receivedAt":"2009-09-27T08:01:18Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> -- >8 --\n> From: Andreas Schwab <schwab@linux-m68k.org>\n> Subject: [PATCH] parse-opt: ignore negation of OPT_NONEG for ambiguity checks\n>\n> parse_long_opt always matches both --opt and --no-opt for any option\n> \"opt\", and only get_value checks whether --no-opt is actually valid.\n> Since the options for git branch contains both \"no-merged\" and \"merged\"\n> there are two matches for --no-merge, but no exact match.  With this\n> patch the negation of a NONEG option is rejected earlier, but it changes\n> the error message from \"option `no-opt' isn't available\" to \"unknown\n> option `no-opt'\".\n>\n> [jk: added test]\n>\n> Signed-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Andreas Schwab <schwab@linux-m68k.org>\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"}]}