Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> I'm not suggesting that we make all the options accept garbage like
> the date option in the name of consistency.
Then "approxidate is case insensitive so others should too" argument
does not hold water. Not that it has to, as I would prefer to see
an honest justification for a feature. Among @{...} and ^{...},
approxidate is the only case insensitive one; if we are shooting for
consistency we'd be making everything case sensitive, which is the
opposite of what we want here.
>> So don't pretend that this is about consistency. You are making a
>> choice for one class of strings that can go inside @{...} and the
>> choice does not depend on the case sensitivity of different classes
>> of strings that can go the same place.
>
> I think this too is really just a reply to what Duy said...
No, everything was a direct reply to you (I haven't even read Duy's
message when I responded). Your patch that started the thread tried
to justify it as making things consistent, and it was a direct
response to it.
> So do you mean you'd like me to change the documentation to be more
> like "While this is canonically lower case this form is case
> insensitive so e.g. so-and-so also work" ?
TL;DR
- In proposed log message, only mention that we are doing this
because holding SHIFT after typing "@{" makes 'Upstream' or
'PUSH' easier to type than 'upstream' or 'push'. Don't invoke
the "case insensitivity for consistency" or "approximate is case
insensitive" as a rationle.
- In documentation, the current description that only mentions
'push' and 'upstream' in lowercase is sufficient for signaling
what is canonical. Just adding a parenthetical note,
e.g. ('push' and 'upstream' are accepted when spelled with
uppercase and they mean the same thing no matter the case), after
the current text finishes describing both of them is sufficient
to help users who wonder why an otherwise undocumented @{PUSH}
that they typed by mistake was accepted, and also help them when
they see others write @{Push} in their scripts and wonder if it
means something different from what they know, i.e. @{push}.
- Do not allow upcasing ^{<type>}, at least for now.
and a longer version.
The reason why I care deeply about how a change justifies itself in
the log message is because it can set the design principle for the
future of the system. We shouldn't say "Anything inside @{...}
should be case insensitive." to avoid limiting the design space
unnecessarily for those who design new features @{/<pattern>}
etc. in the future, for example.
Most parts of the system, not limited to @{} and ^{} syntax, are
case sensitive, and there is no point repeating "this is case
sensitive except..." all over the place.
However, if we make some part case insesitive without documenting,
or make any "hidden" feature without documenting in general, it
risks confusing the users in two possible ways:
(1) the user may trigger an undocumented feature first by mistake,
and will wonder why it worked as expected and will also wonder
if that is guaranteed to work in the future;
(2) the user may see the use of an undocumented feature, and would
wonder if there is any difference in the behaviour with the
documented counterpart.
A parenthetical note should be designed to help with the above two
points.
Because we do not accept "cat-file -t <type>" and "hash-object -t
<type>" in a case insensitive way, it is not a good idea to allow
"^{COMMIT}", especially because it is not clear if allowing
"cat-file -t COMMIT" is a good idea at all. We never restricted
that the "unknown type" must be spelled in lowercase, and without
such restriction, it is never safe to allow <type> to be case
insensitive ("^{<type>}" is much less necessary these days anyway as
"$OBJECT^0" and "$OBJECT:" are shorter and easier to type and can be
used for most of the cases you may want to use "^{<type>}").