"John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: John Cai <johncai86@xxxxxxxxx>
>
> Address NEEDSWORK by switching out manual arg parsing for the
> parse-options API for the delete subcommand.
>
> Moves explicit_expiry flag into cmd_reflog_expire_cb struct so a
> callback can set both the value of the expiration as well as the
> explicit_expiry flag.
>
> Signed-off-by: "John Cai" <johncai86@xxxxxxxxx>
> ---
> builtin/reflog.c | 168 ++++++++++++++++++++++++-----------------------
> 1 file changed, 87 insertions(+), 81 deletions(-)
>
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 175c83e7cc2..3552d749e4b 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -12,15 +12,6 @@
> #include "reachable.h"
> #include "worktree.h"
>
> -/* NEEDSWORK: switch to using parse_options */
> -static const char reflog_expire_usage[] =
> -N_("git reflog expire [--expire=<time>] "
> - "[--expire-unreachable=<time>] "
> - "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] "
> - "[--verbose] [--all] <refs>...");
> -static const char reflog_delete_usage[] =
> -N_("git reflog delete [--rewrite] [--updateref] "
> - "[--dry-run | -n] [--verbose] <refs>...");
> static const char reflog_exists_usage[] =
> N_("git reflog exists <ref>");
>
> @@ -30,6 +21,7 @@ static timestamp_t default_reflog_expire_unreachable;
> struct cmd_reflog_expire_cb {
> struct rev_info revs;
> int stalefix;
> + int explicit_expiry;
> timestamp_t expire_total;
> timestamp_t expire_unreachable;
> int recno;
> @@ -504,18 +496,17 @@ static int reflog_expire_config(const char *var, const char *value, void *cb)
> return 0;
> }
>
> -static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, const char *ref)
> +static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, const char *ref)
> {
> struct reflog_expire_cfg *ent;
> -
> - if (slot == (EXPIRE_TOTAL|EXPIRE_UNREACH))
> + if (cb->explicit_expiry == (EXPIRE_TOTAL|EXPIRE_UNREACH))
> return; /* both given explicitly -- nothing to tweak */
>
> for (ent = reflog_expire_cfg; ent; ent = ent->next) {
> if (!wildmatch(ent->pattern, ref, 0)) {
> - if (!(slot & EXPIRE_TOTAL))
> + if (!(cb->explicit_expiry & EXPIRE_TOTAL))
> cb->expire_total = ent->expire_total;
> - if (!(slot & EXPIRE_UNREACH))
> + if (!(cb->explicit_expiry & EXPIRE_UNREACH))
> cb->expire_unreachable = ent->expire_unreachable;
> return;
> }
> @@ -525,27 +516,75 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c
> * If unconfigured, make stash never expire
> */
> if (!strcmp(ref, "refs/stash")) {
> - if (!(slot & EXPIRE_TOTAL))
> + if (!(cb->explicit_expiry & EXPIRE_TOTAL))
> cb->expire_total = 0;
> - if (!(slot & EXPIRE_UNREACH))
> + if (!(cb->explicit_expiry & EXPIRE_UNREACH))
> cb->expire_unreachable = 0;
> return;
> }
>
> /* Nothing matched -- use the default value */
> - if (!(slot & EXPIRE_TOTAL))
> + if (!(cb->explicit_expiry & EXPIRE_TOTAL))
> cb->expire_total = default_reflog_expire;
> - if (!(slot & EXPIRE_UNREACH))
> + if (!(cb->explicit_expiry & EXPIRE_UNREACH))
> cb->expire_unreachable = default_reflog_expire_unreachable;
> }
OK.
> +static const char * reflog_expire_usage[] = {
> + N_("git reflog expire [--expire=<time>] "
> + "[--expire-unreachable=<time>] "
> + "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] "
> + "[--verbose] [--all] <refs>..."),
> + NULL
> +};
> +
> +static int expire_unreachable_callback(const struct option *opt,
> + const char *arg,
> + int unset)
> +{
> + struct cmd_reflog_expire_cb *cmd = opt->value;
> + cmd->explicit_expiry |= EXPIRE_UNREACH;
> + return parse_opt_expiry_date(&cmd->expire_unreachable, arg, unset);
Just as I suspected in the previous step. Get rid of [1/2] and
add the new helper as a static function to this file.
The thing is that we shouldn't confuse future developers by adding a
random function that cannot be used as OPTION_CALLBACK function in
the parse-options-cb.c file.
> + for (int i = 0; i < argc; i++) {
Documentation/CodingGuidelines:
- Declaring a variable in the for loop "for (int i = 0; i < 10; i++)"
is still not allowed in this codebase.
We have floated a weather balloon by adding a single use of this
construct so that when somebody finds a compiler that do not like
the construct it is easy to revert, and waiting for a feedback. We
do not want to add the use of the construct to random places yet
that makes us scramble and revert them from all over the place.
> const char *spec = strstr(argv[i], "@{");
> char *ep, *ref;
> int recno;
