[PATCH v2 3/3] credential: parse subcommands with OPT_SUBCOMMAND

[Wasswa Edson <edsonwasswa54@gmail.com>]

Now that each subcommand of "git credential" lives in its own
function, switch the dispatch to OPT_SUBCOMMAND(). This gives each
subcommand its own usage string, so that "git credential <subcommand>
-h" shows help for that subcommand, and makes the subcommand names
available to the completion via --git-completion-helper.

Each subcommand takes neither options nor arguments; reject any
with a common helper. Drop NO_PARSEOPT from the git.c entry, as the
command now uses parse-options.

This changes the behavior only on error paths:

 - A missing or unknown subcommand is reported as such ("need a
   subcommand" / "unknown subcommand") before the usage text.

 - Extra arguments are reported as "too many arguments" before the
   usage text of the subcommand.

 - "git credential <subcommand> -h" shows the usage of that
   subcommand and exits with 0, instead of failing with the usage of
   the whole command.

Failures still exit with 129.

Helped-by: Tanga Joris <tangajoris@gmail.com>
Signed-off-by: Wasswa Edson <edsonwasswa54@gmail.com>
---
 builtin/credential.c   | 91 +++++++++++++++++++++++++++++-------------
 git.c                  |  2 +-
 t/t0300-credentials.sh | 27 +++++++++++++
 3 files changed, 91 insertions(+), 29 deletions(-)

diff --git a/builtin/credential.c b/builtin/credential.c
index d6ba21a2cb..b9945d019b 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -5,16 +5,52 @@
 #include "builtin.h"
 #include "environment.h"
 #include "config.h"
+#include "gettext.h"
+#include "parse-options.h"
+
+static const char * const credential_usage[] = {
+	N_("git credential (fill|approve|reject|capability)"),
+	NULL,
+};
+
+static const char * const credential_fill_usage[] = {
+	N_("git credential fill"),
+	NULL,
+};
+
+static const char * const credential_approve_usage[] = {
+	N_("git credential approve"),
+	NULL,
+};
+
+static const char * const credential_reject_usage[] = {
+	N_("git credential reject"),
+	NULL,
+};
+
+static const char * const credential_capability_usage[] = {
+	N_("git credential capability"),
+	NULL,
+};
+
+static void parse_no_args(int argc, const char **argv, const char *prefix,
+			  const char * const *usagestr)
+{
+	struct option options[] = {
+		OPT_END()
+	};
 
-static const char usage_msg[] =
-	"git credential (fill|approve|reject|capability)";
+	if (parse_options(argc, argv, prefix, options, usagestr, 0))
+		usage_msg_opt(_("too many arguments"), usagestr, options);
+}
 
-static int cmd_credential_fill(int argc UNUSED, const char **argv UNUSED,
-			       const char *prefix UNUSED,
+static int cmd_credential_fill(int argc, const char **argv, const char *prefix,
 			       struct repository *repo UNUSED)
 {
 	struct credential c = CREDENTIAL_INIT;
 
+	parse_no_args(argc, argv, prefix, credential_fill_usage);
+
 	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
 		die("unable to read credential from stdin");
 
@@ -26,12 +62,14 @@ static int cmd_credential_fill(int argc UNUSED, const char **argv UNUSED,
 	return 0;
 }
 
-static int cmd_credential_approve(int argc UNUSED, const char **argv UNUSED,
-				  const char *prefix UNUSED,
+static int cmd_credential_approve(int argc, const char **argv,
+				  const char *prefix,
 				  struct repository *repo UNUSED)
 {
 	struct credential c = CREDENTIAL_INIT;
 
+	parse_no_args(argc, argv, prefix, credential_approve_usage);
+
 	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
 		die("unable to read credential from stdin");
 
@@ -42,12 +80,14 @@ static int cmd_credential_approve(int argc UNUSED, const char **argv UNUSED,
 	return 0;
 }
 
-static int cmd_credential_reject(int argc UNUSED, const char **argv UNUSED,
-				 const char *prefix UNUSED,
+static int cmd_credential_reject(int argc, const char **argv,
+				 const char *prefix,
 				 struct repository *repo UNUSED)
 {
 	struct credential c = CREDENTIAL_INIT;
 
+	parse_no_args(argc, argv, prefix, credential_reject_usage);
+
 	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
 		die("unable to read credential from stdin");
 
@@ -58,13 +98,14 @@ static int cmd_credential_reject(int argc UNUSED, const char **argv UNUSED,
 	return 0;
 }
 
-static int cmd_credential_capability(int argc UNUSED,
-				     const char **argv UNUSED,
-				     const char *prefix UNUSED,
+static int cmd_credential_capability(int argc, const char **argv,
+				     const char *prefix,
 				     struct repository *repo UNUSED)
 {
 	struct credential c = CREDENTIAL_INIT;
 
+	parse_no_args(argc, argv, prefix, credential_capability_usage);
+
 	credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
 	credential_announce_capabilities(&c, stdout);
 
@@ -77,24 +118,18 @@ int cmd_credential(int argc,
 		   const char *prefix,
 		   struct repository *repo)
 {
-	const char *op;
+	parse_opt_subcommand_fn *fn = NULL;
+	struct option options[] = {
+		OPT_SUBCOMMAND("fill", &fn, cmd_credential_fill),
+		OPT_SUBCOMMAND("approve", &fn, cmd_credential_approve),
+		OPT_SUBCOMMAND("reject", &fn, cmd_credential_reject),
+		OPT_SUBCOMMAND("capability", &fn, cmd_credential_capability),
+		OPT_END()
+	};
 
 	repo_config(the_repository, git_default_config, NULL);
 
-	show_usage_if_asked(argc, argv, usage_msg);
-	if (argc != 2)
-		usage(usage_msg);
-	op = argv[1];
-
-	if (!strcmp(op, "fill"))
-		return cmd_credential_fill(argc - 1, argv + 1, prefix, repo);
-	if (!strcmp(op, "approve"))
-		return cmd_credential_approve(argc - 1, argv + 1, prefix, repo);
-	if (!strcmp(op, "reject"))
-		return cmd_credential_reject(argc - 1, argv + 1, prefix, repo);
-	if (!strcmp(op, "capability"))
-		return cmd_credential_capability(argc - 1, argv + 1, prefix,
-						 repo);
-
-	usage(usage_msg);
+	argc = parse_options(argc, argv, prefix, options, credential_usage, 0);
+
+	return fn(argc, argv, prefix, repo);
 }
diff --git a/git.c b/git.c
index d7b5f88e52..da370902ab 100644
--- a/git.c
+++ b/git.c
@@ -562,7 +562,7 @@ static struct cmd_struct commands[] = {
 	{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 	{ "config", cmd_config, RUN_SETUP_GENTLY | DELAY_PAGER_CONFIG },
 	{ "count-objects", cmd_count_objects, RUN_SETUP },
-	{ "credential", cmd_credential, RUN_SETUP_GENTLY | NO_PARSEOPT },
+	{ "credential", cmd_credential, RUN_SETUP_GENTLY },
 	{ "credential-cache", cmd_credential_cache },
 	{ "credential-cache--daemon", cmd_credential_cache_daemon },
 	{ "credential-store", cmd_credential_store },
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index ea11cdb4dc..858996ac4a 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -1056,4 +1056,31 @@ test_expect_success 'interactive prompt is sanitized' '
 	EOF
 '
 
+test_expect_success 'credential subcommands -h show subcommand usage' '
+	for sub in fill approve reject capability
+	do
+		git credential $sub -h >out &&
+		test_grep "^usage: git credential $sub" out || return 1
+	done
+'
+
+test_expect_success 'credential requires a subcommand' '
+	test_expect_code 129 git credential 2>err &&
+	test_grep "need a subcommand" err
+'
+
+test_expect_success 'credential rejects unknown subcommand' '
+	test_expect_code 129 git credential foo 2>err &&
+	test_grep "unknown subcommand" err
+'
+
+test_expect_success 'credential subcommands reject extra arguments' '
+	for sub in fill approve reject capability
+	do
+		test_expect_code 129 git credential $sub extra \
+			</dev/null 2>err &&
+		test_grep "too many arguments" err || return 1
+	done
+'
+
 test_done
-- 
2.53.0