On 11/04/2016 08:13 PM, Ian Jackson wrote:
> We are going to want to permit other options with --branch.
>
> So, replace the special case with just an entry for --branch in the
> parser for ordinary options, and check for option compatibility at the
> end.
>
> No overall functional change.
>
> Signed-off-by: Ian Jackson <ijackson@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> builtin/check-ref-format.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
> index 4d56caa..f12c19c 100644
> --- a/builtin/check-ref-format.c
> +++ b/builtin/check-ref-format.c
> @@ -49,13 +49,19 @@ static int check_ref_format_branch(const char *arg)
> }
>
> static int normalize = 0;
> +static int check_branch = 0;
> static int flags = 0;
>
> static int check_one_ref_format(const char *refname)
> {
> + int got;
`got` is an unusual name for this variable, and I don't really
understand what the word means in this context. Is there a reason not to
use the more usual `err`?
> +
> if (normalize)
> refname = collapse_slashes(refname);
> - if (check_refname_format(refname, flags))
> + got = check_branch
> + ? check_ref_format_branch(refname)
> + : check_refname_format(refname, flags);
> + if (got)
> return 1;
> if (normalize)
> printf("%s\n", refname);
> @@ -68,9 +74,6 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
> if (argc == 2 && !strcmp(argv[1], "-h"))
> usage(builtin_check_ref_format_usage);
>
> - if (argc == 3 && !strcmp(argv[1], "--branch"))
> - return check_ref_format_branch(argv[2]);
> -
> for (i = 1; i < argc && argv[i][0] == '-'; i++) {
> if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
> normalize = 1;
> @@ -80,9 +83,15 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
> flags &= ~REFNAME_ALLOW_ONELEVEL;
> else if (!strcmp(argv[i], "--refspec-pattern"))
> flags |= REFNAME_REFSPEC_PATTERN;
> + else if (!strcmp(argv[i], "--branch"))
> + check_branch = 1;
> else
> usage(builtin_check_ref_format_usage);
> }
> +
> + if (check_branch && (flags || normalize))
Is there a reason not to allow `--normalize` with `--branch`?
(Currently, `git check-ref-format --branch` *does* allow input like
`refs/heads/foo`.)
But note that simply allowing `--branch --normalize` without changing
`check_one_ref_format()` would mean generating *two* lines of output per
reference, so something else would have to change, too.
> + usage(builtin_check_ref_format_usage);
> +
> if (! (i == argc - 1))
> usage(builtin_check_ref_format_usage);
>
>
Michael