git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Conrad Irwin <conrad.irwin@gmail.com>,
	Sitaram Chamarty <sitaramc@gmail.com>,
	Michael J Gruber <git@drmicha.warpmail.net>,
	Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	Jeff King <peff@peff.net>, Richard Hansen <rhansen@bbn.com>,
	"Brian M . Carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH] rev-parse: match @{u}, @{push} and ^{<type>} case-insensitively
Date: Sun, 19 Mar 2017 05:55:21 -0700	[thread overview]
Message-ID: <xmqqtw6pzarq.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170318223409.13441-1-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Sat, 18 Mar 2017 22:34:09 +0000")

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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.

  parent reply	other threads:[~2017-03-19 12:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-18 22:34 [PATCH] rev-parse: match @{u}, @{push} and ^{<type>} case-insensitively Ævar Arnfjörð Bjarmason
2017-03-19  9:19 ` Duy Nguyen
2017-03-19 10:04   ` Ævar Arnfjörð Bjarmason
2017-03-19 12:55 ` Junio C Hamano [this message]
2017-03-19 14:26   ` Ævar Arnfjörð Bjarmason
2017-03-19 22:53     ` Junio C Hamano
2017-03-26 12:16       ` [PATCH v2 0/3] rev-parse case insensitivity & @{p} synonym Ævar Arnfjörð Bjarmason
2017-03-26 12:16       ` [PATCH v2 1/3] rev-parse: match @{upstream}, @{u} and @{push} case-insensitively Ævar Arnfjörð Bjarmason
2017-03-27  0:27         ` Junio C Hamano
2017-03-27 11:16           ` [PATCH v3] " Ævar Arnfjörð Bjarmason
2017-03-27 17:45             ` Junio C Hamano
2017-03-27 18:05               ` Ævar Arnfjörð Bjarmason
2017-03-26 12:16       ` [PATCH v2 2/3] rev-parse: add @{p} as a synonym for @{push} Ævar Arnfjörð Bjarmason
2017-03-27  2:53         ` Jeff King
2017-03-27  7:52           ` Ævar Arnfjörð Bjarmason
2017-03-26 12:16       ` [PATCH v2 3/3] rev-parse: match ^{<type>} case-insensitively Ævar Arnfjörð Bjarmason
2017-03-27  0:36         ` Junio C Hamano
2017-03-27  2:58           ` Jeff King
2017-03-27  5:39             ` Junio C Hamano
2017-03-27  7:11               ` Jeff King
2017-03-27  7:33             ` Ævar Arnfjörð Bjarmason
2017-03-21 19:19 ` [PATCH] rev-parse: match @{u}, @{push} and " Ævar Arnfjörð Bjarmason
2017-03-21 19:26   ` Junio C Hamano
2017-03-21 19:43   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtw6pzarq.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=conrad.irwin@gmail.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=rhansen@bbn.com \
    --cc=sandals@crustytoothpaste.net \
    --cc=sitaramc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).