builtin/remote.c: let parse-options parse subcommands

'git remote' parses its subcommands with a long list of if-else if
statements.  parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling unknown subcommands, and listing subcommands for Bash
completion.  Make sure that the default operation mode doesn't accept
any arguments; and while at it remove the capitalization of the error
message and adjust the test checking it accordingly.

Note that 'git remote' has both 'remove' and 'rm' subcommands, and the
former is preferred [1], so hide the latter for completion.

Note also that the functions implementing each subcommand only accept
the 'argc' and '**argv' parameters, so add a (unused) '*prefix'
parameter to make them match the type expected by parse-options, and
thus avoid casting a bunch of function pointers.

[1] e17dba8fe1 (remote: prefer subcommand name 'remove' to 'rm',
    2012-09-06)

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/remote.c b/builtin/remote.c
index d9b8746cb3cb6cf78c8790971ad8f23e6a91d2e3..4a6d47c03a4bb508f7543f07a7ac013577c9c893 100644 (file)
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -150,7 +150,7 @@ static int parse_mirror_opt(const struct option *opt, const char *arg, int not)
        return 0;
 }
 
-static int add(int argc, const char **argv)
+static int add(int argc, const char **argv, const char *prefix)
 {
        int fetch = 0, fetch_tags = TAGS_DEFAULT;
        unsigned mirror = MIRROR_NONE;
@@ -680,7 +680,7 @@ static void handle_push_default(const char* old_name, const char* new_name)
 }
 
 
-static int mv(int argc, const char **argv)
+static int mv(int argc, const char **argv, const char *prefix)
 {
        int show_progress = isatty(2);
        struct option options[] = {
@@ -844,7 +844,7 @@ static int mv(int argc, const char **argv)
        return 0;
 }
 
-static int rm(int argc, const char **argv)
+static int rm(int argc, const char **argv, const char *prefix)
 {
        struct option options[] = {
                OPT_END()
@@ -1255,7 +1255,7 @@ static int show_all(void)
        return result;
 }
 
-static int show(int argc, const char **argv)
+static int show(int argc, const char **argv, const char *prefix)
 {
        int no_query = 0, result = 0, query_flag = 0;
        struct option options[] = {
@@ -1358,7 +1358,7 @@ static int show(int argc, const char **argv)
        return result;
 }
 
-static int set_head(int argc, const char **argv)
+static int set_head(int argc, const char **argv, const char *prefix)
 {
        int i, opt_a = 0, opt_d = 0, result = 0;
        struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
@@ -1463,7 +1463,7 @@ static int prune_remote(const char *remote, int dry_run)
        return result;
 }
 
-static int prune(int argc, const char **argv)
+static int prune(int argc, const char **argv, const char *prefix)
 {
        int dry_run = 0, result = 0;
        struct option options[] = {
@@ -1492,7 +1492,7 @@ static int get_remote_default(const char *key, const char *value, void *priv)
        return 0;
 }
 
-static int update(int argc, const char **argv)
+static int update(int argc, const char **argv, const char *prefix)
 {
        int i, prune = -1;
        struct option options[] = {
@@ -1575,7 +1575,7 @@ static int set_remote_branches(const char *remotename, const char **branches,
        return 0;
 }
 
-static int set_branches(int argc, const char **argv)
+static int set_branches(int argc, const char **argv, const char *prefix)
 {
        int add_mode = 0;
        struct option options[] = {
@@ -1594,7 +1594,7 @@ static int set_branches(int argc, const char **argv)
        return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
-static int get_url(int argc, const char **argv)
+static int get_url(int argc, const char **argv, const char *prefix)
 {
        int i, push_mode = 0, all_mode = 0;
        const char *remotename = NULL;
@@ -1647,7 +1647,7 @@ static int get_url(int argc, const char **argv)
        return 0;
 }
 
-static int set_url(int argc, const char **argv)
+static int set_url(int argc, const char **argv, const char *prefix)
 {
        int i, push_mode = 0, add_mode = 0, delete_mode = 0;
        int matches = 0, negative_matches = 0;
@@ -1739,41 +1739,33 @@ out:
 
 int cmd_remote(int argc, const char **argv, const char *prefix)
 {
+       parse_opt_subcommand_fn *fn = NULL;
        struct option options[] = {
                OPT__VERBOSE(&verbose, N_("be verbose; must be placed before a subcommand")),
+               OPT_SUBCOMMAND("add", &fn, add),
+               OPT_SUBCOMMAND("rename", &fn, mv),
+               OPT_SUBCOMMAND_F("rm", &fn, rm, PARSE_OPT_NOCOMPLETE),
+               OPT_SUBCOMMAND("remove", &fn, rm),
+               OPT_SUBCOMMAND("set-head", &fn, set_head),
+               OPT_SUBCOMMAND("set-branches", &fn, set_branches),
+               OPT_SUBCOMMAND("get-url", &fn, get_url),
+               OPT_SUBCOMMAND("set-url", &fn, set_url),
+               OPT_SUBCOMMAND("show", &fn, show),
+               OPT_SUBCOMMAND("prune", &fn, prune),
+               OPT_SUBCOMMAND("update", &fn, update),
                OPT_END()
        };
-       int result;
 
        argc = parse_options(argc, argv, prefix, options, builtin_remote_usage,
-               PARSE_OPT_STOP_AT_NON_OPTION);
+                            PARSE_OPT_SUBCOMMAND_OPTIONAL);
 
-       if (argc < 1)
-               result = show_all();
-       else if (!strcmp(argv[0], "add"))
-               result = add(argc, argv);
-       else if (!strcmp(argv[0], "rename"))
-               result = mv(argc, argv);
-       else if (!strcmp(argv[0], "rm") || !strcmp(argv[0], "remove"))
-               result = rm(argc, argv);
-       else if (!strcmp(argv[0], "set-head"))
-               result = set_head(argc, argv);
-       else if (!strcmp(argv[0], "set-branches"))
-               result = set_branches(argc, argv);
-       else if (!strcmp(argv[0], "get-url"))
-               result = get_url(argc, argv);
-       else if (!strcmp(argv[0], "set-url"))
-               result = set_url(argc, argv);
-       else if (!strcmp(argv[0], "show"))
-               result = show(argc, argv);
-       else if (!strcmp(argv[0], "prune"))
-               result = prune(argc, argv);
-       else if (!strcmp(argv[0], "update"))
-               result = update(argc, argv);
-       else {
-               error(_("Unknown subcommand: %s"), argv[0]);
-               usage_with_options(builtin_remote_usage, options);
+       if (fn) {
+               return !!fn(argc, argv, prefix);
+       } else {
+               if (argc) {
+                       error(_("unknown subcommand: %s"), argv[0]);
+                       usage_with_options(builtin_remote_usage, options);
+               }
+               return !!show_all();
        }
-
-       return result ? 1 : 0;
 }

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index a549a21ef6ae69c802ed340454857e0fba5cae85..9006196ac601e8f93f046b54eafe11aa9df58563 100755 (executable)
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -258,7 +258,7 @@ test_expect_success 'without subcommand accepts -v' '
 
 test_expect_success 'without subcommand does not take arguments' '
        test_expect_code 129 git -C test remote origin 2>err &&
-       grep "^error: Unknown subcommand:" err
+       grep "^error: unknown subcommand:" err
 '
 
 cat >test/expect <<EOF