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 <[email protected]>
> ---
>  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

Reply via email to