* [PATCH 0/2] reflog.c: switch to use parse-options API
@ 2021-12-31 6:29 John Cai via GitGitGadget
2021-12-31 6:29 ` [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand John Cai via GitGitGadget
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: John Cai via GitGitGadget @ 2021-12-31 6:29 UTC (permalink / raw)
To: git; +Cc: John Cai
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.
John Cai (2):
builtin/reflog.c: use parse-options for expire subcommand
builtin/reflog.c: switch to use parse-options API for delete
subcommand
builtin/reflog.c | 122 +++++++++++++++++++++++------------------------
1 file changed, 60 insertions(+), 62 deletions(-)
base-commit: 55b058a8bbcc54bd93c733035c995abc7967e539
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1175%2Fjohn-cai%2Fjc%2Freflog-switch-to-option-parse-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1175/john-cai/jc/reflog-switch-to-option-parse-v1
Pull-Request: https://github.com/git/git/pull/1175
--
gitgitgadget
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand 2021-12-31 6:29 [PATCH 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget @ 2021-12-31 6:29 ` John Cai via GitGitGadget 2022-01-01 2:06 ` John Cai 2021-12-31 6:29 ` [PATCH 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2 siblings, 1 reply; 19+ messages in thread From: John Cai via GitGitGadget @ 2021-12-31 6:29 UTC (permalink / raw) To: git; +Cc: John Cai, John Cai From: John Cai <johncai86@gmail.com> Switch out manual argv parsing for the reflog expire subcommand to use the parse-options API. Signed-off-by: "John Cai" <johncai86@gmail.com> --- builtin/reflog.c | 72 ++++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index 175c83e7cc2..afaf5ba67e2 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -11,13 +11,8 @@ #include "revision.h" #include "reachable.h" #include "worktree.h" +#include "parse-options.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>..."); @@ -539,6 +534,14 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c 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 cmd_reflog_expire(int argc, const char **argv, const char *prefix) { struct expire_reflog_policy_cb cb; @@ -547,6 +550,29 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) int explicit_expiry = 0; unsigned int flags = 0; + 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), + EXPIRE_REFLOGS_VERBOSE), + OPT_EXPIRY_DATE(0, "expire", &cb.cmd.expire_total, + N_("prune entries older than the specified time")), + OPT_EXPIRY_DATE(0, "expire-unreachable", &cb.cmd.expire_unreachable, + N_("prune entries older than <time> that are not reachable from the current tip of the branch")), + OPT_BOOL(0, "stale-fix", &cb.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; git_config(reflog_expire_config, NULL); @@ -560,41 +586,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) 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, &cb.cmd.expire_total)) - die(_("'%s' is not a valid timestamp"), arg); + if (starts_with(arg, "--expire=")) { explicit_expiry |= EXPIRE_TOTAL; - } - else if (skip_prefix(arg, "--expire-unreachable=", &arg)) { - if (parse_expiry_date(arg, &cb.cmd.expire_unreachable)) - die(_("'%s' is not a valid timestamp"), arg); + } else if (starts_with(arg, "--expire-unreachable=")) { explicit_expiry |= EXPIRE_UNREACH; } - else if (!strcmp(arg, "--stale-fix")) - cb.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")) - flags |= EXPIRE_REFLOGS_VERBOSE; - 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); + /* * We can trust the commits and objects reachable from refs * even in older repository. We cannot trust what's reachable -- gitgitgadget ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand 2021-12-31 6:29 ` [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand John Cai via GitGitGadget @ 2022-01-01 2:06 ` John Cai 2022-01-01 11:16 ` René Scharfe 0 siblings, 1 reply; 19+ messages in thread From: John Cai @ 2022-01-01 2:06 UTC (permalink / raw) To: John Cai via GitGitGadget; +Cc: git > On Dec 30, 2021, at 10:29 PM, John Cai via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: John Cai <johncai86@gmail.com> > > Switch out manual argv parsing for the reflog expire subcommand to use > the parse-options API. > > Signed-off-by: "John Cai" <johncai86@gmail.com> > --- > builtin/reflog.c | 72 ++++++++++++++++++++++++------------------------ > 1 file changed, 36 insertions(+), 36 deletions(-) > > diff --git a/builtin/reflog.c b/builtin/reflog.c > index 175c83e7cc2..afaf5ba67e2 100644 > --- a/builtin/reflog.c > +++ b/builtin/reflog.c > @@ -11,13 +11,8 @@ > #include "revision.h" > #include "reachable.h" > #include "worktree.h" > +#include "parse-options.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>..."); > @@ -539,6 +534,14 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c > 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 cmd_reflog_expire(int argc, const char **argv, const char *prefix) > { > struct expire_reflog_policy_cb cb; > @@ -547,6 +550,29 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) > int explicit_expiry = 0; > unsigned int flags = 0; > > + 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), > + EXPIRE_REFLOGS_VERBOSE), > + OPT_EXPIRY_DATE(0, "expire", &cb.cmd.expire_total, > + N_("prune entries older than the specified time")), > + OPT_EXPIRY_DATE(0, "expire-unreachable", &cb.cmd.expire_unreachable, > + N_("prune entries older than <time> that are not reachable from the current tip of the branch")), > + OPT_BOOL(0, "stale-fix", &cb.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; > git_config(reflog_expire_config, NULL); > @@ -560,41 +586,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) > > for (i = 1; i < argc; i++) { I was hoping we could get rid of this for loop altogether, but I couldn’t figure out a clean way since --expire and expire-unreachable take a value __and__ set a flag bit. So I kept this for loop for the sole purpose of setting the explicit_expiry bit flag. Any suggestions? > 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, &cb.cmd.expire_total)) > - die(_("'%s' is not a valid timestamp"), arg); > + if (starts_with(arg, "--expire=")) { > explicit_expiry |= EXPIRE_TOTAL; > - } > - else if (skip_prefix(arg, "--expire-unreachable=", &arg)) { > - if (parse_expiry_date(arg, &cb.cmd.expire_unreachable)) > - die(_("'%s' is not a valid timestamp"), arg); > + } else if (starts_with(arg, "--expire-unreachable=")) { > explicit_expiry |= EXPIRE_UNREACH; > } > - else if (!strcmp(arg, "--stale-fix")) > - cb.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")) > - flags |= EXPIRE_REFLOGS_VERBOSE; > - 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); > + > /* > * We can trust the commits and objects reachable from refs > * even in older repository. We cannot trust what's reachable > -- > gitgitgadget > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand 2022-01-01 2:06 ` John Cai @ 2022-01-01 11:16 ` René Scharfe 2022-01-01 19:09 ` John Cai 0 siblings, 1 reply; 19+ messages in thread From: René Scharfe @ 2022-01-01 11:16 UTC (permalink / raw) To: John Cai, John Cai via GitGitGadget; +Cc: git Am 01.01.22 um 03:06 schrieb John Cai: > > >> On Dec 30, 2021, at 10:29 PM, John Cai via GitGitGadget <gitgitgadget@gmail.com> wrote: >> >> From: John Cai <johncai86@gmail.com> >> >> Switch out manual argv parsing for the reflog expire subcommand to use >> the parse-options API. >> >> Signed-off-by: "John Cai" <johncai86@gmail.com> >> --- >> builtin/reflog.c | 72 ++++++++++++++++++++++++------------------------ >> 1 file changed, 36 insertions(+), 36 deletions(-) >> >> diff --git a/builtin/reflog.c b/builtin/reflog.c >> index 175c83e7cc2..afaf5ba67e2 100644 >> --- a/builtin/reflog.c >> +++ b/builtin/reflog.c >> @@ -11,13 +11,8 @@ >> #include "revision.h" >> #include "reachable.h" >> #include "worktree.h" >> +#include "parse-options.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>..."); >> @@ -539,6 +534,14 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c >> 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 cmd_reflog_expire(int argc, const char **argv, const char *prefix) >> { >> struct expire_reflog_policy_cb cb; >> @@ -547,6 +550,29 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) >> int explicit_expiry = 0; >> unsigned int flags = 0; >> >> + 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), >> + EXPIRE_REFLOGS_VERBOSE), >> + OPT_EXPIRY_DATE(0, "expire", &cb.cmd.expire_total, >> + N_("prune entries older than the specified time")), >> + OPT_EXPIRY_DATE(0, "expire-unreachable", &cb.cmd.expire_unreachable, >> + N_("prune entries older than <time> that are not reachable from the current tip of the branch")), >> + OPT_BOOL(0, "stale-fix", &cb.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; >> git_config(reflog_expire_config, NULL); >> @@ -560,41 +586,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) >> >> for (i = 1; i < argc; i++) { > > I was hoping we could get rid of this for loop altogether, but I > couldn’t figure out a clean way since --expire and > expire-unreachable take a value __and__ set a flag bit. So I kept > this for loop for the sole purpose of setting the explicit_expiry bit > flag. Any suggestions? The problem is that the default value can vary between reflogs and we only know which ones are to be expired after option parsing, right? The easiest way is probably to initialize the date variables to a magic value that is unlikely to be specified explicitly. parse_expiry_date() already uses two such magic values: 0 for "never" and TIME_MAX for "now". Perhaps 1 for "default"? cb.cmd.expire_total = cb.cmd.expire_unreachable = 1; argc = parse_options(...); if (cb.cmd.expire_total == 1) cb.cmd.expire_total = default_reflog_expire; else explicit_expiry |= EXPIRE_TOTAL; if (cb.cmd.expire_unreachable == 1) cb.cmd.expire_unreachable = default_reflog_expire_unreachable; else explicit_expiry |= EXPIRE_UNREACH; A somewhat cleaner approach would be to store that bit separately: struct expire_date { unsigned is_explicitly_set:1; timestamp_t at; }; ... and add a callback function that wraps parse_opt_expiry_date_cb(), expects the new struct (instead of timestamp_t directlly) and sets that bit. > >> 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, &cb.cmd.expire_total)) >> - die(_("'%s' is not a valid timestamp"), arg); >> + if (starts_with(arg, "--expire=")) { >> explicit_expiry |= EXPIRE_TOTAL; >> - } >> - else if (skip_prefix(arg, "--expire-unreachable=", &arg)) { >> - if (parse_expiry_date(arg, &cb.cmd.expire_unreachable)) >> - die(_("'%s' is not a valid timestamp"), arg); >> + } else if (starts_with(arg, "--expire-unreachable=")) { >> explicit_expiry |= EXPIRE_UNREACH; >> } >> - else if (!strcmp(arg, "--stale-fix")) >> - cb.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")) >> - flags |= EXPIRE_REFLOGS_VERBOSE; >> - 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); >> + >> /* >> * We can trust the commits and objects reachable from refs >> * even in older repository. We cannot trust what's reachable >> -- >> gitgitgadget >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand 2022-01-01 11:16 ` René Scharfe @ 2022-01-01 19:09 ` John Cai 2022-01-02 9:00 ` René Scharfe 0 siblings, 1 reply; 19+ messages in thread From: John Cai @ 2022-01-01 19:09 UTC (permalink / raw) To: René Scharfe; +Cc: John Cai via GitGitGadget, git > On Jan 1, 2022, at 3:16 AM, René Scharfe <l.s.r@web.de> wrote: > > Am 01.01.22 um 03:06 schrieb John Cai: >> >> >>> On Dec 30, 2021, at 10:29 PM, John Cai via GitGitGadget <gitgitgadget@gmail.com> wrote: >>> >>> From: John Cai <johncai86@gmail.com> >>> >>> Switch out manual argv parsing for the reflog expire subcommand to use >>> the parse-options API. >>> >>> Signed-off-by: "John Cai" <johncai86@gmail.com> >>> --- >>> builtin/reflog.c | 72 ++++++++++++++++++++++++------------------------ >>> 1 file changed, 36 insertions(+), 36 deletions(-) >>> >>> diff --git a/builtin/reflog.c b/builtin/reflog.c >>> index 175c83e7cc2..afaf5ba67e2 100644 >>> --- a/builtin/reflog.c >>> +++ b/builtin/reflog.c >>> @@ -11,13 +11,8 @@ >>> #include "revision.h" >>> #include "reachable.h" >>> #include "worktree.h" >>> +#include "parse-options.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>..."); >>> @@ -539,6 +534,14 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c >>> 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 cmd_reflog_expire(int argc, const char **argv, const char *prefix) >>> { >>> struct expire_reflog_policy_cb cb; >>> @@ -547,6 +550,29 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) >>> int explicit_expiry = 0; >>> unsigned int flags = 0; >>> >>> + 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), >>> + EXPIRE_REFLOGS_VERBOSE), >>> + OPT_EXPIRY_DATE(0, "expire", &cb.cmd.expire_total, >>> + N_("prune entries older than the specified time")), >>> + OPT_EXPIRY_DATE(0, "expire-unreachable", &cb.cmd.expire_unreachable, >>> + N_("prune entries older than <time> that are not reachable from the current tip of the branch")), >>> + OPT_BOOL(0, "stale-fix", &cb.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; >>> git_config(reflog_expire_config, NULL); >>> @@ -560,41 +586,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) >>> >>> for (i = 1; i < argc; i++) { >> >> I was hoping we could get rid of this for loop altogether, but I >> couldn’t figure out a clean way since --expire and >> expire-unreachable take a value __and__ set a flag bit. So I kept >> this for loop for the sole purpose of setting the explicit_expiry bit >> flag. Any suggestions? > > The problem is that the default value can vary between reflogs and we > only know which ones are to be expired after option parsing, right? That’s a good point. Does it matter that the default value varies between reflogs? Would something like this suffice? - for (i = 1; i < argc; i++) { - const char *arg = argv[i]; - if (starts_with(arg, "--expire=")) { - explicit_expiry |= EXPIRE_TOTAL; - } else if (starts_with(arg, "--expire-unreachable=")) { - explicit_expiry |= EXPIRE_UNREACH; - } - } - argc = parse_options(argc, argv, prefix, options, reflog_expire_usage, 0); + if (cb.cmd.expire_total != default_reflog_expire) + explicit_expiry |= EXPIRE_TOTAL; + if (cb.cmd.expire_unreachable != default_reflog_expire_unreachable) + explicit_expiry |= EXPIRE_UNREACH; > > The easiest way is probably to initialize the date variables to a > magic value that is unlikely to be specified explicitly. > parse_expiry_date() already uses two such magic values: 0 for "never" > and TIME_MAX for "now". Perhaps 1 for "default"? > > cb.cmd.expire_total = cb.cmd.expire_unreachable = 1; > > argc = parse_options(...); > > if (cb.cmd.expire_total == 1) > cb.cmd.expire_total = default_reflog_expire; > else > explicit_expiry |= EXPIRE_TOTAL; > if (cb.cmd.expire_unreachable == 1) > cb.cmd.expire_unreachable = default_reflog_expire_unreachable; > else > explicit_expiry |= EXPIRE_UNREACH; > > A somewhat cleaner approach would be to store that bit separately: > > struct expire_date { > unsigned is_explicitly_set:1; > timestamp_t at; > }; > > ... and add a callback function that wraps parse_opt_expiry_date_cb(), > expects the new struct (instead of timestamp_t directlly) and sets > that bit. > >> >>> 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, &cb.cmd.expire_total)) >>> - die(_("'%s' is not a valid timestamp"), arg); >>> + if (starts_with(arg, "--expire=")) { >>> explicit_expiry |= EXPIRE_TOTAL; >>> - } >>> - else if (skip_prefix(arg, "--expire-unreachable=", &arg)) { >>> - if (parse_expiry_date(arg, &cb.cmd.expire_unreachable)) >>> - die(_("'%s' is not a valid timestamp"), arg); >>> + } else if (starts_with(arg, "--expire-unreachable=")) { >>> explicit_expiry |= EXPIRE_UNREACH; >>> } >>> - else if (!strcmp(arg, "--stale-fix")) >>> - cb.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")) >>> - flags |= EXPIRE_REFLOGS_VERBOSE; >>> - 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); >>> + >>> /* >>> * We can trust the commits and objects reachable from refs >>> * even in older repository. We cannot trust what's reachable >>> -- >>> gitgitgadget ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand 2022-01-01 19:09 ` John Cai @ 2022-01-02 9:00 ` René Scharfe 0 siblings, 0 replies; 19+ messages in thread From: René Scharfe @ 2022-01-02 9:00 UTC (permalink / raw) To: John Cai; +Cc: John Cai via GitGitGadget, git Am 01.01.22 um 20:09 schrieb John Cai: > > >> On Jan 1, 2022, at 3:16 AM, René Scharfe <l.s.r@web.de> wrote: >> >> The problem is that the default value can vary between reflogs and we >> only know which ones are to be expired after option parsing, right? > > That’s a good point. Does it matter that the default value varies between reflogs? > > Would something like this suffice? > > - for (i = 1; i < argc; i++) { > - const char *arg = argv[i]; > - if (starts_with(arg, "--expire=")) { > - explicit_expiry |= EXPIRE_TOTAL; > - } else if (starts_with(arg, "--expire-unreachable=")) { > - explicit_expiry |= EXPIRE_UNREACH; > - } > - } > - > argc = parse_options(argc, argv, prefix, options, reflog_expire_usage, 0); > > + if (cb.cmd.expire_total != default_reflog_expire) > + explicit_expiry |= EXPIRE_TOTAL; > + if (cb.cmd.expire_unreachable != default_reflog_expire_unreachable) > + explicit_expiry |= EXPIRE_UNREACH; This would ignore --expire and -expire-unreachable options with the value of default_reflog_expire and default_reflog_expire_unreachable, respectively. E.g. "git reflog expire --expire=90.days.ago refs/stash" would not expire anything. René ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand 2021-12-31 6:29 [PATCH 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2021-12-31 6:29 ` [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand John Cai via GitGitGadget @ 2021-12-31 6:29 ` John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2 siblings, 0 replies; 19+ messages in thread From: John Cai via GitGitGadget @ 2021-12-31 6:29 UTC (permalink / raw) To: git; +Cc: John Cai, John Cai From: John Cai <johncai86@gmail.com> Address NEEDSWORK by switching out manual arg parsing for the parse-options API for the delete subcommand. Signed-off-by: "John Cai" <johncai86@gmail.com> --- builtin/reflog.c | 52 +++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index afaf5ba67e2..a405c38a139 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -11,11 +11,7 @@ #include "revision.h" #include "reachable.h" #include "worktree.h" -#include "parse-options.h" -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>"); @@ -641,7 +637,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) free(collected.e); } - for (; i < argc; i++) { + for (i = 0; i < argc; i++) { char *ref; if (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) { status |= error(_("%s points nowhere!"), argv[i]); @@ -668,38 +664,40 @@ 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 expire_reflog_policy_cb cb; - int i, status = 0; + int status = 0; unsigned int flags = 0; + 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), + EXPIRE_REFLOGS_VERBOSE), + OPT_END() + }; + memset(&cb, 0, sizeof(cb)); - 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")) - flags |= EXPIRE_REFLOGS_VERBOSE; - else if (!strcmp(arg, "--")) { - i++; - break; - } - else if (arg[0] == '-') - usage(_(reflog_delete_usage)); - else - break; - } + argc = parse_options(argc, argv, prefix, options, reflog_delete_usage, 0); - if (argc - i < 1) + if (argc < 1) return error(_("no reflog specified to delete")); - for ( ; i < argc; i++) { + for (int i = 0; i < argc; i++) { const char *spec = strstr(argv[i], "@{"); char *ep, *ref; int recno; -- gitgitgadget ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 0/2] reflog.c: switch to use parse-options API 2021-12-31 6:29 [PATCH 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2021-12-31 6:29 ` [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand John Cai via GitGitGadget 2021-12-31 6:29 ` [PATCH 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand John Cai via GitGitGadget @ 2022-01-03 15:20 ` John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper John Cai via GitGitGadget ` (2 more replies) 2 siblings, 3 replies; 19+ messages in thread From: John Cai via GitGitGadget @ 2022-01-03 15:20 UTC (permalink / raw) To: git; +Cc: René Scharfe, John Cai 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. changes since v1: * add helper parse_opt_expiry_date so we can use this shared logic to parse the expire date option. * place explicit_expiry inside of cmd_reflog_expire_cb to make it easier to use a callback to set both the value of the timestamp and the explicit_expiry flag. John Cai (2): parse-options.h: add parse_opt_expiry_date helper builtin/reflog.c: switch to use parse-options API for delete subcommand builtin/reflog.c | 168 +++++++++++++++++++++++---------------------- parse-options-cb.c | 7 +- parse-options.h | 1 + 3 files changed, 94 insertions(+), 82 deletions(-) base-commit: 55b058a8bbcc54bd93c733035c995abc7967e539 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1175%2Fjohn-cai%2Fjc%2Freflog-switch-to-option-parse-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1175/john-cai/jc/reflog-switch-to-option-parse-v2 Pull-Request: https://github.com/git/git/pull/1175 Range-diff vs v1: 1: 9bd3c6672c4 < -: ----------- builtin/reflog.c: use parse-options for expire subcommand -: ----------- > 1: bcd74559c24 parse-options.h: add parse_opt_expiry_date helper 2: fffea298642 ! 2: f9de21e0f26 builtin/reflog.c: switch to use parse-options API for delete subcommand @@ Commit message Address NEEDSWORK by switching out manual arg parsing for the parse-options API for the delete subcommand. + Moves explicit_expiry flag into cmd_reflog_expire_cb struct so a + callback can set both the value of the expiration as well as the + explicit_expiry flag. + Signed-off-by: "John Cai" <johncai86@gmail.com> ## builtin/reflog.c ## @@ - #include "revision.h" #include "reachable.h" #include "worktree.h" --#include "parse-options.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>"); +@@ builtin/reflog.c: static timestamp_t default_reflog_expire_unreachable; + struct cmd_reflog_expire_cb { + struct rev_info revs; + int stalefix; ++ int explicit_expiry; + timestamp_t expire_total; + timestamp_t expire_unreachable; + int recno; +@@ builtin/reflog.c: 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; + } +@@ builtin/reflog.c: 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; ++ cmd->explicit_expiry |= EXPIRE_UNREACH; ++ return parse_opt_expiry_date(&cmd->expire_unreachable, arg, unset); ++} ++ ++static int expire_total_callback(const struct option *opt, ++ const char *arg, ++ int unset) ++{ ++ struct cmd_reflog_expire_cb *cmd = opt->value; ++ cmd->explicit_expiry |= EXPIRE_TOTAL; ++ return parse_opt_expiry_date(&cmd->expire_total, arg, unset); ++} ++ + static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) + { + struct expire_reflog_policy_cb cb; + timestamp_t now = time(NULL); + int i, status, do_all, all_worktrees = 1; +- int explicit_expiry = 0; + unsigned int flags = 0; ++ 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), ++ EXPIRE_REFLOGS_VERBOSE), ++ OPT_CALLBACK(0, "expire", &cb.cmd, N_("timestamp"), ++ N_("prune entries older than the specified time"), expire_total_callback), ++ OPT_CALLBACK(0, "expire-unreachable", &cb.cmd, N_("timestamp"), ++ N_("prune entries older than <time> that are not reachable from the current tip of the branch"), ++ expire_unreachable_callback), ++ OPT_BOOL(0, "stale-fix", &cb.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; +@@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) + do_all = status = 0; + memset(&cb, 0, sizeof(cb)); + ++ cb.cmd.explicit_expiry = 0; + cb.cmd.expire_total = default_reflog_expire; + cb.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, &cb.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, &cb.cmd.expire_unreachable)) +- die(_("'%s' is not a valid timestamp"), arg); +- explicit_expiry |= EXPIRE_UNREACH; +- } +- else if (!strcmp(arg, "--stale-fix")) +- cb.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")) +- flags |= EXPIRE_REFLOGS_VERBOSE; +- 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); + /* + * We can trust the commits and objects reachable from refs + * even in older repository. We cannot trust what's reachable +@@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) + for (i = 0; i < collected.nr; i++) { + struct collected_reflog *e = collected.e[i]; + +- set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog); ++ set_reflog_expiry_param(&cb.cmd, e->reflog); + status |= reflog_expire(e->reflog, flags, + reflog_expiry_prepare, + should_expire_reflog_ent, @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) free(collected.e); } @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons char *ref; if (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) { 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_expire_reflog_ent, @@ builtin/reflog.c: static int count_reflog_ent(struct object_id *ooid, struct object_id *noid, return 0; } -- gitgitgadget ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper 2022-01-03 15:20 ` [PATCH v2 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget @ 2022-01-03 15:20 ` John Cai via GitGitGadget 2022-01-04 2:26 ` Junio C Hamano 2022-01-03 15:20 ` [PATCH v2 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand John Cai via GitGitGadget 2022-01-04 17:41 ` [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands John Cai via GitGitGadget 2 siblings, 1 reply; 19+ messages in thread From: John Cai via GitGitGadget @ 2022-01-03 15:20 UTC (permalink / raw) To: git; +Cc: René Scharfe, John Cai, John Cai From: John Cai <johncai86@gmail.com> Extract the logic in parse_opt_expiry_date_cb into a helper parse_opt_expiry_date. This is to prepare for the following commit where we need to parse an expiry date that gets passed into a callback function in opt->value as part of a struct. The next commit will utilize this helper in a callback function that aims to wrap the functionality in parse_opt_expiry_date_cb. Signed-off-by: "John Cai" <johncai86@gmail.com> --- parse-options-cb.c | 7 ++++++- parse-options.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/parse-options-cb.c b/parse-options-cb.c index 3c811e1e4a7..3edb88a54d8 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -34,10 +34,15 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) int parse_opt_expiry_date_cb(const struct option *opt, const char *arg, int unset) +{ + return parse_opt_expiry_date((timestamp_t *)opt->value, arg, unset); +} + +int parse_opt_expiry_date(timestamp_t *t, const char *arg, int unset) { if (unset) arg = "never"; - if (parse_expiry_date(arg, (timestamp_t *)opt->value)) + if (parse_expiry_date(arg, t)) die(_("malformed expiration date '%s'"), arg); return 0; } diff --git a/parse-options.h b/parse-options.h index 275fb440818..0a15bac8619 100644 --- a/parse-options.h +++ b/parse-options.h @@ -301,6 +301,7 @@ enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const char *, int); int parse_opt_passthru(const struct option *, const char *, int); int parse_opt_passthru_argv(const struct option *, const char *, int); +int parse_opt_expiry_date(timestamp_t *, const char *, int); #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) #define OPT__QUIET(var, h) OPT_COUNTUP('q', "quiet", (var), (h)) -- gitgitgadget ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper 2022-01-03 15:20 ` [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper John Cai via GitGitGadget @ 2022-01-04 2:26 ` Junio C Hamano 0 siblings, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2022-01-04 2:26 UTC (permalink / raw) To: John Cai via GitGitGadget; +Cc: git, René Scharfe, John Cai "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: John Cai <johncai86@gmail.com> > > Extract the logic in parse_opt_expiry_date_cb into a helper > parse_opt_expiry_date. This is to prepare for the following commit where > we need to parse an expiry date that gets passed into a callback > function in opt->value as part of a struct. > > The next commit will utilize this helper in a callback function that > aims to wrap the functionality in parse_opt_expiry_date_cb. > > Signed-off-by: "John Cai" <johncai86@gmail.com> > --- > parse-options-cb.c | 7 ++++++- > parse-options.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/parse-options-cb.c b/parse-options-cb.c > index 3c811e1e4a7..3edb88a54d8 100644 > --- a/parse-options-cb.c > +++ b/parse-options-cb.c > @@ -34,10 +34,15 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) > > int parse_opt_expiry_date_cb(const struct option *opt, const char *arg, > int unset) > +{ > + return parse_opt_expiry_date((timestamp_t *)opt->value, arg, unset); > +} > + > +int parse_opt_expiry_date(timestamp_t *t, const char *arg, int unset) > { > if (unset) > arg = "never"; > - if (parse_expiry_date(arg, (timestamp_t *)opt->value)) > + if (parse_expiry_date(arg, t)) > die(_("malformed expiration date '%s'"), arg); > return 0; > } Does this even belong to parse-options-cb.c file, though? It's interface tells us that it is not limited to the parse-options infrastructure (i.e. it does not even take "struct option" at all). I am not sure how having this new function will be helpful to begin with. We'll see why it helps by looking at the next step, I presume, but I wonder if the new caller of this function should just call parse_expiry_date() itself (with "unset means 'never'"). > diff --git a/parse-options.h b/parse-options.h > index 275fb440818..0a15bac8619 100644 > --- a/parse-options.h > +++ b/parse-options.h > @@ -301,6 +301,7 @@ enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, > const char *, int); > int parse_opt_passthru(const struct option *, const char *, int); > int parse_opt_passthru_argv(const struct option *, const char *, int); > +int parse_opt_expiry_date(timestamp_t *, const char *, int); > > #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) > #define OPT__QUIET(var, h) OPT_COUNTUP('q', "quiet", (var), (h)) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand 2022-01-03 15:20 ` [PATCH v2 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper John Cai via GitGitGadget @ 2022-01-03 15:20 ` John Cai via GitGitGadget 2022-01-04 2:36 ` Junio C Hamano 2022-01-04 17:41 ` [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands John Cai via GitGitGadget 2 siblings, 1 reply; 19+ messages in thread From: John Cai via GitGitGadget @ 2022-01-03 15:20 UTC (permalink / raw) To: git; +Cc: René Scharfe, John Cai, John Cai From: John Cai <johncai86@gmail.com> Address NEEDSWORK by switching out manual arg parsing for the parse-options API for the delete subcommand. Moves explicit_expiry flag into cmd_reflog_expire_cb struct so a callback can set both the value of the expiration as well as the explicit_expiry flag. Signed-off-by: "John Cai" <johncai86@gmail.com> --- builtin/reflog.c | 168 ++++++++++++++++++++++++----------------------- 1 file changed, 87 insertions(+), 81 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index 175c83e7cc2..3552d749e4b 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>"); @@ -30,6 +21,7 @@ static timestamp_t default_reflog_expire_unreachable; struct cmd_reflog_expire_cb { struct rev_info revs; int stalefix; + int explicit_expiry; timestamp_t expire_total; timestamp_t expire_unreachable; int recno; @@ -504,18 +496,17 @@ 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; } @@ -525,27 +516,75 @@ 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; + cmd->explicit_expiry |= EXPIRE_UNREACH; + return parse_opt_expiry_date(&cmd->expire_unreachable, arg, unset); +} + +static int expire_total_callback(const struct option *opt, + const char *arg, + int unset) +{ + struct cmd_reflog_expire_cb *cmd = opt->value; + cmd->explicit_expiry |= EXPIRE_TOTAL; + return parse_opt_expiry_date(&cmd->expire_total, arg, unset); +} + static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) { struct expire_reflog_policy_cb cb; timestamp_t now = time(NULL); int i, status, do_all, all_worktrees = 1; - int explicit_expiry = 0; unsigned int flags = 0; + 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), + EXPIRE_REFLOGS_VERBOSE), + OPT_CALLBACK(0, "expire", &cb.cmd, N_("timestamp"), + N_("prune entries older than the specified time"), expire_total_callback), + OPT_CALLBACK(0, "expire-unreachable", &cb.cmd, N_("timestamp"), + N_("prune entries older than <time> that are not reachable from the current tip of the branch"), + expire_unreachable_callback), + OPT_BOOL(0, "stale-fix", &cb.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; @@ -555,46 +594,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) do_all = status = 0; memset(&cb, 0, sizeof(cb)); + cb.cmd.explicit_expiry = 0; cb.cmd.expire_total = default_reflog_expire; cb.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, &cb.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, &cb.cmd.expire_unreachable)) - die(_("'%s' is not a valid timestamp"), arg); - explicit_expiry |= EXPIRE_UNREACH; - } - else if (!strcmp(arg, "--stale-fix")) - cb.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")) - flags |= EXPIRE_REFLOGS_VERBOSE; - 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); /* * We can trust the commits and objects reachable from refs * even in older repository. We cannot trust what's reachable @@ -630,7 +634,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) for (i = 0; i < collected.nr; i++) { struct collected_reflog *e = collected.e[i]; - set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog); + set_reflog_expiry_param(&cb.cmd, e->reflog); status |= reflog_expire(e->reflog, flags, reflog_expiry_prepare, should_expire_reflog_ent, @@ -641,13 +645,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) free(collected.e); } - for (; i < argc; i++) { + for (i = 0; i < argc; i++) { char *ref; if (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) { 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_expire_reflog_ent, @@ -668,38 +672,40 @@ 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 expire_reflog_policy_cb cb; - int i, status = 0; + int status = 0; unsigned int flags = 0; + 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), + EXPIRE_REFLOGS_VERBOSE), + OPT_END() + }; + memset(&cb, 0, sizeof(cb)); - 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")) - flags |= EXPIRE_REFLOGS_VERBOSE; - else if (!strcmp(arg, "--")) { - i++; - break; - } - else if (arg[0] == '-') - usage(_(reflog_delete_usage)); - else - break; - } + argc = parse_options(argc, argv, prefix, options, reflog_delete_usage, 0); - if (argc - i < 1) + if (argc < 1) return error(_("no reflog specified to delete")); - for ( ; i < argc; i++) { + for (int i = 0; i < argc; i++) { const char *spec = strstr(argv[i], "@{"); char *ep, *ref; int recno; -- gitgitgadget ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand 2022-01-03 15:20 ` [PATCH v2 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand John Cai via GitGitGadget @ 2022-01-04 2:36 ` Junio C Hamano 0 siblings, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2022-01-04 2:36 UTC (permalink / raw) To: John Cai via GitGitGadget; +Cc: git, René Scharfe, John Cai "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: John Cai <johncai86@gmail.com> > > Address NEEDSWORK by switching out manual arg parsing for the > parse-options API for the delete subcommand. > > Moves explicit_expiry flag into cmd_reflog_expire_cb struct so a > callback can set both the value of the expiration as well as the > explicit_expiry flag. > > Signed-off-by: "John Cai" <johncai86@gmail.com> > --- > builtin/reflog.c | 168 ++++++++++++++++++++++++----------------------- > 1 file changed, 87 insertions(+), 81 deletions(-) > > diff --git a/builtin/reflog.c b/builtin/reflog.c > index 175c83e7cc2..3552d749e4b 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>"); > > @@ -30,6 +21,7 @@ static timestamp_t default_reflog_expire_unreachable; > struct cmd_reflog_expire_cb { > struct rev_info revs; > int stalefix; > + int explicit_expiry; > timestamp_t expire_total; > timestamp_t expire_unreachable; > int recno; > @@ -504,18 +496,17 @@ 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; > } > @@ -525,27 +516,75 @@ 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; > } OK. > +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; > + cmd->explicit_expiry |= EXPIRE_UNREACH; > + return parse_opt_expiry_date(&cmd->expire_unreachable, arg, unset); Just as I suspected in the previous step. Get rid of [1/2] and add the new helper as a static function to this file. The thing is that we shouldn't confuse future developers by adding a random function that cannot be used as OPTION_CALLBACK function in the parse-options-cb.c file. > + for (int i = 0; i < argc; i++) { Documentation/CodingGuidelines: - Declaring a variable in the for loop "for (int i = 0; i < 10; i++)" is still not allowed in this codebase. We have floated a weather balloon by adding a single use of this construct so that when somebody finds a compiler that do not like the construct it is easy to revert, and waiting for a feedback. We do not want to add the use of the construct to random places yet that makes us scramble and revert them from all over the place. > const char *spec = strstr(argv[i], "@{"); > char *ep, *ref; > int recno; ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands 2022-01-03 15:20 ` [PATCH v2 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand John Cai via GitGitGadget @ 2022-01-04 17:41 ` John Cai via GitGitGadget 2022-01-04 22:04 ` Junio C Hamano 2022-01-05 4:06 ` [PATCH v4] " John Cai via GitGitGadget 2 siblings, 2 replies; 19+ messages in thread From: John Cai via GitGitGadget @ 2022-01-04 17:41 UTC (permalink / raw) To: git; +Cc: René Scharfe, John Cai, John Cai 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. 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-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1175/john-cai/jc/reflog-switch-to-option-parse-v3 Pull-Request: https://github.com/git/git/pull/1175 Range-diff vs v2: 1: bcd74559c24 < -: ----------- parse-options.h: add parse_opt_expiry_date helper 2: f9de21e0f26 ! 1: 99a97e6ee52 builtin/reflog.c: switch to use parse-options API for delete subcommand @@ Metadata Author: John Cai <johncai86@gmail.com> ## Commit message ## - builtin/reflog.c: switch to use parse-options API for delete subcommand + builtin/reflog.c: use parse-options api for expire, delete subcommands - Address NEEDSWORK by switching out manual arg parsing for the - parse-options API for the delete subcommand. + Switching out manual arg parsing for the parse-options API for the + expire and delete subcommands. - Moves explicit_expiry flag into cmd_reflog_expire_cb struct so a - callback can set both the value of the expiration as well as the - explicit_expiry flag. + 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> @@ builtin/reflog.c N_("git reflog exists <ref>"); @@ builtin/reflog.c: static timestamp_t default_reflog_expire_unreachable; + struct cmd_reflog_expire_cb { - struct rev_info revs; int stalefix; + int explicit_expiry; timestamp_t expire_total; @@ builtin/reflog.c: static int reflog_expire_config(const char *var, const char *v +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 */ @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c + 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 parse_opt_expiry_date(&cmd->expire_unreachable, arg, unset); ++ return 0; +} + +static int expire_total_callback(const struct option *opt, @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c + 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 parse_opt_expiry_date(&cmd->expire_total, arg, unset); ++ return 0; +} + static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) { - struct expire_reflog_policy_cb cb; + 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), ++ 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), ++ 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), -+ EXPIRE_REFLOGS_VERBOSE), -+ OPT_CALLBACK(0, "expire", &cb.cmd, N_("timestamp"), -+ N_("prune entries older than the specified time"), expire_total_callback), -+ OPT_CALLBACK(0, "expire-unreachable", &cb.cmd, N_("timestamp"), ++ 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", &cb.cmd.stalefix, -+ N_("prune any reflog entries that point to broken commits")), ++ 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.")), ++ 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; @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) + save_commit_buffer = 0; do_all = status = 0; - memset(&cb, 0, sizeof(cb)); -+ cb.cmd.explicit_expiry = 0; - cb.cmd.expire_total = default_reflog_expire; - cb.cmd.expire_unreachable = default_reflog_expire_unreachable; ++ 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]; @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons - 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, &cb.cmd.expire_total)) +- 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, &cb.cmd.expire_unreachable)) +- 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")) -- cb.cmd.stalefix = 1; +- cmd.stalefix = 1; - else if (!strcmp(arg, "--rewrite")) - flags |= EXPIRE_REFLOGS_REWRITE; - else if (!strcmp(arg, "--updateref")) @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons - else if (!strcmp(arg, "--single-worktree")) - all_worktrees = 0; - else if (!strcmp(arg, "--verbose")) -- flags |= EXPIRE_REFLOGS_VERBOSE; +- verbose = 1; - else if (!strcmp(arg, "--")) { - i++; - break; @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons - else - break; - } -- + argc = parse_options(argc, argv, prefix, options, reflog_expire_usage, 0); - /* - * We can trust the commits and objects reachable from refs - * even in older repository. We cannot trust what's reachable + + if (verbose) + should_prune_fn = should_expire_reflog_ent_verbose; @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) - for (i = 0; i < collected.nr; i++) { - struct collected_reflog *e = collected.e[i]; + .dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN), + }; -- set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog); -+ set_reflog_expiry_param(&cb.cmd, e->reflog); - status |= reflog_expire(e->reflog, flags, +- 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_expire_reflog_ent, + should_prune_fn, @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) - free(collected.e); + string_list_clear(&collected.reflogs, 0); } - for (; i < argc; i++) { + for (i = 0; i < argc; i++) { char *ref; - if (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) { + struct expire_reflog_policy_cb cb = { .cmd = cmd }; + +@@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) status |= error(_("%s points nowhere!"), argv[i]); continue; } @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons + set_reflog_expiry_param(&cb.cmd, ref); status |= reflog_expire(ref, flags, reflog_expiry_prepare, - should_expire_reflog_ent, + should_prune_fn, @@ builtin/reflog.c: static int count_reflog_ent(struct object_id *ooid, struct object_id *noid, return 0; } @@ builtin/reflog.c: static int count_reflog_ent(struct object_id *ooid, struct obj + static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) { - struct expire_reflog_policy_cb cb; -- int i, status = 0; -+ int status = 0; + struct cmd_reflog_expire_cb cmd = { 0 }; +@@ builtin/reflog.c: static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) unsigned int flags = 0; - -+ 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_BIT(0, "verbose", &flags, N_("print extra information on screen."), -+ EXPIRE_REFLOGS_VERBOSE), -+ OPT_END() -+ }; -+ - memset(&cb, 0, sizeof(cb)); - + 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")) @@ builtin/reflog.c: static int count_reflog_ent(struct object_id *ooid, struct obj - else if (!strcmp(arg, "--updateref")) - flags |= EXPIRE_REFLOGS_UPDATE_REF; - else if (!strcmp(arg, "--verbose")) -- flags |= EXPIRE_REFLOGS_VERBOSE; +- verbose = 1; - else if (!strcmp(arg, "--")) { - i++; - break; @@ builtin/reflog.c: static int count_reflog_ent(struct object_id *ooid, struct obj - 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 (int i = 0; i < argc; i++) { ++ for (i = 0; i < argc; i++) { const char *spec = strstr(argv[i], "@{"); char *ep, *ref; int recno; builtin/reflog.c | 174 ++++++++++++++++++++++++++--------------------- 1 file changed, 95 insertions(+), 79 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index a4b1dd27e13..7f92fd6f7f7 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands 2022-01-04 17:41 ` [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands John Cai via GitGitGadget @ 2022-01-04 22:04 ` Junio C Hamano 2022-01-05 4:06 ` [PATCH v4] " John Cai via GitGitGadget 1 sibling, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2022-01-04 22:04 UTC (permalink / raw) To: John Cai via GitGitGadget; +Cc: git, René Scharfe, John Cai "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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> Makes sense. > -/* 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_expire_usage[] = { > + N_("git reflog expire [--expire=<time>] " > + "[--expire-unreachable=<time>] " > + "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] " > + "[--verbose] [--all] <refs>..."), Funny indentation? Let's align the opening double-quote of all the other lines to that of the first line on a fixed-column terminal, like was done in the original. > 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[] = { This does not seem to apply. Please leave a note on how the 'base' was chosen when sending a patch that requires something not yet in 'master'. I've based the topic for this patch by prepar ng a merge of 'ab/reflog-prep' into 'master', at least for now. Other than that, looks nicely done. The new approach seems much simpler and sensible than the previous round. Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4] builtin/reflog.c: use parse-options api for expire, delete subcommands 2022-01-04 17:41 ` [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands John Cai via GitGitGadget 2022-01-04 22:04 ` Junio C Hamano @ 2022-01-05 4:06 ` John Cai via GitGitGadget 2022-01-05 20:34 ` Junio C Hamano 2022-01-06 19:06 ` [PATCH v5] " John Cai via GitGitGadget 1 sibling, 2 replies; 19+ messages in thread From: John Cai via GitGitGadget @ 2022-01-05 4:06 UTC (permalink / raw) To: git; +Cc: René Scharfe, John Cai, John Cai 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4] builtin/reflog.c: use parse-options api for expire, delete subcommands 2022-01-05 4:06 ` [PATCH v4] " John Cai via GitGitGadget @ 2022-01-05 20:34 ` Junio C Hamano 2022-01-06 19:06 ` [PATCH v5] " John Cai via GitGitGadget 1 sibling, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2022-01-05 20:34 UTC (permalink / raw) To: John Cai via GitGitGadget; +Cc: git, René Scharfe, John Cai "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes: > +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); Was there a reason to indent this overly deep? The same for the other die() we can see below. > + 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); We used to say "'%s' is not a valid timestamp". The same for the other die() we saw earlier. "expiration date" is more specific to "timestamp" and in that sense, the new message might be an improvement, but if we were to improve messages better, perhaps we should go one step further and tell the user which option got an invalid value, e.g. die(_("invalid timestamp '%s' given to '--%s'"), arg, opt->long_name); perhaps. > 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.")), Somebody else might suggest OPT__VERBOSE() or OPT__VERBOSITY(), but for the purpose of this "use parse-options" topic, a simple BOOL is sufficient. When we start supporting different levels of verbosity, we can switch to more featureful variant (and change the behaviour). > +static const char * reflog_delete_usage[] = { > + N_("git reflog delete [--rewrite] [--updateref] " > + "[--dry-run | -n] [--verbose] <refs>..."), Funny indentation still here? > + 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; > + const struct option options[] = { > + OPT_BIT(0, "dry-run", &flags, N_("do not actually prune any entries"), > + EXPIRE_REFLOGS_DRY_RUN), This is formatted quite differently from the one in reflog_expire(). Be consistent and make sure each line of what's inside "OPT_BIT(...)" begins at the same column, e.g. OPT_BIT(0, "dry-run", &flags, N_("do not actually prune any entries"), EXPIRE_REFLOGS_DRY_RUN), > + 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")); Nice. > - for ( ; i < argc; i++) { > + for (i = 0; i < argc; i++) { > const char *spec = strstr(argv[i], "@{"); > char *ep, *ref; > int recno; Nice, too. Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5] builtin/reflog.c: use parse-options api for expire, delete subcommands 2022-01-05 4:06 ` [PATCH v4] " John Cai via GitGitGadget 2022-01-05 20:34 ` Junio C Hamano @ 2022-01-06 19:06 ` John Cai via GitGitGadget 2022-01-06 21:09 ` Junio C Hamano 1 sibling, 1 reply; 19+ messages in thread From: John Cai via GitGitGadget @ 2022-01-06 19:06 UTC (permalink / raw) To: git; +Cc: René Scharfe, John Cai, John Cai 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 v4: * fixed indentation of option blocks * adjusted error message for invalid timestamps 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-v5 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1175/john-cai/jc/reflog-switch-to-option-parse-v5 Pull-Request: https://github.com/git/git/pull/1175 Range-diff vs v4: 1: 0efdf3d2cb1 ! 1: 9bb4de0f543 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 + struct cmd_reflog_expire_cb *cmd = opt->value; + + if (parse_expiry_date(arg, &cmd->expire_unreachable)) -+ die(_("malformed expiration date '%s'"), arg); ++ die(_("invalid timestamp '%s' given to '--%s'"), ++ arg, opt->long_name); + + cmd->explicit_expiry |= EXPIRE_UNREACH; + return 0; @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c + struct cmd_reflog_expire_cb *cmd = opt->value; + + if (parse_expiry_date(arg, &cmd->expire_total)) -+ die(_("malformed expiration date '%s'"), arg); ++ die(_("invalid timestamp '%s' given to '--%s'"), ++ arg, opt->long_name); + + cmd->explicit_expiry |= EXPIRE_TOTAL; + return 0; @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c + 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, ++ 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, @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c + 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), ++ 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), ++ 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")), ++ 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.")), ++ N_("limits processing to reflogs from the current worktree only.")), + OPT_END() + }; @@ builtin/reflog.c: static int count_reflog_ent(struct object_id *ooid, struct obj +static const char * reflog_delete_usage[] = { + N_("git reflog delete [--rewrite] [--updateref] " -+ "[--dry-run | -n] [--verbose] <refs>..."), ++ "[--dry-run | -n] [--verbose] <refs>..."), + NULL +}; + @@ builtin/reflog.c: static int cmd_reflog_delete(int argc, const char **argv, cons - } + const struct option options[] = { + OPT_BIT(0, "dry-run", &flags, N_("do not actually prune any entries"), -+ EXPIRE_REFLOGS_DRY_RUN), ++ 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), ++ 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), ++ 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() + }; builtin/reflog.c | 176 ++++++++++++++++++++++++++--------------------- 1 file changed, 97 insertions(+), 79 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index a4b1dd27e13..644f29cc49e 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,89 @@ 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(_("invalid timestamp '%s' given to '--%s'"), + arg, opt->long_name); + + 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(_("invalid timestamp '%s' given to '--%s'"), + arg, opt->long_name); + + 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 +624,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 +675,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 +685,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 +693,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 +714,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 +727,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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v5] builtin/reflog.c: use parse-options api for expire, delete subcommands 2022-01-06 19:06 ` [PATCH v5] " John Cai via GitGitGadget @ 2022-01-06 21:09 ` Junio C Hamano 2022-01-06 21:32 ` John Cai 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2022-01-06 21:09 UTC (permalink / raw) To: John Cai via GitGitGadget; +Cc: git, René Scharfe, John Cai "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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> > --- Thanks. Will queue. > @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c > + 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, > ++ OPT_BIT(0, "rewrite", &flags, > + N_("rewrite the old SHA1 with the new SHA1 of the entry that now precedes it"), I think this change is a fat-finger; will correct while queuing. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5] builtin/reflog.c: use parse-options api for expire, delete subcommands 2022-01-06 21:09 ` Junio C Hamano @ 2022-01-06 21:32 ` John Cai 0 siblings, 0 replies; 19+ messages in thread From: John Cai @ 2022-01-06 21:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: John Cai via GitGitGadget, git, René Scharfe On 6 Jan 2022, at 16:09, Junio C Hamano wrote: > "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> 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> >> --- > > Thanks. Will queue. > >> @@ builtin/reflog.c: static void set_reflog_expiry_param(struct cmd_reflog_expire_c >> + 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, >> ++ OPT_BIT(0, "rewrite", &flags, >> + N_("rewrite the old SHA1 with the new SHA1 of the entry that now precedes it"), > > I think this change is a fat-finger; will correct while queuing. Oops, yes. Thanks for that and helping me through the review process :) ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-01-06 21:32 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-31 6:29 [PATCH 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2021-12-31 6:29 ` [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand John Cai via GitGitGadget 2022-01-01 2:06 ` John Cai 2022-01-01 11:16 ` René Scharfe 2022-01-01 19:09 ` John Cai 2022-01-02 9:00 ` René Scharfe 2021-12-31 6:29 ` [PATCH 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 0/2] reflog.c: switch to use parse-options API John Cai via GitGitGadget 2022-01-03 15:20 ` [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper John Cai via GitGitGadget 2022-01-04 2:26 ` Junio C Hamano 2022-01-03 15:20 ` [PATCH v2 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand John Cai via GitGitGadget 2022-01-04 2:36 ` Junio C Hamano 2022-01-04 17:41 ` [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands John Cai via GitGitGadget 2022-01-04 22:04 ` Junio C Hamano 2022-01-05 4:06 ` [PATCH v4] " John Cai via GitGitGadget 2022-01-05 20:34 ` Junio C Hamano 2022-01-06 19:06 ` [PATCH v5] " John Cai via GitGitGadget 2022-01-06 21:09 ` Junio C Hamano 2022-01-06 21:32 ` John Cai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.