Re: [PATCH 6/6] builtin/tag: add --format argument for tag -v

[Junio C Hamano]

santi...@nyu.edu writes:

> @@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char 
> *prefix)
>               die(_("--merged and --no-merged option are only allowed with 
> -l"));
>       if (cmdmode == 'd')
>               return for_each_tag_name(argv, delete_tag);
> -     if (cmdmode == 'v')
> +     if (cmdmode == 'v') {
> +             if (fmt_pretty)
> +                     verify_ref_format(fmt_pretty);
>               return for_each_tag_name(argv, verify_tag);
> +     }

OK, you said something about for_each_ref() in an earlier commit,
but what you meant was this one, which takes each_tag_name_fn.

The function for_each_tag_name(), the type each_tag_name_fn, and the
function of that type verify_tag(), are ALL file-scope static in
this single file, builtin/tag.c.  It seems to me that it is not
necessary to make the format string global at all.

> @@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char 
> *prefix)
>               die(_("--merged and --no-merged option are only allowed with 
> -l"));
>       if (cmdmode == 'd')
>               return for_each_tag_name(argv, delete_tag);
> -     if (cmdmode == 'v')
> +     if (cmdmode == 'v') {
> +             if (fmt_pretty)
> +                     verify_ref_format(fmt_pretty);
>               return for_each_tag_name(argv, verify_tag);
> +     }

There are minor implementation and design issues I spotted, but
overall I think the feature the series attempts to add may be a good
thing to have.

Thanks.