reflog.c: switch to use parse-options API
cover 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.
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
Signature · 1 line
gitgitgadget
PATCH 1/2 builtin/reflog.c: use parse-options for expire subcommand
From: John Cai <[email protected]>
Switch out manual argv parsing for the reflog expire subcommand to use the parse-options API.
Signed-off-by "John Cai" <[email protected]>
--- builtin/reflog.c | 72 ++++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 36 deletions(-)
builtin/reflog.c +36 -36
@@ -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
Signature · 1 line
gitgitgadget
Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand
Quoted 79 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Switch out manual argv parsing for the reflog expire subcommand to use the parse-options API.
Signed-off-by "John Cai" <[email protected]>---
builtin/reflog.c| 72 ++++++++++++++++++++++++------------------------1 file changed, 36 insertions(+), 36 deletions(-)builtin/reflog.c +1
-6@@ -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>...");builtin/reflog.c +31
-0@@ -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 );
builtin/reflog.c static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) · line 587
Quoted 2 lines
builtin/reflog.c +0
-0@@ -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?
Quoted 46 lines
builtin/reflog.c +4
-30const 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 Signature · 1 line
gitgitgadget
Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand
Am 01.01.22 um 03:06 schrieb John Cai:
Quoted 80 lines
Quoted 80 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Switch out manual argv parsing for the reflog expire subcommand to use the parse-options API.
Signed-off-by "John Cai" <[email protected]>---
builtin/reflog.c| 72 ++++++++++++++++++++++++------------------------1 file changed, 36 insertions(+), 36 deletions(-)builtin/reflog.c +1
-6@@ -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>...");builtin/reflog.c +31
-0@@ -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)
Quoted 7 lines
builtin/reflog.c static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) · line 587
Quoted 1 line
builtin/reflog.c +0
-0for (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 theexplicit_expirybit 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.
Quoted 48 lines
Quoted 47 lines
builtin/reflog.c +4
-31const 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
Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand
Quoted 93 lines · René Scharfe
Am 01.01.22 um 03:06 schrieb John Cai:
Quoted 80 lines
Quoted 80 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Switch out manual argv parsing for the reflog expire subcommand to use the parse-options API.
Signed-off-by "John Cai" <[email protected]>---
builtin/reflog.c| 72 ++++++++++++++++++++++++------------------------1 file changed, 36 insertions(+), 36 deletions(-)builtin/reflog.c +1
-6@@ -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>...");builtin/reflog.c +31
-0@@ -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)Quoted 7 lines
builtin/reflog.c static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) · line 587
Quoted 1 line
builtin/reflog.c +0
-0for (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 theexplicit_expirybit flag. Any suggestions?
Quoted 2 lines
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;
Quoted 76 lines
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" andTIME_MAXfor "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 oftimestamp_tdirectlly) and sets that bit.Quoted 46 lines
Quoted 46 lines
builtin/reflog.c +4
-31const 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
Re: [PATCH 1/2] builtin/reflog.c: use parse-options for expire subcommand
Am 01.01.22 um 20:09 schrieb John Cai:
Quoted 16 lines
Quoted 6 lines · René Scharfe
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?
(quoted diff) +0
-5- 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=")) {
Review
Quoted 10 lines
(quoted diff) +4
-4- 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é
PATCH 2/2 builtin/reflog.c: switch to use parse-options API for delete subcommand
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
Address NEEDSWORK by switching out manual arg parsing for the parse-options API for the delete subcommand.
Signed-off-by "John Cai" <[email protected]>
--- builtin/reflog.c | 52 +++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 27 deletions(-)
builtin/reflog.c +25 -27
@@ -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;
Signature · 1 line
gitgitgadget
v2 cover reflog.c: switch to use parse-options API
In reply to John Cai via GitGitGadgetSwitch 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" <[email protected]>
## 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;
}Signature · 1 line
gitgitgadget
PATCH v2 1/2 parse-options.h: add parse_opt_expiry_date helper
From: John Cai <[email protected]>
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" <[email protected]>
--- parse-options-cb.c | 7 ++++++- parse-options.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-)
parse-options-cb.c +6 -1
@@ -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 ; }
parse-options.h +1 -0
@@ -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))
Signature · 1 line
gitgitgadget
Re: [PATCH v2 1/2] parse-options.h: add parse_opt_expiry_date helper
Quoted 27 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Extract the logic in
parse_opt_expiry_date_cbinto a helperparse_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 inopt->valueas 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" <[email protected]>--- parse-options-cb.c | 7 ++++++- parse-options.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-)parse-options-cb.c +3
-0@@ -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); +}
parse-options-cb.c int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) · line 48
Quoted 10 lines
parse-options-cb.c +3
-1+ +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'").
Quoted 12 lines
parse-options.h +1
-0@@ -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))
PATCH v2 2/2 builtin/reflog.c: switch to use parse-options API for delete subcommand
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
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" <[email protected]>
--- builtin/reflog.c | 168 ++++++++++++++++++++++++----------------------- 1 file changed, 87 insertions(+), 81 deletions(-)
builtin/reflog.c +87 -81
@@ -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;
Signature · 1 line
gitgitgadget
Re: [PATCH v2 2/2] builtin/reflog.c: switch to use parse-options API for delete subcommand
Quoted 76 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Address NEEDSWORK by switching out manual arg parsing for the parse-options API for the delete subcommand.
Moves
explicit_expiryflag intocmd_reflog_expire_cbstruct so a callback can set both the value of the expiration as well as theexplicit_expiryflag.Signed-off-by "John Cai" <[email protected]>--- builtin/reflog.c | 168 ++++++++++++++++++++++++----------------------- 1 file changed, 87 insertions(+), 81 deletions(-)builtin/reflog.c +7
-16@@ -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 ;
builtin/reflog.c static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c · line 531
Quoted 10 lines
builtin/reflog.c +2
-2}/* 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.
Quoted 5 lines
builtin/reflog.c +5
-0+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>..." ),
builtin/reflog.c static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c · line 547
Quoted 10 lines
builtin/reflog.c +10
-0+ 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.
builtin/reflog.c static int count_reflog_ent(struct object_id *ooid, struct object_id *noid, · line 708
Quoted 1 line
builtin/reflog.c +1
-0+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.
Quoted 3 lines
builtin/reflog.c +0
-0const char *spec =strstr (argv[i],"@{" );char *ep, *ref;int recno;
PATCH v3 1/1 builtin/reflog.c: use parse-options api for expire, delete subcommands
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
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" <[email protected]>
---
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 <[email protected]>
## 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" <[email protected]>
@@ 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(-)
builtin/reflog.c +95 -79
@@ -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
Signature · 1 line
gitgitgadget
Re: [PATCH v3] builtin/reflog.c: use parse-options api for expire, delete subcommands
Quoted 10 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Switching out manual arg parsing for the parse-options API for the expire and delete subcommands.
Move
explicit_expiryflag intocmd_reflog_expire_cbstruct so callbacks can set both the value of the timestamp as well as theexplicit_expiryflag.Signed-off-by "John Cai" <[email protected]>
Makes sense.
Quoted 6 lines
builtin/reflog.c +0
-6-/* 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>..." );
...
builtin/reflog.c static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c · line 554
Quoted 5 lines
builtin/reflog.c +5
-0+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.
builtin/reflog.c static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c · line 592
Quoted 10 lines
static int
cmd_reflog_expire(int argc, const char **argv, const char *prefix){builtin/reflog.c +1
-1struct 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.
PATCH v4 1/1 builtin/reflog.c: use parse-options api for expire, delete subcommands
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
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" <[email protected]>
---
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(-)
builtin/reflog.c +95 -79
@@ -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
Signature · 1 line
gitgitgadget
Re: [PATCH v4] builtin/reflog.c: use parse-options api for expire, delete subcommands
builtin/reflog.c static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c · line 565
Quoted 8 lines · John Cai via GitGitGadget
builtin/reflog.c +8
-0+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.
builtin/reflog.c static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c · line 578
Quoted 12 lines
builtin/reflog.c +12
-0+ 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.
Quoted 9 lines
static int
cmd_reflog_expire(int argc, const char **argv, const char *prefix){builtin/reflog.c +0
-1struct 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;
builtin/reflog.c static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c · line 601
Quoted 10 lines
builtin/reflog.c +10
-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_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).
builtin/reflog.c static int count_reflog_ent(struct object_id *ooid, struct object_id *noid, · line 717
Quoted 3 lines
builtin/reflog.c +3
-0+static const char * reflog_delete_usage[] = { +N_ ("git reflog delete [--rewrite] [--updateref] " +"[--dry-run | -n] [--verbose] <refs>..." ),
Funny indentation still here?
builtin/reflog.c static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) · line 730
Quoted 13 lines
builtin/reflog.c +6
-0+ 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),
builtin/reflog.c static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) · line 747
Quoted 8 lines
builtin/reflog.c +2
-1+ 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.
builtin/reflog.c static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) · line 752
Quoted 5 lines
builtin/reflog.c +1
-1-for ( ; i < argc; i++) { +for (i =0 ; i < argc; i++) {const char *spec =strstr (argv[i],"@{" );char *ep, *ref;int recno;
Nice, too.
Thanks.
PATCH v5 1/1 builtin/reflog.c: use parse-options api for expire, delete subcommands
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
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" <[email protected]>
---
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(-)
builtin/reflog.c +97 -79
@@ -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
Signature · 1 line
gitgitgadget
Re: [PATCH v5] builtin/reflog.c: use parse-options api for expire, delete subcommands
Quoted 11 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Switching out manual arg parsing for the parse-options API for the expire and delete subcommands.
Move
explicit_expiryflag intocmd_reflog_expire_cbstruct so callbacks can set both the value of the timestamp as well as theexplicit_expiryflag.Signed-off-by "John Cai" <[email protected]>---
Thanks. Will queue.
Quoted 7 lines
@@ 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.
Re: [PATCH v5] builtin/reflog.c: use parse-options api for expire, delete subcommands
Quoted 24 lines · Junio C Hamano
Quoted 11 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Switching out manual arg parsing for the parse-options API for the expire and delete subcommands.
Move
explicit_expiryflag intocmd_reflog_expire_cbstruct so callbacks can set both the value of the timestamp as well as theexplicit_expiryflag.Signed-off-by "John Cai" <[email protected]>---
Thanks. Will queue.
Quoted 7 lines
@@ 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"),
Quoted 1 line
I think this change is a fat-finger; will correct while queuing.
Oops, yes. Thanks for that and helping me through the review process :)