Re: [PATCH 0/2] Fix regression: CamelCased aliases

[Jeff King <peff@peff.net>]

On Fri, Jul 14, 2017 at 08:14:18AM -0700, Junio C Hamano wrote:

> >> It was possible before v2.13.3 to invoke:
> >> 
> >> 	git config alias.CamelCased <something>
> >> 	git CamelCased
> >> 
> >> This regressed (due to a stupid mistake of mine that was not caught in
> >> patch review, sadly) in v2.13.3.
> >
> > Interesting. I don't think this was ever intended to work.
> > ...
> > The patches look obviously correct.
> 
> How can something be "(n)ever intended to work" and yet patches to
> make it work be "obviously correct"? ;-)

You snipped the part where I said "this is probably reasonable to do in
the meantime." :)

My "obviously correct" is only that the patches fulfill that.

> But I think that it is still reasonable for an end user to expect
> that 'git Foo' would trigger that alias.  And that is what was
> recently changed, inadvertently.
> 
> So the problem may need to be explained better in this series, but I
> think the usage was expected to work and the series is fixing a real
> regression.

Sort of. It did not work until it accidentally did work, and now it
accidentally does not work again. So "usage was expected to work" was
certainly not true for Git developers (or at least nobody intentionally
wrote code to make it so). And anybody relying on it started doing so
since v2.2.0.

But like I said, it's probably reasonable to make it work. There's
little harm in doing so.  The only downside I can see is that doing:

  git config alias.foo <something>
  git Foo

now triggers the alias. That seems like at worst a minor bug, and
possibly even the right thing to do (see below).

> Do we want to promise to keep the following "working"?
> 
>     git config alias.Foo <something>
>     git foo
> 
> By designing the system in such a way that an alias is created with
> a two-level name in our system, we are saying that alias names are
> case insensitive to the end users, so I _think_ the above is
> intended to work, and we are effectively promising that it will keep
> working.

Yes, I think we must. Keys are case-insensitive, and you are allowed to
write them in whatever case you like. The more interesting case is the
reverse, that I showed above. I think there are basically two mental
models that are reasonable:

  1. Uppercase in key names is treated the same as lowercase. Therefore
     we must allow "alias.Foo" to match "git foo", but "git Foo" can
     never have a match (in the current schema).

  2. Keys are case-insensitive, and anything that matches them is
     considered case-insensitive, too. That means "Foo" and "foo" are
     identical for these purposes, and you can never have two aliases
     "Foo" and "foo".

In either mental model, "alias.Foo" for "git foo" must work. But the
reverse only works in (2).

I think either model is fine. These patches push us into (2).

-Peff