On Tue, Dec 13, 2016 at 09:48:58PM +1300, Chris Packham wrote: > + if (continue_current_merge) { > + int nargc = 1; > + const char *nargv[] = {"commit", NULL}; > + > + if (argc) > + usage_msg_opt("--continue expects no arguments", > + builtin_merge_usage, builtin_merge_options);
This checks that we don't have: git merge --continue foobar but still allows: git merge --continue --some-option because parse_options() decrements argc. It would be insane to check individually which options might have been set. But I wonder if we could do something like: int orig_argc = argc; ... argc = parse_options(argc, argv, ...); if (continue_current_merge) { if (orig_argc != 1) /* maybe 2, to account for argv[0] ? */ usage_msg_opt("--continue expects no arguments", ...); } That gets trickier if there ever is an option that's OK to use with --continue. We might want to forward along "--quiet", for example. On the other hand, we silently ignore it now, so maybe it is better to complain and then let --quiet get added later if somebody cares. Whatever we do here, I think "--abort" should get the same treatment (probably as a separate patch). > diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh > index 85248a14b..44b34ef3a 100755 > --- a/t/t7600-merge.sh > +++ b/t/t7600-merge.sh > @@ -154,6 +154,7 @@ test_expect_success 'test option parsing' ' > test_must_fail git merge -s foobar c1 && > test_must_fail git merge -s=foobar c1 && > test_must_fail git merge -m && > + test_must_fail git merge --continue foobar && > test_must_fail git merge > ' Your tests look good, though obviously if you check for options above, that should be covered in this test. -Peff