diff mbox series

[7/8] reflog: convert to parse_options() API

Message ID patch-7.8-19e1ff0c569-20220317T180439Z-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series reflog: migrate fully to parse_options() | expand

Commit Message

Ævar Arnfjörð Bjarmason March 17, 2022, 6:08 p.m. UTC
Continue the work started in 33d7bdd6459 (builtin/reflog.c: use
parse-options api for expire, delete subcommands, 2022-01-06) and
convert the cmd_reflog() function itself to use the parse_options()
API.

Let's also add a test which would fail if we forgot
PARSE_OPT_NO_INTERNAL_HELP here, as well as making sure that we'll
still pass through "--" by supplying PARSE_OPT_KEEP_DASHDASH. For that
test we need to change "test_commit()" to accept files starting with
"--".

The "git reflog -h" usage will now show the usage for all of the
sub-commands, rather than a terse summary which wasn't
correct (e.g. "git reflog exists" is not a valid command). See my
8757b35d443 (commit-graph: define common usage with a macro,
2021-08-23) for prior art.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/reflog.c        | 40 ++++++++++++++++++++++++++++++++--------
 t/t1410-reflog.sh       | 17 +++++++++++++++++
 t/test-lib-functions.sh |  2 +-
 3 files changed, 50 insertions(+), 9 deletions(-)

Comments

Junio C Hamano March 18, 2022, 1:49 a.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

>  int cmd_reflog(int argc, const char **argv, const char *prefix)
>  {
> -	if (argc > 1 && !strcmp(argv[1], "-h"))
> -		usage(_(reflog_usage));
> +	struct option options[] = {
> +		OPT_END()
> +	};
>  
> -	/* With no command, we default to showing it. */
> -	if (argc < 2 || *argv[1] == '-')
> -		return cmd_log_reflog(argc, argv, prefix);
> +	argc = parse_options(argc, argv, prefix, options, reflog_usage,
> +			     PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0 |
> +			     PARSE_OPT_KEEP_UNKNOWN |
> +			     PARSE_OPT_NO_INTERNAL_HELP);
> +
> +	/*
> +	  * With "git reflog" we default to showing it. !argc is
> +	  * impossible with PARSE_OPT_KEEP_ARGV0.
> +	  */

Funny indentation?

> +	if (argc == 1)
> +		goto log_reflog;
> +
> +	if (!strcmp(argv[1], "-h"))
> +		usage_with_options(reflog_usage, options);
> +	else if (*argv[1] == '-')
> +		goto log_reflog;

Unfortunate.  KEEP_UNKNOWN is unwieldy, but it is no worse than the
original.  We do not have options that are common to all "git reflog"
subcommands, so the first token after "git reflog" being anything
that begins with "-" is a sign that the default "show" command is
being asked for.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 0f439c99d61..1d004744589 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -329,7 +329,7 @@ test_commit () {
>  	else
>  		$echo "${3-$1}" >"$indir$file"
>  	fi &&
> -	git ${indir:+ -C "$indir"} add "$file" &&
> +	git ${indir:+ -C "$indir"} add -- "$file" &&

OK.

Looking good.
diff mbox series

Patch

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 9847e9db3de..3971921fc14 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -32,6 +32,14 @@  static const char *const reflog_exists_usage[] = {
 	NULL,
 };
 
+static const char *const reflog_usage[] = {
+	N_("git reflog [show] [<log-options>] [<ref>]"),
+	BUILTIN_REFLOG_EXPIRE_USAGE,
+	BUILTIN_REFLOG_DELETE_USAGE,
+	BUILTIN_REFLOG_EXISTS_USAGE,
+	NULL
+};
+
 static timestamp_t default_reflog_expire;
 static timestamp_t default_reflog_expire_unreachable;
 
@@ -372,17 +380,28 @@  static int cmd_reflog_exists(int argc, const char **argv, const char *prefix)
  * main "reflog"
  */
 
-static const char reflog_usage[] =
-"git reflog [ show | expire | delete | exists ]";
-
 int cmd_reflog(int argc, const char **argv, const char *prefix)
 {
-	if (argc > 1 && !strcmp(argv[1], "-h"))
-		usage(_(reflog_usage));
+	struct option options[] = {
+		OPT_END()
+	};
 
-	/* With no command, we default to showing it. */
-	if (argc < 2 || *argv[1] == '-')
-		return cmd_log_reflog(argc, argv, prefix);
+	argc = parse_options(argc, argv, prefix, options, reflog_usage,
+			     PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0 |
+			     PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_NO_INTERNAL_HELP);
+
+	/*
+	  * With "git reflog" we default to showing it. !argc is
+	  * impossible with PARSE_OPT_KEEP_ARGV0.
+	  */
+	if (argc == 1)
+		goto log_reflog;
+
+	if (!strcmp(argv[1], "-h"))
+		usage_with_options(reflog_usage, options);
+	else if (*argv[1] == '-')
+		goto log_reflog;
 
 	if (!strcmp(argv[1], "show"))
 		return cmd_log_reflog(argc - 1, argv + 1, prefix);
@@ -393,5 +412,10 @@  int cmd_reflog(int argc, const char **argv, const char *prefix)
 	else if (!strcmp(argv[1], "exists"))
 		return cmd_reflog_exists(argc - 1, argv + 1, prefix);
 
+	/*
+	 * Fall-through for e.g. "git reflog -1", "git reflog master",
+	 * as well as the plain "git reflog" above goto above.
+	 */
+log_reflog:
 	return cmd_log_reflog(argc, argv, prefix);
 }
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 68f69bb5431..0dc36d842b0 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -106,6 +106,23 @@  test_expect_success setup '
 	test_line_count = 4 output
 '
 
+test_expect_success 'correct usage on sub-command -h' '
+	test_expect_code 129 git reflog expire -h >err &&
+	grep "git reflog expire" err
+'
+
+test_expect_success 'pass through -- to sub-command' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	test_commit -C repo message --a-file contents dash-tag &&
+
+	git -C repo reflog show -- --does-not-exist >out &&
+	test_must_be_empty out &&
+	git -C repo reflog show >expect &&
+	git -C repo reflog show -- --a-file >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success rewind '
 	test_tick && git reset --hard HEAD~2 &&
 	test -f C &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 0f439c99d61..1d004744589 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -329,7 +329,7 @@  test_commit () {
 	else
 		$echo "${3-$1}" >"$indir$file"
 	fi &&
-	git ${indir:+ -C "$indir"} add "$file" &&
+	git ${indir:+ -C "$indir"} add -- "$file" &&
 	if test -z "$notick"
 	then
 		test_tick