[PATCH] credential: parse subcommands with OPT_SUBCOMMAND

[PATCH] credential: parse subcommands with OPT_SUBCOMMAND

[Wasswa Edson]

"git credential" dispatches its four subcommands (fill, approve,
reject, capability) with a strcmp() chain after an argc check,
instead of using the parse-options API used by other builtins.

Convert the dispatch to OPT_SUBCOMMAND(). Besides making the code
shorter and more idiomatic, each subcommand gets its own usage
string, so "git credential <subcommand> -h" shows a targeted help
text, and the subcommand names are reported via
--git-completion-helper.

Since the builtin now uses parse-options, drop NO_PARSEOPT from its
git.c entry so "git --list-cmds=parseopt" includes it.

Two pre-existing inconsistencies are fixed while at it:

- The usage string did not mention "capability" even though it is a
  documented subcommand; it now lists all four.
- Documentation/git-credential.adoc marked its SYNOPSIS as a plain
  literal block, which t0450 cannot extract; mark it [verse] and drop
  "credential" from t0450/adoc-help-mismatches.

Behavior changes are limited to error paths: a missing or unknown
subcommand is reported as such ("need a subcommand" / "unknown
subcommand") before the usage text, instead of only showing the
usage text, and "git credential <subcommand> -h" exits 0 after
showing the subcommand's usage. Exit codes for failures are
unchanged (129).

Signed-off-by: Wasswa Edson <edsonwasswa54@gmail.com>
---
 Documentation/git-credential.adoc |   3 +-
 builtin/credential.c              | 148 ++++++++++++++++++++++++------
 git.c                             |   2 +-
 t/t0300-credentials.sh            |  18 ++++
 t/t0450/adoc-help-mismatches      |   1 -
 5 files changed, 138 insertions(+), 34 deletions(-)

diff --git a/Documentation/git-credential.adoc b/Documentation/git-credential.adoc
index e41493292f..e12d429f43 100644
--- a/Documentation/git-credential.adoc
+++ b/Documentation/git-credential.adoc
@@ -7,9 +7,8 @@ git-credential - Retrieve and store user credentials
 
 SYNOPSIS
 --------
-------------------
+[verse]
 'git credential' (fill|approve|reject|capability)
-------------------
 
 DESCRIPTION
 -----------
diff --git a/builtin/credential.c b/builtin/credential.c
index a295c80b36..7b398658bf 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -5,48 +5,136 @@
 #include "builtin.h"
 #include "environment.h"
 #include "config.h"
+#include "gettext.h"
+#include "parse-options.h"
 
-static const char usage_msg[] =
-	"git credential (fill|approve|reject)";
+static const char * const credential_usage[] = {
+	N_("git credential (fill|approve|reject|capability)"),
+	NULL,
+};
 
-int cmd_credential(int argc,
-		   const char **argv,
-		   const char *prefix UNUSED,
-		   struct repository *repo UNUSED)
+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 int cmd_credential_fill(int argc, const char **argv, const char *prefix,
+			       struct repository *repo UNUSED)
 {
-	const char *op;
+	struct option options[] = { OPT_END() };
 	struct credential c = CREDENTIAL_INIT;
 
-	repo_config(the_repository, git_default_config, NULL);
+	argc = parse_options(argc, argv, prefix, options,
+			     credential_fill_usage, 0);
+	if (argc)
+		usage_with_options(credential_fill_usage, options);
+
+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
+		die("unable to read credential from stdin");
+
+	credential_fill(the_repository, &c, 0);
+	credential_next_state(&c);
+	credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
+
+	credential_clear(&c);
+	return 0;
+}
 
-	show_usage_if_asked(argc, argv, usage_msg);
-	if (argc != 2)
-		usage(usage_msg);
-	op = argv[1];
+static int cmd_credential_approve(int argc, const char **argv,
+				const char *prefix,
+				struct repository *repo UNUSED)
+{
+	struct option options[] = { OPT_END() };
+	struct credential c = CREDENTIAL_INIT;
 
-	if (!strcmp(op, "capability")) {
-		credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
-		credential_announce_capabilities(&c, stdout);
-		return 0;
-	}
+	argc = parse_options(argc, argv, prefix, options,
+			     credential_approve_usage, 0);
+	if (argc)
+		usage_with_options(credential_approve_usage, options);
 
 	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
 		die("unable to read credential from stdin");
 
-	if (!strcmp(op, "fill")) {
-		credential_fill(the_repository, &c, 0);
-		credential_next_state(&c);
-		credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
-	} else if (!strcmp(op, "approve")) {
-		credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
-		credential_approve(the_repository, &c);
-	} else if (!strcmp(op, "reject")) {
-		credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
-		credential_reject(the_repository, &c);
-	} else {
-		usage(usage_msg);
-	}
+	credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
+	credential_approve(the_repository, &c);
 
 	credential_clear(&c);
 	return 0;
 }
+
+static int cmd_credential_reject(int argc, const char **argv,
+				 const char *prefix,
+				 struct repository *repo UNUSED)
+{
+	struct option options[] = { OPT_END() };
+	struct credential c = CREDENTIAL_INIT;
+
+	argc = parse_options(argc, argv, prefix, options,
+			     credential_reject_usage, 0);
+	if (argc)
+		usage_with_options(credential_reject_usage, options);
+
+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
+		die("unable to read credential from stdin");
+
+	credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
+	credential_reject(the_repository, &c);
+
+	credential_clear(&c);
+	return 0;
+}
+
+static int cmd_credential_capability(int argc, const char **argv,
+				     const char *prefix,
+				     struct repository *repo UNUSED)
+{
+	struct option options[] = { OPT_END() };
+	struct credential c = CREDENTIAL_INIT;
+
+	argc = parse_options(argc, argv, prefix, options,
+			     credential_capability_usage, 0);
+	if (argc)
+		usage_with_options(credential_capability_usage, options);
+
+	credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
+	credential_announce_capabilities(&c, stdout);
+
+	credential_clear(&c);
+	return 0;
+}
+
+int cmd_credential(int argc,
+		   const char **argv,
+		   const char *prefix,
+		   struct repository *repo)
+{
+	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);
+
+	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..ce230caff1 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -1056,4 +1056,22 @@ 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_done
diff --git a/t/t0450/adoc-help-mismatches b/t/t0450/adoc-help-mismatches
index baf3b1d809..939e47bf3a 100644
--- a/t/t0450/adoc-help-mismatches
+++ b/t/t0450/adoc-help-mismatches
@@ -9,7 +9,6 @@ checkout
 checkout-index
 clone
 column
-credential
 credential-cache
 credential-store
 fetch-pack
-- 
2.53.0

Re: [PATCH] credential: parse subcommands with OPT_SUBCOMMAND

[Tanga Joris]

Hi Edson,

Thanks for working on this, nice work adding subcommands completion for
"git credential".

On Thu, 5 Nov 2026 at 03:59, Wasswa Edson <edsonwasswa54@gmail.com> wrote:
> Besides making the code
> shorter and more idiomatic,

builtin/credential.c goes from 52 lines to 140, so maybe "shorter"
could go.

Overall, the patch looks good, but most of the new lines are the same
parse_options() call and check, repeated four times. Maybe a small helper
would work here?

> +     if (argc)
> +             usage_with_options(credential_fill_usage, options);

One small thing: no test covers this check. When I removed it, t0300
still passed. Maybe something like this?

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 || return 1
        done
'

Thanks,
Tanga

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

[Wasswa Edson]

"git credential" dispatches its subcommands by hand with strcmp().
This series converts it to OPT_SUBCOMMAND(), so that each subcommand
gets its own "-h" usage and the subcommand names become available
to completion.

Changes since v1:

 - Split into three patches. The pre-existing mismatch between the
   "-h" output and the SYNOPSIS (1/3) and a refactor moving each
   subcommand into its own function (2/3) are now separate from the
   switch to OPT_SUBCOMMAND() (3/3).

 - Factor the repeated parse_options() call and argument check into
   a helper, which rejects leftover arguments with "too many
   arguments" (Tanga).

 - Add a test for the rejection of extra arguments (Tanga).

 - Drop the claim that the code gets shorter (Tanga).

v1: https://lore.kernel.org/git/20261104202228.24642-1-edsonwasswa54@gmail.com/

Wasswa Edson (3):
  credential: make -h output and SYNOPSIS agree
  credential: move each subcommand into its own function
  credential: parse subcommands with OPT_SUBCOMMAND

 Documentation/git-credential.adoc |   3 +-
 builtin/credential.c              | 143 +++++++++++++++++++++++-------
 git.c                             |   2 +-
 t/t0300-credentials.sh            |  27 ++++++
 t/t0450/adoc-help-mismatches      |   1 -
 5 files changed, 142 insertions(+), 34 deletions(-)

Range-diff against v1:
-:  ---------- > 1:  d1bffb83df credential: make -h output and SYNOPSIS agree
-:  ---------- > 2:  c7708ac31b credential: move each subcommand into its own function
1:  85e3f2ce94 ! 3:  7c9842bb9e credential: parse subcommands with OPT_SUBCOMMAND
    @@ Metadata
      ## Commit message ##
         credential: parse subcommands with OPT_SUBCOMMAND
     
    -    "git credential" dispatches its four subcommands (fill, approve,
    -    reject, capability) with a strcmp() chain after an argc check,
    -    instead of using the parse-options API used by other builtins.
    +    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.
     
    -    Convert the dispatch to OPT_SUBCOMMAND(). Besides making the code
    -    shorter and more idiomatic, each subcommand gets its own usage
    -    string, so "git credential <subcommand> -h" shows a targeted help
    -    text, and the subcommand names are reported 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.
     
    -    Since the builtin now uses parse-options, drop NO_PARSEOPT from its
    -    git.c entry so "git --list-cmds=parseopt" includes it.
    +    This changes the behavior only on error paths:
     
    -    Two pre-existing inconsistencies are fixed while at it:
    +     - A missing or unknown subcommand is reported as such ("need a
    +       subcommand" / "unknown subcommand") before the usage text.
     
    -    - The usage string did not mention "capability" even though it is a
    -      documented subcommand; it now lists all four.
    -    - Documentation/git-credential.adoc marked its SYNOPSIS as a plain
    -      literal block, which t0450 cannot extract; mark it [verse] and drop
    -      "credential" from t0450/adoc-help-mismatches.
    +     - Extra arguments are reported as "too many arguments" before the
    +       usage text of the subcommand.
     
    -    Behavior changes are limited to error paths: a missing or unknown
    -    subcommand is reported as such ("need a subcommand" / "unknown
    -    subcommand") before the usage text, instead of only showing the
    -    usage text, and "git credential <subcommand> -h" exits 0 after
    -    showing the subcommand's usage. Exit codes for failures are
    -    unchanged (129).
    +     - "git credential <subcommand> -h" shows the usage of that
    +       subcommand and exits with 0, instead of failing with the usage of
    +       the whole command.
     
    -    Signed-off-by: Wasswa Edson <edsonwasswa54@gmail.com>
    +    Failures still exit with 129.
     
    - ## Documentation/git-credential.adoc ##
    -@@ Documentation/git-credential.adoc: git-credential - Retrieve and store user credentials
    - 
    - SYNOPSIS
    - --------
    --------------------
    -+[verse]
    - 'git credential' (fill|approve|reject|capability)
    --------------------
    - 
    - DESCRIPTION
    - -----------
    +    Helped-by: Tanga Joris <tangajoris@gmail.com>
    +    Signed-off-by: Wasswa Edson <edsonwasswa54@gmail.com>
     
      ## builtin/credential.c ##
     @@
    @@ builtin/credential.c
      #include "config.h"
     +#include "gettext.h"
     +#include "parse-options.h"
    - 
    --static const char usage_msg[] =
    --	"git credential (fill|approve|reject)";
    ++
     +static const char * const credential_usage[] = {
     +	N_("git credential (fill|approve|reject|capability)"),
     +	NULL,
     +};
    - 
    --int cmd_credential(int argc,
    --		   const char **argv,
    --		   const char *prefix UNUSED,
    --		   struct repository *repo UNUSED)
    ++
     +static const char * const credential_fill_usage[] = {
     +	N_("git credential fill"),
     +	NULL,
    @@ builtin/credential.c
     +	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 repository *repo UNUSED)
      {
    --	const char *op;
    -+	struct option options[] = { OPT_END() };
      	struct credential c = CREDENTIAL_INIT;
      
    --	repo_config(the_repository, git_default_config, NULL);
    -+	argc = parse_options(argc, argv, prefix, options,
    -+			     credential_fill_usage, 0);
    -+	if (argc)
    -+		usage_with_options(credential_fill_usage, options);
    -+
    -+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
    -+		die("unable to read credential from stdin");
    ++	parse_no_args(argc, argv, prefix, credential_fill_usage);
     +
    -+	credential_fill(the_repository, &c, 0);
    -+	credential_next_state(&c);
    -+	credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
    -+
    -+	credential_clear(&c);
    -+	return 0;
    -+}
    + 	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
    + 		die("unable to read credential from stdin");
      
    --	show_usage_if_asked(argc, argv, usage_msg);
    --	if (argc != 2)
    --		usage(usage_msg);
    --	op = argv[1];
    -+static int cmd_credential_approve(int argc, const char **argv,
    -+				const char *prefix,
    -+				struct repository *repo UNUSED)
    -+{
    -+	struct option options[] = { OPT_END() };
    -+	struct credential c = CREDENTIAL_INIT;
    +@@ builtin/credential.c: static int cmd_credential_fill(int argc UNUSED, const char **argv UNUSED,
    + 	return 0;
    + }
      
    --	if (!strcmp(op, "capability")) {
    --		credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
    --		credential_announce_capabilities(&c, stdout);
    --		return 0;
    --	}
    -+	argc = parse_options(argc, argv, prefix, options,
    -+			     credential_approve_usage, 0);
    -+	if (argc)
    -+		usage_with_options(credential_approve_usage, options);
    +-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");
      
    --	if (!strcmp(op, "fill")) {
    --		credential_fill(the_repository, &c, 0);
    --		credential_next_state(&c);
    --		credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
    --	} else if (!strcmp(op, "approve")) {
    --		credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
    --		credential_approve(the_repository, &c);
    --	} else if (!strcmp(op, "reject")) {
    --		credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
    --		credential_reject(the_repository, &c);
    --	} else {
    --		usage(usage_msg);
    --	}
    -+	credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
    -+	credential_approve(the_repository, &c);
    - 
    - 	credential_clear(&c);
    +@@ builtin/credential.c: 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 option options[] = { OPT_END() };
    -+	struct credential c = CREDENTIAL_INIT;
    -+
    -+	argc = parse_options(argc, argv, prefix, options,
    -+			     credential_reject_usage, 0);
    -+	if (argc)
    -+		usage_with_options(credential_reject_usage, options);
    -+
    -+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
    -+		die("unable to read credential from stdin");
    -+
    -+	credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
    -+	credential_reject(the_repository, &c);
    -+
    -+	credential_clear(&c);
    -+	return 0;
    -+}
    + 				 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");
    + 
    +@@ builtin/credential.c: 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 option options[] = { OPT_END() };
    -+	struct credential c = CREDENTIAL_INIT;
    -+
    -+	argc = parse_options(argc, argv, prefix, options,
    -+			     credential_capability_usage, 0);
    -+	if (argc)
    -+		usage_with_options(credential_capability_usage, options);
    -+
    -+	credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
    -+	credential_announce_capabilities(&c, stdout);
    -+
    -+	credential_clear(&c);
    -+	return 0;
    -+}
    + 				     struct repository *repo UNUSED)
    + {
    + 	struct credential c = CREDENTIAL_INIT;
    + 
    ++	parse_no_args(argc, argv, prefix, credential_capability_usage);
     +
    -+int cmd_credential(int argc,
    -+		   const char **argv,
    -+		   const char *prefix,
    -+		   struct repository *repo)
    -+{
    + 	credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
    + 	credential_announce_capabilities(&c, stdout);
    + 
    +@@ builtin/credential.c: 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),
    @@ builtin/credential.c
     +		OPT_SUBCOMMAND("capability", &fn, cmd_credential_capability),
     +		OPT_END()
     +	};
    -+
    -+	repo_config(the_repository, git_default_config, NULL);
    -+
    + 
    + 	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);
    -+}
    + }
     
      ## git.c ##
     @@ git.c: static struct cmd_struct commands[] = {
    @@ t/t0300-credentials.sh: test_expect_success 'interactive prompt is sanitized' '
     +	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
    -
    - ## t/t0450/adoc-help-mismatches ##
    -@@ t/t0450/adoc-help-mismatches: checkout
    - checkout-index
    - clone
    - column
    --credential
    - credential-cache
    - credential-store
    - fetch-pack

base-commit: 37b3475fb530c14109211faca33716a173db61ed
-- 
2.53.0

[PATCH v2 1/3] credential: make -h output and SYNOPSIS agree

[Wasswa Edson]

The usage string of "git credential" does not mention the
"capability" subcommand, even though it is documented and
implemented. The SYNOPSIS in its documentation does list it, but
is written as a plain literal block instead of using [verse], so
t0450 cannot extract it, and "credential" has been listed as a
known mismatch between the "-h" output and the documentation.

Add "capability" to the usage string, mark the SYNOPSIS as
[verse], and drop "credential" from the list of known mismatches.

Signed-off-by: Wasswa Edson <edsonwasswa54@gmail.com>
---
 Documentation/git-credential.adoc | 3 +--
 builtin/credential.c              | 2 +-
 t/t0450/adoc-help-mismatches      | 1 -
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-credential.adoc b/Documentation/git-credential.adoc
index e41493292f..e12d429f43 100644
--- a/Documentation/git-credential.adoc
+++ b/Documentation/git-credential.adoc
@@ -7,9 +7,8 @@ git-credential - Retrieve and store user credentials
 
 SYNOPSIS
 --------
-------------------
+[verse]
 'git credential' (fill|approve|reject|capability)
-------------------
 
 DESCRIPTION
 -----------
diff --git a/builtin/credential.c b/builtin/credential.c
index a295c80b36..72e7b2af4e 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -7,7 +7,7 @@
 #include "config.h"
 
 static const char usage_msg[] =
-	"git credential (fill|approve|reject)";
+	"git credential (fill|approve|reject|capability)";
 
 int cmd_credential(int argc,
 		   const char **argv,
diff --git a/t/t0450/adoc-help-mismatches b/t/t0450/adoc-help-mismatches
index baf3b1d809..939e47bf3a 100644
--- a/t/t0450/adoc-help-mismatches
+++ b/t/t0450/adoc-help-mismatches
@@ -9,7 +9,6 @@ checkout
 checkout-index
 clone
 column
-credential
 credential-cache
 credential-store
 fetch-pack
-- 
2.53.0

[PATCH v2 2/3] credential: move each subcommand into its own function

[Wasswa Edson]

cmd_credential() handles all of its subcommands inline, sharing a
single "struct credential" and dispatching with a chain of
strcmp() calls.

Move each subcommand into its own function with the signature
expected by parse-options' subcommand callbacks, in preparation for
switching the dispatch to OPT_SUBCOMMAND() in the next commit.

The only change in behavior is that an unknown subcommand is now
rejected before reading a credential from the standard input.

Signed-off-by: Wasswa Edson <edsonwasswa54@gmail.com>
---
 builtin/credential.c | 102 +++++++++++++++++++++++++++++++------------
 1 file changed, 75 insertions(+), 27 deletions(-)

diff --git a/builtin/credential.c b/builtin/credential.c
index 72e7b2af4e..d6ba21a2cb 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -9,13 +9,75 @@
 static const char usage_msg[] =
 	"git credential (fill|approve|reject|capability)";
 
+static int cmd_credential_fill(int argc UNUSED, const char **argv UNUSED,
+			       const char *prefix UNUSED,
+			       struct repository *repo UNUSED)
+{
+	struct credential c = CREDENTIAL_INIT;
+
+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
+		die("unable to read credential from stdin");
+
+	credential_fill(the_repository, &c, 0);
+	credential_next_state(&c);
+	credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
+
+	credential_clear(&c);
+	return 0;
+}
+
+static int cmd_credential_approve(int argc UNUSED, const char **argv UNUSED,
+				  const char *prefix UNUSED,
+				  struct repository *repo UNUSED)
+{
+	struct credential c = CREDENTIAL_INIT;
+
+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
+		die("unable to read credential from stdin");
+
+	credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
+	credential_approve(the_repository, &c);
+
+	credential_clear(&c);
+	return 0;
+}
+
+static int cmd_credential_reject(int argc UNUSED, const char **argv UNUSED,
+				 const char *prefix UNUSED,
+				 struct repository *repo UNUSED)
+{
+	struct credential c = CREDENTIAL_INIT;
+
+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
+		die("unable to read credential from stdin");
+
+	credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
+	credential_reject(the_repository, &c);
+
+	credential_clear(&c);
+	return 0;
+}
+
+static int cmd_credential_capability(int argc UNUSED,
+				     const char **argv UNUSED,
+				     const char *prefix UNUSED,
+				     struct repository *repo UNUSED)
+{
+	struct credential c = CREDENTIAL_INIT;
+
+	credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
+	credential_announce_capabilities(&c, stdout);
+
+	credential_clear(&c);
+	return 0;
+}
+
 int cmd_credential(int argc,
 		   const char **argv,
-		   const char *prefix UNUSED,
-		   struct repository *repo UNUSED)
+		   const char *prefix,
+		   struct repository *repo)
 {
 	const char *op;
-	struct credential c = CREDENTIAL_INIT;
 
 	repo_config(the_repository, git_default_config, NULL);
 
@@ -24,29 +86,15 @@ int cmd_credential(int argc,
 		usage(usage_msg);
 	op = argv[1];
 
-	if (!strcmp(op, "capability")) {
-		credential_set_all_capabilities(&c, CREDENTIAL_OP_INITIAL);
-		credential_announce_capabilities(&c, stdout);
-		return 0;
-	}
-
-	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
-		die("unable to read credential from stdin");
-
-	if (!strcmp(op, "fill")) {
-		credential_fill(the_repository, &c, 0);
-		credential_next_state(&c);
-		credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
-	} else if (!strcmp(op, "approve")) {
-		credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
-		credential_approve(the_repository, &c);
-	} else if (!strcmp(op, "reject")) {
-		credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
-		credential_reject(the_repository, &c);
-	} else {
-		usage(usage_msg);
-	}
+	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);
 
-	credential_clear(&c);
-	return 0;
+	usage(usage_msg);
 }
-- 
2.53.0

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

[Wasswa Edson]

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