Michael Haggerty writes ("Re: [PATCH 2/5] check-ref-format: Refactor to make
--branch code more common"):
> On 11/04/2016 08:13 PM, Ian Jackson wrote:
> > 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`?
I have no real opinion about the name of this variable. `err' is a
fine name too.
> > + 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`.)
It was like that when I found it :-). I wasn't sure why this
restriction was there so I left it alone.
Looking at it again: AFAICT from the documentation --branch is a
completely different mode. The effect of --normalize is not to do
additional work, but simply to produce additional output. It really
means --print-normalized. --branch already prints output, but AFAICT
it does not collapse slashes. This seems like a confusing collection
of options. But, sorting that out is beyond the scope of what I was
trying to do.
In my series I have at least managed not to make any of this any
worse, I think: the --stdin option I introduce applies to both modes
equally, and doesn't make future improvements to the conflict between
--branch and --normalize any harder.
(In _this_ patch, certainly, allowing --normalize with --branch would
be wrong, since _this_ patch is just refactoring.)
Thanks,
Ian.
--
Ian Jackson <ijack...@chiark.greenend.org.uk> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
