Re: [PATCH] rev-parse: match @{u}, @{push} and ^{<type>} case-insensitively

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> writes:

> Change the revision parsing logic to match @{upstream}, @{u}, @{push},
> ^{commit}, ^{tree} etc. case-insensitively. All of these cases
> currently emit "unknown revision or path not in the working tree"
> errors.
>
> This change makes them equivalent to their lower-case versions, and
> consistent with other revision format specifications, e.g. 'master@{6
> hours ago}', which is equivalent to 'master@{6 HoUrS aGo}'.

Approxidate is not just case insensitive, but it takes random
non-word characters (e.g. a dot and a slash in "@{4.minutes/}") that
are not spaces at word boundaries, and I do not think you want to
accept @{.Upstream.} for consistency.

It is an odd-man-out and "consistency" with it is a nonsense
justification.

> The use-case for this is being able to hold the shift key down while
> typing @{u} on certain keyboard layouts, which makes the sequence
> easier to type, and reduces cases where git throws an error at the
> user where it could do what he means instead.

This, on the hand, is a sane justification that can be sympathized.

> The objection from Junio at the time[2] was that by lower-casing
> {...}:
>
>     [The door would be closed on] allow[ing] @{/regexp} to find a
>     reflog entry that matches the given pattern, and in such a use
>     case we would certainly want to take the pattern in a case
>     sensitive way.
>
> This appears to be an objection related to the code structure at the
> time,...

This objection, which is not about code structure but about design,
still applies, I would think, if your justification is "consistency
by making everything case-insensitive".  

Whoever is doing @{/<pattern>} cannot add the feature in a case
sensitive way without violating the declaration you are making here:
"everything inside @{...} is case-insensitive".

And if you extend that declaration to say "everything inside ^{...},
too, is case-insensitive", I think it already is broken as I think
"^{/<pattern>}" is case sensitive, by the way.

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.  There is no sensible
"consistency" justification possible.

I think "immediately after typing '{', you often have SHIFT
pressed", even though it may sound lame, is a much better
justification.  At least, it is an honest one.  And I do not mind
too much if the way this feature is sold to the users were "these
keywards inside @{...} can be spelled in any case: push, upstream.
Type names in the peel-onion operator ^{<type>} can be too", not as
a general rule but as special cases.  Unlike end-user supplied
strings taken from an unbounded set (e.g. /<search patterns>), there
is no strong reason to insist that a set of keywords taken from a
limited vocabulary has to be spelled in one case, as long as it does
not introduce ambiguity or limit our possible future.  It's not like
we may want to keep the door open to make @{push} and @{PUSH} mean
different things later.

Even in that case, however, I'd strongly prefer to spell all the
examples in lowercase and declare that lowercase is the canonical
spelling in our documentation.  What I want to avoid is to have
three Git textbooks, that use @{UPSTREAM}, @{Upstream}, and
@{upstream} in their samples and descriptions, and have the readers
waste their time wondering, and waste our time by asking here, where
the different preferences of the authors of these three books come
from and which one the canonical way to spell it is.

Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]