git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4] builtin/reflog.c: use parse-options api for expire, delete subcommands

From
John Cai via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 5, 2022, 04:06 UTC
Message-ID
<pull.1175.v4.git.git.1641355561700.gitgitgadget@gmail.com>
In-Reply-To
<pull.1175.v3.git.git.1641318061821.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>

Switching out manual arg parsing for the parse-options API for the expire and delete subcommands.

Move explicit_expiry flag into cmd_reflog_expire_cb struct so callbacks can set both the value of the timestamp as well as the explicit_expiry flag.

Signed-off-by: "John Cai" <johncai86@gmail.com>
---
    reflog.c: switch to use parse-options API
    
    Switch out manual argv parsing for the reflog expire subcommand to use
    the parse-options API. This reduces code redundancy by consolidating
    option parsing logic.
    
    NOTE: this patch was based off of next. I know now that I should base it
    off of master. But practically, nothing in master touched reflog.c
    except for ab/reflog-prep
    
    changes since v3:
    
     * fixed indentation
    
    changes since v2:
    
     * got rid of parse_opt_expiry_date helper based on feedback from Junio.
       Just call parse_expiry_date directly in the callbacks, and don't
       worry about unset, as we can just set the PARSE_OPT_NONEG flag for
       --expire and --expire-unreachable since the original code didn't
       support a negated version of those flags anyway. The documentation
       also explicitly states to use "never" as the argument.
     * fix places where we set int i inside a for loop construct.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1175%2Fjohn-cai%2Fjc%2Freflog-switch-to-option-parse-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1175/john-cai/jc/reflog-switch-to-option-parse-v4
Pull-Request: https://github.com/git/git/pull/1175
Range-diff vs v3:
 1:  99a97e6ee52 ! 1:  0efdf3d2cb1 builtin/reflog.c: use parse-options api for expire, delete subcommands
     @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c
       
      +static const char * reflog_expire_usage[] = {
      +	N_("git reflog expire [--expire=<time>] "
     -+   "[--expire-unreachable=<time>] "
     -+   "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] "
     -+   "[--verbose] [--all] <refs>..."),
     ++	   "[--expire-unreachable=<time>] "
     ++	   "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] "
     ++	   "[--verbose] [--all] <refs>..."),
      +	NULL
      +};
      +
 builtin/reflog.c | 174 ++++++++++++++++++++++++++---------------------
 1 file changed, 95 insertions(+), 79 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index a4b1dd27e13..1b3b298eec4 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -12,15 +12,6 @@
 #include "reachable.h"
 #include "worktree.h"
 
-/* NEEDSWORK: switch to using parse_options */
-static const char reflog_expire_usage[] =
-N_("git reflog expire [--expire=<time>] "
-   "[--expire-unreachable=<time>] "
-   "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] "
-   "[--verbose] [--all] <refs>...");
-static const char reflog_delete_usage[] =
-N_("git reflog delete [--rewrite] [--updateref] "
-   "[--dry-run | -n] [--verbose] <refs>...");
 static const char reflog_exists_usage[] =
 N_("git reflog exists <ref>");
 
@@ -29,6 +20,7 @@ static timestamp_t default_reflog_expire_unreachable;
 
 struct cmd_reflog_expire_cb {
 	int stalefix;
+	int explicit_expiry;
 	timestamp_t expire_total;
 	timestamp_t expire_unreachable;
 	int recno;
@@ -520,18 +512,18 @@ static int reflog_expire_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
-static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, const char *ref)
+static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, const char *ref)
 {
 	struct reflog_expire_cfg *ent;
 
-	if (slot == (EXPIRE_TOTAL|EXPIRE_UNREACH))
+	if (cb->explicit_expiry == (EXPIRE_TOTAL|EXPIRE_UNREACH))
 		return; /* both given explicitly -- nothing to tweak */
 
 	for (ent = reflog_expire_cfg; ent; ent = ent->next) {
 		if (!wildmatch(ent->pattern, ref, 0)) {
-			if (!(slot & EXPIRE_TOTAL))
+			if (!(cb->explicit_expiry & EXPIRE_TOTAL))
 				cb->expire_total = ent->expire_total;
-			if (!(slot & EXPIRE_UNREACH))
+			if (!(cb->explicit_expiry & EXPIRE_UNREACH))
 				cb->expire_unreachable = ent->expire_unreachable;
 			return;
 		}
@@ -541,29 +533,87 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c
 	 * If unconfigured, make stash never expire
 	 */
 	if (!strcmp(ref, "refs/stash")) {
-		if (!(slot & EXPIRE_TOTAL))
+		if (!(cb->explicit_expiry & EXPIRE_TOTAL))
 			cb->expire_total = 0;
-		if (!(slot & EXPIRE_UNREACH))
+		if (!(cb->explicit_expiry & EXPIRE_UNREACH))
 			cb->expire_unreachable = 0;
 		return;
 	}
 
 	/* Nothing matched -- use the default value */
-	if (!(slot & EXPIRE_TOTAL))
+	if (!(cb->explicit_expiry & EXPIRE_TOTAL))
 		cb->expire_total = default_reflog_expire;
-	if (!(slot & EXPIRE_UNREACH))
+	if (!(cb->explicit_expiry & EXPIRE_UNREACH))
 		cb->expire_unreachable = default_reflog_expire_unreachable;
 }
 
+static const char * reflog_expire_usage[] = {
+	N_("git reflog expire [--expire=<time>] "
+	   "[--expire-unreachable=<time>] "
+	   "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] "
+	   "[--verbose] [--all] <refs>..."),
+	NULL
+};
+
+static int expire_unreachable_callback(const struct option *opt,
+				 const char *arg,
+				 int unset)
+{
+	struct cmd_reflog_expire_cb *cmd = opt->value;
+
+	if (parse_expiry_date(arg, &cmd->expire_unreachable))
+			die(_("malformed expiration date '%s'"), arg);
+
+	cmd->explicit_expiry |= EXPIRE_UNREACH;
+	return 0;
+}
+
+static int expire_total_callback(const struct option *opt,
+				 const char *arg,
+				 int unset)
+{
+	struct cmd_reflog_expire_cb *cmd = opt->value;
+
+	if (parse_expiry_date(arg, &cmd->expire_total))
+			die(_("malformed expiration date '%s'"), arg);
+
+	cmd->explicit_expiry |= EXPIRE_TOTAL;
+	return 0;
+}
+
 static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 {
 	struct cmd_reflog_expire_cb cmd = { 0 };
 	timestamp_t now = time(NULL);
 	int i, status, do_all, all_worktrees = 1;
-	int explicit_expiry = 0;
 	unsigned int flags = 0;
 	int verbose = 0;
 	reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
+	const struct option options[] = {
+		OPT_BIT(0, "dry-run", &flags, N_("do not actually prune any entries"),
+			EXPIRE_REFLOGS_DRY_RUN),
+		OPT_BIT(0, "rewrite", &flags,
+			N_("rewrite the old SHA1 with the new SHA1 of the entry that now precedes it"),
+			EXPIRE_REFLOGS_REWRITE),
+		OPT_BIT(0, "updateref", &flags,
+			N_("update the reference to the value of the top reflog entry"),
+			EXPIRE_REFLOGS_UPDATE_REF),
+		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen.")),
+		OPT_CALLBACK_F(0, "expire", &cmd, N_("timestamp"),
+			N_("prune entries older than the specified time"),
+			PARSE_OPT_NONEG,
+			expire_total_callback),
+		OPT_CALLBACK_F(0, "expire-unreachable", &cmd, N_("timestamp"),
+			N_("prune entries older than <time> that are not reachable from the current tip of the branch"),
+			PARSE_OPT_NONEG,
+			expire_unreachable_callback),
+		OPT_BOOL(0, "stale-fix", &cmd.stalefix,
+			N_("prune any reflog entries that point to broken commits")),
+		OPT_BOOL(0, "all", &do_all, N_("process the reflogs of all references")),
+		OPT_BOOL(1, "single-worktree", &all_worktrees,
+			N_("limits processing to reflogs from the current worktree only.")),
+		OPT_END()
+	};
 
 	default_reflog_expire_unreachable = now - 30 * 24 * 3600;
 	default_reflog_expire = now - 90 * 24 * 3600;
@@ -572,45 +622,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 	save_commit_buffer = 0;
 	do_all = status = 0;
 
+	cmd.explicit_expiry = 0;
 	cmd.expire_total = default_reflog_expire;
 	cmd.expire_unreachable = default_reflog_expire_unreachable;
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n"))
-			flags |= EXPIRE_REFLOGS_DRY_RUN;
-		else if (skip_prefix(arg, "--expire=", &arg)) {
-			if (parse_expiry_date(arg, &cmd.expire_total))
-				die(_("'%s' is not a valid timestamp"), arg);
-			explicit_expiry |= EXPIRE_TOTAL;
-		}
-		else if (skip_prefix(arg, "--expire-unreachable=", &arg)) {
-			if (parse_expiry_date(arg, &cmd.expire_unreachable))
-				die(_("'%s' is not a valid timestamp"), arg);
-			explicit_expiry |= EXPIRE_UNREACH;
-		}
-		else if (!strcmp(arg, "--stale-fix"))
-			cmd.stalefix = 1;
-		else if (!strcmp(arg, "--rewrite"))
-			flags |= EXPIRE_REFLOGS_REWRITE;
-		else if (!strcmp(arg, "--updateref"))
-			flags |= EXPIRE_REFLOGS_UPDATE_REF;
-		else if (!strcmp(arg, "--all"))
-			do_all = 1;
-		else if (!strcmp(arg, "--single-worktree"))
-			all_worktrees = 0;
-		else if (!strcmp(arg, "--verbose"))
-			verbose = 1;
-		else if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		else if (arg[0] == '-')
-			usage(_(reflog_expire_usage));
-		else
-			break;
-	}
+	argc = parse_options(argc, argv, prefix, options, reflog_expire_usage, 0);
 
 	if (verbose)
 		should_prune_fn = should_expire_reflog_ent_verbose;
@@ -657,7 +673,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 				.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
 			};
 
-			set_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);
+			set_reflog_expiry_param(&cb.cmd,  item->string);
 			status |= reflog_expire(item->string, flags,
 						reflog_expiry_prepare,
 						should_prune_fn,
@@ -667,7 +683,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 		string_list_clear(&collected.reflogs, 0);
 	}
 
-	for (; i < argc; i++) {
+	for (i = 0; i < argc; i++) {
 		char *ref;
 		struct expire_reflog_policy_cb cb = { .cmd = cmd };
 
@@ -675,7 +691,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 			status |= error(_("%s points nowhere!"), argv[i]);
 			continue;
 		}
-		set_reflog_expiry_param(&cb.cmd, explicit_expiry, ref);
+		set_reflog_expiry_param(&cb.cmd, ref);
 		status |= reflog_expire(ref, flags,
 					reflog_expiry_prepare,
 					should_prune_fn,
@@ -696,6 +712,12 @@ static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
+static const char * reflog_delete_usage[] = {
+	N_("git reflog delete [--rewrite] [--updateref] "
+   "[--dry-run | -n] [--verbose] <refs>..."),
+	NULL
+};
+
 static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
 {
 	struct cmd_reflog_expire_cb cmd = { 0 };
@@ -703,34 +725,28 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
 	unsigned int flags = 0;
 	int verbose = 0;
 	reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
-
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n"))
-			flags |= EXPIRE_REFLOGS_DRY_RUN;
-		else if (!strcmp(arg, "--rewrite"))
-			flags |= EXPIRE_REFLOGS_REWRITE;
-		else if (!strcmp(arg, "--updateref"))
-			flags |= EXPIRE_REFLOGS_UPDATE_REF;
-		else if (!strcmp(arg, "--verbose"))
-			verbose = 1;
-		else if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		else if (arg[0] == '-')
-			usage(_(reflog_delete_usage));
-		else
-			break;
-	}
+	const struct option options[] = {
+		OPT_BIT(0, "dry-run", &flags, N_("do not actually prune any entries"),
+				EXPIRE_REFLOGS_DRY_RUN),
+		OPT_BIT(0, "rewrite", &flags,
+				N_("rewrite the old SHA1 with the new SHA1 of the entry that now precedes it"),
+				EXPIRE_REFLOGS_REWRITE),
+		OPT_BIT(0, "updateref", &flags,
+				N_("update the reference to the value of the top reflog entry"),
+				EXPIRE_REFLOGS_UPDATE_REF),
+		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen.")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options, reflog_delete_usage, 0);
 
 	if (verbose)
 		should_prune_fn = should_expire_reflog_ent_verbose;
 
-	if (argc - i < 1)
+	if (argc < 1)
 		return error(_("no reflog specified to delete"));
 
-	for ( ; i < argc; i++) {
+	for (i = 0; i < argc; i++) {
 		const char *spec = strstr(argv[i], "@{");
 		char *ep, *ref;
 		int recno;

base-commit: 194610f4cf2df839ebfd040c5da187c8c0162620

-- 
gitgitgadget
Previous: Junio C HamanoNext: Junio C Hamano
Message 15 of 19 in “reflog.c: switch to use parse-options API”
  1. 0/2 reflog.c: switch to use parse-options APIJohn Cai via GitGitGadget, Dec 31, 2021
  2. 1/2 builtin/reflog.c: use parse-options for expire subcommandJohn Cai via GitGitGadget, Dec 31, 2021
  3. John CaiJan 1, 2022
  4. René ScharfeJan 1, 2022
  5. John CaiJan 1, 2022
  6. René ScharfeJan 2, 2022
  7. 2/2 builtin/reflog.c: switch to use parse-options API for delete subcommandJohn Cai via GitGitGadget, Dec 31, 2021
  8. 0/2 reflog.c: switch to use parse-options APIJohn Cai via GitGitGadget, Jan 3, 2022
  9. 1/2 parse-options.h: add parse_opt_expiry_date helperJohn Cai via GitGitGadget, Jan 3, 2022
  10. Junio C HamanoJan 4, 2022
  11. 2/2 builtin/reflog.c: switch to use parse-options API for delete subcommandJohn Cai via GitGitGadget, Jan 3, 2022
  12. Junio C HamanoJan 4, 2022
  13. builtin/reflog.c: use parse-options api for expire, delete subcommandsJohn Cai via GitGitGadget, Jan 4, 2022
  14. Junio C HamanoJan 4, 2022
  15. builtin/reflog.c: use parse-options api for expire, delete subcommandsJohn Cai via GitGitGadget, Jan 5, 2022
  16. Junio C HamanoJan 5, 2022
  17. builtin/reflog.c: use parse-options api for expire, delete subcommandsJohn Cai via GitGitGadget, Jan 6, 2022
  18. Junio C HamanoJan 6, 2022
  19. John CaiJan 6, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.