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

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}'.

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 change was independently authored to scratch a longtime itch, but when I was about to submit it I discovered that a similar patch had been submitted unsuccessfully before by Conrad Irwin in August 2011 as "rev-parse: Allow @{U} as a synonym for @{u}"[1].

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, but the current code does not conflate the parsing of @{upstream}, @{push}, ^{tree} etc. with any other @{STRING} or ^{TYPE} we might add in the future and want to keep case-sensitive.

The new starts_with_case() function is a copy of the existing adjacent starts_with(), just with a tolower() in the "else if".

The tests for this patch are more exhaustive than in the 2011 submission. The starting point for them was to first change the code to only support upper-case versions of the existing words, seeing what broke, and amending the breaking tests to check upper case & mixed case as appropriate, and where not redundant to other similar tests.

1. <[email protected]>
2. <[email protected]>

---
 Documentation/revisions.txt   | 11 ++++++++++-
 git-compat-util.h             |  1 +
 sha1_name.c                   | 12 ++++++------
 strbuf.c                      |  9 +++++++++
 t/t1450-fsck.sh               |  7 +++++++
 t/t1507-rev-parse-upstream.sh | 15 +++++++++++----
 t/t1511-rev-parse-caret.sh    | 13 +++++++++++++
 t/t1514-rev-parse-push.sh     |  8 ++++++--
 8 files changed, 63 insertions(+), 13 deletions(-)

index ba11b9c95e..55bde6ea65 100644
@@ -96,7 +96,8 @@ some output processing may assume ref names in UTF-8.
   refers to the branch that the branch specified by branchname is set to build on
   top of (configured with `branch.<name>.remote` and
   `branch.<name>.merge`).  A missing branchname defaults to the
-  current one.
+  current one. Both '@\{upstream\}', '@\{u\}' are case-insensitive, so e.g.
+  '@\{UPSTREAM\}', '@\{U\}' or '@\{Upstream\}' also work.
 
 '<branchname>@\{push\}', e.g. 'master@\{push\}', '@\{push\}'::
   The suffix '@\{push}' reports the branch "where we would push to" if

@@ -122,6 +123,9 @@ refs/remotes/myfork/mybranch
 Note in the example that we set up a triangular workflow, where we pull
 from one location and push to another. In a non-triangular workflow,
 '@\{push}' is the same as '@\{upstream}', and there is no need for it.
++
+'@\{push}' is matched case-insensitively, so e.g. '@\{PUSH}' or
+'@\{Push}' also works.
 
 '<rev>{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
   A suffix '{caret}' to a revision parameter means the first parent of

@@ -158,6 +162,11 @@ it does not have to be dereferenced even once to get to an object.
 +
 'rev{caret}\{tag\}' can be used to ensure that 'rev' identifies an
 existing tag object.
++
+The {caret}{<type>} part is matched case-insensitively. So
+e.g. '{caret}\{commit\}' can be equivalently specified as
+'{caret}\{COMMIT\}', '{caret}\{Commit\}' etc., '{caret}\{tree\}' as
+'{caret}\{TREE\}' and so forth.
 
 '<rev>{caret}{}', e.g. 'v0.99.8{caret}{}'::
   A suffix '{caret}' followed by an empty brace pair

index e626851fe9..2b3b581a7c 100644
@@ -448,6 +448,7 @@ extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
 extern int starts_with(const char *str, const char *prefix);
+extern int starts_with_case(const char *str, const char *prefix);
 
 /*
  * If the string "str" begins with the string found in "prefix", return 1.

index cda9e49b12..8ff215932d 100644
@@ -549,7 +549,7 @@ static inline int at_mark(const char *string, int len,
 	for (i = 0; i < nr; i++) {
 		int suffix_len = strlen(suffix[i]);
 		if (suffix_len <= len
-		    && !memcmp(string, suffix[i], suffix_len))
+		    && !strncasecmp(string, suffix[i], suffix_len))
 			return suffix_len;
 	}
 	return 0;

@@ -821,15 +821,15 @@ static int peel_onion(const char *name, int len, unsigned char *sha1,
 		return -1;
 
 	sp++; /* beginning of type name, or closing brace for empty */
-	if (starts_with(sp, "commit}"))
+	if (starts_with_case(sp, "commit}"))
 		expected_type = OBJ_COMMIT;
-	else if (starts_with(sp, "tag}"))
+	else if (starts_with_case(sp, "tag}"))
 		expected_type = OBJ_TAG;
-	else if (starts_with(sp, "tree}"))
+	else if (starts_with_case(sp, "tree}"))
 		expected_type = OBJ_TREE;
-	else if (starts_with(sp, "blob}"))
+	else if (starts_with_case(sp, "blob}"))
 		expected_type = OBJ_BLOB;
-	else if (starts_with(sp, "object}"))
+	else if (starts_with_case(sp, "object}"))
 		expected_type = OBJ_ANY;
 	else if (sp[0] == '}')
 		expected_type = OBJ_NONE;

index ace58e7367..d6483381c4 100644
@@ -11,6 +11,15 @@ int starts_with(const char *str, const char *prefix)
 			return 0;
 }
 
+int starts_with_case(const char *str, const char *prefix)
+{
+	for (; ; str++, prefix++)
+		if (!*prefix)
+			return 1;
+		else if (tolower(*str) != tolower(*prefix))
+			return 0;
+}
+
 /*
  * Used as the default ->buf value, so that people can always assume
  * buf is non NULL and ->buf is NUL terminated even for a freshly

index 33a51c9a67..b6c1989671 100755
@@ -505,6 +505,13 @@ test_expect_success 'fsck notices missing tagged object' '
 	test_must_fail git -C missing fsck
 '
 
+test_expect_success 'fsck notices missing tagged object with case insensitive {blob}' '
+	create_repo_missing tag^{BLOB} &&
+	test_must_fail git -C missing fsck &&
+	create_repo_missing tag^{BloB} &&
+	test_must_fail git -C missing fsck
+'
+
 test_expect_success 'fsck notices ref pointing to missing commit' '
 	create_repo_missing HEAD &&
 	test_must_fail git -C missing fsck

index 46ef1f22dc..b23c4e3fab 100755
@@ -46,11 +46,14 @@ error_message () {
 }
 
 test_expect_success '@{upstream} resolves to correct full name' '
-	test refs/remotes/origin/master = "$(full_name @{upstream})"
+	test refs/remotes/origin/master = "$(full_name @{upstream})" &&
+	test refs/remotes/origin/master = "$(full_name @{UPSTREAM})" &&
+	test refs/remotes/origin/master = "$(full_name @{UpSTReam})"
 '
 
 test_expect_success '@{u} resolves to correct full name' '
-	test refs/remotes/origin/master = "$(full_name @{u})"
+	test refs/remotes/origin/master = "$(full_name @{u})" &&
+	test refs/remotes/origin/master = "$(full_name @{U})"
 '
 
 test_expect_success 'my-side@{upstream} resolves to correct full name' '

@@ -60,6 +63,8 @@ test_expect_success 'my-side@{upstream} resolves to correct full name' '
 test_expect_success 'upstream of branch with @ in middle' '
 	full_name fun@ny@{u} >actual &&
 	echo refs/remotes/origin/side >expect &&
+	test_cmp expect actual &&
+	full_name fun@ny@{U} >actual &&
 	test_cmp expect actual
 '
 

@@ -96,12 +101,14 @@ test_expect_success 'not-tracking@{u} fails' '
 test_expect_success '<branch>@{u}@{1} resolves correctly' '
 	test_commit 6 &&
 	(cd clone && git fetch) &&
-	test 5 = $(commit_subject my-side@{u}@{1})
+	test 5 = $(commit_subject my-side@{u}@{1}) &&
+	test 5 = $(commit_subject my-side@{U}@{1})
 '
 
 test_expect_success '@{u} without specifying branch fails on a detached HEAD' '
 	git checkout HEAD^0 &&
-	test_must_fail git rev-parse @{u}
+	test_must_fail git rev-parse @{u} &&
+	test_must_fail git rev-parse @{U}
 '
 
 test_expect_success 'checkout -b new my-side@{u} forks from the same' '

index e0a49a651f..56750f99c6 100755
@@ -48,6 +48,10 @@ test_expect_success 'ref^{commit}' '
 	git rev-parse ref >expected &&
 	git rev-parse ref^{commit} >actual &&
 	test_cmp expected actual &&
+	git rev-parse ref^{COMMIT} >actual &&
+	test_cmp expected actual &&
+	git rev-parse ref^{CoMMiT} >actual &&
+	test_cmp expected actual &&
 	git rev-parse commit-tag^{commit} >actual &&
 	test_cmp expected actual &&
 	test_must_fail git rev-parse tree-tag^{commit} &&

@@ -58,6 +62,10 @@ test_expect_success 'ref^{tree}' '
 	echo $TREE_SHA1 >expected &&
 	git rev-parse ref^{tree} >actual &&
 	test_cmp expected actual &&
+	git rev-parse ref^{TREE} >actual &&
+	test_cmp expected actual &&
+	git rev-parse ref^{TrEe} >actual &&
+	test_cmp expected actual &&
 	git rev-parse commit-tag^{tree} >actual &&
 	test_cmp expected actual &&
 	git rev-parse tree-tag^{tree} >actual &&

@@ -67,8 +75,13 @@ test_expect_success 'ref^{tree}' '
 
 test_expect_success 'ref^{tag}' '
 	test_must_fail git rev-parse HEAD^{tag} &&
+	test_must_fail git rev-parse HEAD^{TAG} &&
 	git rev-parse commit-tag >expected &&
 	git rev-parse commit-tag^{tag} >actual &&
+	test_cmp expected actual &&
+	git rev-parse commit-tag^{TAG} >actual &&
+	test_cmp expected actual &&
+	git rev-parse commit-tag^{Tag} >actual &&
 	test_cmp expected actual
 '
 

index 623a32aa6e..788cc91e45 100755
@@ -24,12 +24,16 @@ test_expect_success 'setup' '
 
 test_expect_success '@{push} with default=nothing' '
 	test_config push.default nothing &&
-	test_must_fail git rev-parse master@{push}
+	test_must_fail git rev-parse master@{push} &&
+	test_must_fail git rev-parse master@{PUSH} &&
+	test_must_fail git rev-parse master@{PuSH}
 '
 
 test_expect_success '@{push} with default=simple' '
 	test_config push.default simple &&
-	resolve master@{push} refs/remotes/origin/master
+	resolve master@{push} refs/remotes/origin/master &&
+	resolve master@{PUSH} refs/remotes/origin/master &&
+	resolve master@{pUSh} refs/remotes/origin/master
 '
 
 test_expect_success 'triangular @{push} fails with default=simple' '

2.11.0

I have a slight bias to keeping it the way it is, just because of the distracted user in a hurry use-case that's looking up only @{u} and not reading the manpage all the way through.

But yeah, I could do that, i.e. have some blurb at the top noting that all of the magical syntax is case insensitive unless otherwise noted. AFAICT the only thing that's case sensitive now is the ^{/regex} form.

Also an advantage of that would be that currently we don't note that e.g. sha1s like dae86e aren't case sensitive and can be written like DAE86E, and the @{<date>} syntax doesn't note that it's case insensitive. A blurb at the top would cover that.

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.

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

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.

I'm not suggesting that we make all the options accept garbage like the date option in the name of consistency.

I found it helpful to make a table out of this. Key CI = Case Insensitive (now)?, CIP = Case Insensitive Possible (without ambiguities)?, AG = Accepts Garbage (.e.g. @{./.4.minutes./.})?

Before this patch:

|----------------+-----+------+-----|
| What?          | CI? | CIP? | AG? |
|----------------+-----+------+-----|
| sha1           | Y   | -    | N   |
| describeOutput | N   | N    | N   |
| refname        | N   | N    | N   |
| @{<date>}      | Y   | Y    | Y   |
| @{<n>}         | N/A | N/A  | N   |
| @{<n>}         | N/A | N/A  | N   |
| @{upstream}    | N   | Y    | N   |
| @{push}        | N   | Y    | N   |
| ^{<type>}      | N   | Y    | N   |
| ^{/regex}      | N   | N    | N   |
|----------------+-----+------+-----|

After:

|----------------+-----+------+-----|
| What?          | CI? | CIP? | AG? |
|----------------+-----+------+-----|
| sha1           | Y   | -    | N   |
| describeOutput | N   | N    | N   |
| refname        | N   | N    | N   |
| @{<date>}      | Y   | Y    | Y   |
| @{<n>}         | N/A | N/A  | N   |
| @{<n>}         | N/A | N/A  | N   |
| @{upstream}    | Y   | -    | N   |
| @{push}        | Y   | -    | N   |
| ^{<type>}      | Y   | -    | N   |
| ^{/regex}      | N   | N    | N   |
|----------------+-----+------+-----|

I.e. now we have 3x forms that could without any ambiguity be case insensitive, this patch makes that so. We have one option that's very loose about accepting garbage (@{<date>}). I don't see any reason to try to pursue making the other options accept similar garbage.

It's the reason I wrote the patch, and I'm not using consistency as some argument for the change, I just had to take an inventory of all these special forms and found out that these were the odd ones out in the sense that everything else that can be case insensitive is.

That's a quote from Duy's E-Mail. I don't think we should document document anything like that, and my patch doesn't do that.

It is a legit question whether we document things as "unless otherwise noted everything's case insensitive", and then list the exceptions, or "unless otherwise noted everything's case sensitive", and then list the exceptions. My patch does the former, Duy was suggesting the latter.

I don't have any strong preference for either really, but neither locks us into any future promises. It's just a matter of how the current documentation phrases things.

Yes, I agree that phrasing things like Duy suggested offhand in that E-Mail would be broken.

I think this too is really just a reply to what Duy said...

*nod*

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" ?

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.

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.

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>}").

This v2 addresses the feedback on my "rev-parse: match @{u}, @{push} and ^{<type>} case-insensitively" patch. I've split this into a 3 patch series:

Ævar Arnfjörð Bjarmason (3):
rev-parse: match @{upstream}, @{u} and @{push} case-insensitively

Reworded the documentation as Junio suggested in
<[email protected]>.

rev-parse: add @{p} as a synonym for @{push}

While I'm at it why don't we have a shorthand for @{push} like @{upstream}? Add it as @{p}.

rev-parse: match ^{<type>} case-insensitively

Junio didn't want ^{<type>} case-insensitive "for now", so I split it out of the first patch.

I'm not overly excited about it, neither is Junio, so this'll probably be dropped, but I wanted to submit it as a standalone patch in case anyone wanted to pick this up in the future.

 Documentation/revisions.txt   | 15 ++++++++++++---
 git-compat-util.h             |  1 +
 sha1_name.c                   | 14 +++++++-------
 strbuf.c                      |  9 +++++++++
 t/t1450-fsck.sh               |  7 +++++++
 t/t1507-rev-parse-upstream.sh | 15 +++++++++++----
 t/t1511-rev-parse-caret.sh    | 13 +++++++++++++
 t/t1514-rev-parse-push.sh     | 10 ++++++++--
 8 files changed, 68 insertions(+), 16 deletions(-)

2.11.0

Change the revision parsing logic to match @{upstream}, @{u} & @{push} case-insensitively.

Before this change supplying anything except the lower-case forms emits an "unknown revision or path not in the working tree" error. This change makes upper-case & mixed-case versions equivalent to the lower-case versions.

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.

These suffixes now join various other suffixes & special syntax documented in gitrevisions(7) that matches case-insensitively. A table showing the status of the various forms documented there before & after this patch is shown below. The key for the table is:

- CI = Case Insensitive
- CIP = Case Insensitive Possible (without ambiguities)
- AG = Accepts Garbage (.e.g. @{./.4.minutes./.})

Before this change:

    |----------------+-----+------+-----|
    | What?          | CI? | CIP? | AG? |
    |----------------+-----+------+-----|
    | sha1           | Y   | -    | N   |
    | describeOutput | N   | N    | N   |
    | refname        | N   | N    | N   |
    | @{<date>}      | Y   | Y    | Y   |
    | @{<n>}         | N/A | N/A  | N   |
    | @{-<n>}        | N/A | N/A  | N   |
    | @{upstream}    | N   | Y    | N   |
    | @{push}        | N   | Y    | N   |
    | ^{<type>}      | N   | Y    | N   |
    | ^{/regex}      | N   | N    | N   |
    |----------------+-----+------+-----|

After it:

    |----------------+-----+------+-----|
    | What?          | CI? | CIP? | AG? |
    |----------------+-----+------+-----|
    | sha1           | Y   | -    | N   |
    | describeOutput | N   | N    | N   |
    | refname        | N   | N    | N   |
    | @{<date>}      | Y   | Y    | Y   |
    | @{<n>}         | N/A | N/A  | N   |
    | @{-<n>}        | N/A | N/A  | N   |
    | @{upstream}    | Y   | -    | N   |
    | @{push}        | Y   | -    | N   |
    | ^{<type>}      | N   | Y    | N   |
    | ^{/regex}      | N   | N    | N   |
    |----------------+-----+------+-----|

The ^{<type>} suffix could be changed to be case-insensitive as well without introducing any ambiguities. That was included in an earlier version of this patch, but Junio objected to its inclusion in [2].

This change was independently authored to scratch a longtime itch, but when I was about to submit it I discovered that a similar patch had been submitted unsuccessfully before by Conrad Irwin in August 2011 as "rev-parse: Allow @{U} as a synonym for @{u}"[1].

The tests for this patch are more exhaustive than in the 2011 submission. The starting point for them was to first change the code to only support upper-case versions of the existing words, seeing what broke, and amending the breaking tests to check upper case & mixed case as appropriate, and where not redundant to other similar tests. The implementation itself is equivalent.

1. <[email protected]>
2. <[email protected]>

---
 Documentation/revisions.txt   |  6 +++++-
 sha1_name.c                   |  2 +-
 t/t1507-rev-parse-upstream.sh | 15 +++++++++++----
 t/t1514-rev-parse-push.sh     |  8 ++++++--
 4 files changed, 23 insertions(+), 8 deletions(-)

index ba11b9c95e..09e0d51b9e 100644
@@ -96,7 +96,8 @@ some output processing may assume ref names in UTF-8.
   refers to the branch that the branch specified by branchname is set to build on
   top of (configured with `branch.<name>.remote` and
   `branch.<name>.merge`).  A missing branchname defaults to the
-  current one.
+  current one. These suffixes are accepted when spelled in uppercase, and
+  they mean the same thing no matter the case.
 
 '<branchname>@\{push\}', e.g. 'master@\{push\}', '@\{push\}'::
   The suffix '@\{push}' reports the branch "where we would push to" if

@@ -122,6 +123,9 @@ refs/remotes/myfork/mybranch
 Note in the example that we set up a triangular workflow, where we pull
 from one location and push to another. In a non-triangular workflow,
 '@\{push}' is the same as '@\{upstream}', and there is no need for it.
++
+This suffix is accepted when spelled in uppercase, and means the same
+thing no matter the case.
 
 '<rev>{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
   A suffix '{caret}' to a revision parameter means the first parent of

index cda9e49b12..d9d1b2fce8 100644
@@ -549,7 +549,7 @@ static inline int at_mark(const char *string, int len,
 	for (i = 0; i < nr; i++) {
 		int suffix_len = strlen(suffix[i]);
 		if (suffix_len <= len
-		    && !memcmp(string, suffix[i], suffix_len))
+		    && !strncasecmp(string, suffix[i], suffix_len))
 			return suffix_len;
 	}
 	return 0;

index 46ef1f22dc..b23c4e3fab 100755
@@ -46,11 +46,14 @@ error_message () {
 }
 
 test_expect_success '@{upstream} resolves to correct full name' '
-	test refs/remotes/origin/master = "$(full_name @{upstream})"
+	test refs/remotes/origin/master = "$(full_name @{upstream})" &&
+	test refs/remotes/origin/master = "$(full_name @{UPSTREAM})" &&
+	test refs/remotes/origin/master = "$(full_name @{UpSTReam})"
 '
 
 test_expect_success '@{u} resolves to correct full name' '
-	test refs/remotes/origin/master = "$(full_name @{u})"
+	test refs/remotes/origin/master = "$(full_name @{u})" &&
+	test refs/remotes/origin/master = "$(full_name @{U})"
 '
 
 test_expect_success 'my-side@{upstream} resolves to correct full name' '

@@ -60,6 +63,8 @@ test_expect_success 'my-side@{upstream} resolves to correct full name' '
 test_expect_success 'upstream of branch with @ in middle' '
 	full_name fun@ny@{u} >actual &&
 	echo refs/remotes/origin/side >expect &&
+	test_cmp expect actual &&
+	full_name fun@ny@{U} >actual &&
 	test_cmp expect actual
 '
 

@@ -96,12 +101,14 @@ test_expect_success 'not-tracking@{u} fails' '
 test_expect_success '<branch>@{u}@{1} resolves correctly' '
 	test_commit 6 &&
 	(cd clone && git fetch) &&
-	test 5 = $(commit_subject my-side@{u}@{1})
+	test 5 = $(commit_subject my-side@{u}@{1}) &&
+	test 5 = $(commit_subject my-side@{U}@{1})
 '
 
 test_expect_success '@{u} without specifying branch fails on a detached HEAD' '
 	git checkout HEAD^0 &&
-	test_must_fail git rev-parse @{u}
+	test_must_fail git rev-parse @{u} &&
+	test_must_fail git rev-parse @{U}
 '
 
 test_expect_success 'checkout -b new my-side@{u} forks from the same' '

index 623a32aa6e..788cc91e45 100755
@@ -24,12 +24,16 @@ test_expect_success 'setup' '
 
 test_expect_success '@{push} with default=nothing' '
 	test_config push.default nothing &&
-	test_must_fail git rev-parse master@{push}
+	test_must_fail git rev-parse master@{push} &&
+	test_must_fail git rev-parse master@{PUSH} &&
+	test_must_fail git rev-parse master@{PuSH}
 '
 
 test_expect_success '@{push} with default=simple' '
 	test_config push.default simple &&
-	resolve master@{push} refs/remotes/origin/master
+	resolve master@{push} refs/remotes/origin/master &&
+	resolve master@{PUSH} refs/remotes/origin/master &&
+	resolve master@{pUSh} refs/remotes/origin/master
 '
 
 test_expect_success 'triangular @{push} fails with default=simple' '

2.11.0

We try not to be personal in our log message. It's not like my objection weighs heavier than objections from others. The same number of bytes in the log message can better to spent to allow readers of "git log" independently to judge the rationle without referring to external material.

Perhaps replace this entire paragraph, including the URL in [2], with something like

The ^{type} suffix is not made case-insensitive, because other places that take <type> like "cat-file -t <type>" do want them case sensitively (after all we never declared that type names are case insensitive). Allowing case-insensitive typename only with this syntax will make the resulting Git as a whole inconsistent.

Other than that, the change to the code and the new tests all makes sense to me.

Thanks.

Change the revision parsing logic to match @{upstream}, @{u} & @{push} case-insensitively.

Before this change supplying anything except the lower-case forms emits an "unknown revision or path not in the working tree" error. This change makes upper-case & mixed-case versions equivalent to the lower-case versions.

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.

These suffixes now join various other suffixes & special syntax documented in gitrevisions(7) that matches case-insensitively. A table showing the status of the various forms documented there before & after this patch is shown below. The key for the table is:

- CI = Case Insensitive
- CIP = Case Insensitive Possible (without ambiguities)
- AG = Accepts Garbage (.e.g. @{./.4.minutes./.})

Before this change:

    |----------------+-----+------+-----|
    | What?          | CI? | CIP? | AG? |
    |----------------+-----+------+-----|
    | sha1           | Y   | -    | N   |
    | describeOutput | N   | N    | N   |
    | refname        | N   | N    | N   |
    | @{<date>}      | Y   | Y    | Y   |
    | @{<n>}         | N/A | N/A  | N   |
    | @{-<n>}        | N/A | N/A  | N   |
    | @{upstream}    | N   | Y    | N   |
    | @{push}        | N   | Y    | N   |
    | ^{<type>}      | N   | Y    | N   |
    | ^{/regex}      | N   | N    | N   |
    |----------------+-----+------+-----|

After it:

    |----------------+-----+------+-----|
    | What?          | CI? | CIP? | AG? |
    |----------------+-----+------+-----|
    | sha1           | Y   | -    | N   |
    | describeOutput | N   | N    | N   |
    | refname        | N   | N    | N   |
    | @{<date>}      | Y   | Y    | Y   |
    | @{<n>}         | N/A | N/A  | N   |
    | @{-<n>}        | N/A | N/A  | N   |
    | @{upstream}    | Y   | -    | N   |
    | @{push}        | Y   | -    | N   |
    | ^{<type>}      | N   | Y    | N   |
    | ^{/regex}      | N   | N    | N   |
    |----------------+-----+------+-----|

The ^{<type>} suffix is not made case-insensitive, because other places that take <type> like "cat-file -t <type>" do want them case sensitively (after all we never declared that type names are case insensitive). Allowing case-insensitive typename only with this syntax will make the resulting Git as a whole inconsistent.

This change was independently authored to scratch a longtime itch, but when I was about to submit it I discovered that a similar patch had been submitted unsuccessfully before by Conrad Irwin in August 2011 as "rev-parse: Allow @{U} as a synonym for
@{u}" (<[email protected]>).

The tests for this patch are more exhaustive than in the 2011 submission. The starting point for them was to first change the code to only support upper-case versions of the existing words, seeing what broke, and amending the breaking tests to check upper case & mixed case as appropriate, and where not redundant to other similar tests. The implementation itself is equivalent.

---

FWIW I just cited it because you went into a lot more detail in your message, and thought I'd link to it in lieu of trying to paraphrase it at even greater length.

Makes sense, replaced that note with that summary. Here's hopefully a final v3 with that change. I've omitted the other two patches as noted in the discussion about those two, I don't think it makes sense to include them.

 Documentation/revisions.txt   |  6 +++++-
 sha1_name.c                   |  2 +-
 t/t1507-rev-parse-upstream.sh | 15 +++++++++++----
 t/t1514-rev-parse-push.sh     |  8 ++++++--
 4 files changed, 23 insertions(+), 8 deletions(-)

index ba11b9c95e..09e0d51b9e 100644
@@ -96,7 +96,8 @@ some output processing may assume ref names in UTF-8.
   refers to the branch that the branch specified by branchname is set to build on
   top of (configured with `branch.<name>.remote` and
   `branch.<name>.merge`).  A missing branchname defaults to the
-  current one.
+  current one. These suffixes are accepted when spelled in uppercase, and
+  they mean the same thing no matter the case.
 
 '<branchname>@\{push\}', e.g. 'master@\{push\}', '@\{push\}'::
   The suffix '@\{push}' reports the branch "where we would push to" if

@@ -122,6 +123,9 @@ refs/remotes/myfork/mybranch
 Note in the example that we set up a triangular workflow, where we pull
 from one location and push to another. In a non-triangular workflow,
 '@\{push}' is the same as '@\{upstream}', and there is no need for it.
++
+This suffix is accepted when spelled in uppercase, and means the same
+thing no matter the case.
 
 '<rev>{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
   A suffix '{caret}' to a revision parameter means the first parent of

index cda9e49b12..d9d1b2fce8 100644
@@ -549,7 +549,7 @@ static inline int at_mark(const char *string, int len,
 	for (i = 0; i < nr; i++) {
 		int suffix_len = strlen(suffix[i]);
 		if (suffix_len <= len
-		    && !memcmp(string, suffix[i], suffix_len))
+		    && !strncasecmp(string, suffix[i], suffix_len))
 			return suffix_len;
 	}
 	return 0;

index 46ef1f22dc..b23c4e3fab 100755
@@ -46,11 +46,14 @@ error_message () {
 }
 
 test_expect_success '@{upstream} resolves to correct full name' '
-	test refs/remotes/origin/master = "$(full_name @{upstream})"
+	test refs/remotes/origin/master = "$(full_name @{upstream})" &&
+	test refs/remotes/origin/master = "$(full_name @{UPSTREAM})" &&
+	test refs/remotes/origin/master = "$(full_name @{UpSTReam})"
 '
 
 test_expect_success '@{u} resolves to correct full name' '
-	test refs/remotes/origin/master = "$(full_name @{u})"
+	test refs/remotes/origin/master = "$(full_name @{u})" &&
+	test refs/remotes/origin/master = "$(full_name @{U})"
 '
 
 test_expect_success 'my-side@{upstream} resolves to correct full name' '

@@ -60,6 +63,8 @@ test_expect_success 'my-side@{upstream} resolves to correct full name' '
 test_expect_success 'upstream of branch with @ in middle' '
 	full_name fun@ny@{u} >actual &&
 	echo refs/remotes/origin/side >expect &&
+	test_cmp expect actual &&
+	full_name fun@ny@{U} >actual &&
 	test_cmp expect actual
 '
 

@@ -96,12 +101,14 @@ test_expect_success 'not-tracking@{u} fails' '
 test_expect_success '<branch>@{u}@{1} resolves correctly' '
 	test_commit 6 &&
 	(cd clone && git fetch) &&
-	test 5 = $(commit_subject my-side@{u}@{1})
+	test 5 = $(commit_subject my-side@{u}@{1}) &&
+	test 5 = $(commit_subject my-side@{U}@{1})
 '
 
 test_expect_success '@{u} without specifying branch fails on a detached HEAD' '
 	git checkout HEAD^0 &&
-	test_must_fail git rev-parse @{u}
+	test_must_fail git rev-parse @{u} &&
+	test_must_fail git rev-parse @{U}
 '
 
 test_expect_success 'checkout -b new my-side@{u} forks from the same' '

index 623a32aa6e..788cc91e45 100755
@@ -24,12 +24,16 @@ test_expect_success 'setup' '
 
 test_expect_success '@{push} with default=nothing' '
 	test_config push.default nothing &&
-	test_must_fail git rev-parse master@{push}
+	test_must_fail git rev-parse master@{push} &&
+	test_must_fail git rev-parse master@{PUSH} &&
+	test_must_fail git rev-parse master@{PuSH}
 '
 
 test_expect_success '@{push} with default=simple' '
 	test_config push.default simple &&
-	resolve master@{push} refs/remotes/origin/master
+	resolve master@{push} refs/remotes/origin/master &&
+	resolve master@{PUSH} refs/remotes/origin/master &&
+	resolve master@{pUSh} refs/remotes/origin/master
 '
 
 test_expect_success 'triangular @{push} fails with default=simple' '

2.11.0

As we are not touching ^{<type>} or ^{/regex}, and it is obvious numbers do not have cases, I'll trim this down to focus only on things that are relevant while queuing:

    Before this change:

        |----------------+-----+------+-----|
        | What?          | CI? | CIP? | AG? |
        |----------------+-----+------+-----|
        | @{<date>}      | Y   | Y    | Y   |
        | @{upstream}    | N   | Y    | N   |
        | @{push}        | N   | Y    | N   |
        |----------------+-----+------+-----|

    After it:

        |----------------+-----+------+-----|
        | What?          | CI? | CIP? | AG? |
        |----------------+-----+------+-----|
        | @{<date>}      | Y   | Y    | Y   |
        | @{upstream}    | Y   | Y    | N   |
        | @{push}        | Y   | Y    | N   |
        |----------------+-----+------+-----|

should be sufficient to highlight that it was possible to safely make these two things case insensitive, and we made so.

For that matter, I do not know the value of AG? field---it only serves to show that @{<approxidate>} is an odd-man out and cannot be used as a good example to follow, but I am too lazy to remove it ;-)

Thanks.

All of the above sounds good, thanks for fixing it up.

Add @{p} as a shorthand for @{push} for consistency with the @{u} shorthand for @{upstream}.

This wasn't added when @{push} was introduced in commit adfe5d0434 ("sha1_name: implement @{push} shorthand", 2015-05-21), but it can be added without any ambiguity and saves the user some typing.

---
 Documentation/revisions.txt | 8 ++++----
 sha1_name.c                 | 2 +-
 t/t1514-rev-parse-push.sh   | 2 ++
 3 files changed, 7 insertions(+), 5 deletions(-)

index 09e0d51b9e..5fe90e411d 100644
@@ -99,8 +99,8 @@ some output processing may assume ref names in UTF-8.
   current one. These suffixes are accepted when spelled in uppercase, and
   they mean the same thing no matter the case.
 
-'<branchname>@\{push\}', e.g. 'master@\{push\}', '@\{push\}'::
-  The suffix '@\{push}' reports the branch "where we would push to" if
+'<branchname>@\{push\}', e.g. 'master@\{push\}', '@\{p\}'::
+  The suffix '@\{push}' (short form '@\{push}') reports the branch "where we would push to" if
   `git push` were run while `branchname` was checked out (or the current
   `HEAD` if no branchname is specified). Since our push destination is
   in a remote repository, of course, we report the local tracking branch

@@ -124,8 +124,8 @@ Note in the example that we set up a triangular workflow, where we pull
 from one location and push to another. In a non-triangular workflow,
 '@\{push}' is the same as '@\{upstream}', and there is no need for it.
 +
-This suffix is accepted when spelled in uppercase, and means the same
-thing no matter the case.
+These suffixes are accepted when spelled in uppercase, and they mean
+the same thing no matter the case.
 
 '<rev>{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
   A suffix '{caret}' to a revision parameter means the first parent of

index d9d1b2fce8..2deb9bfdf6 100644
@@ -563,7 +563,7 @@ static inline int upstream_mark(const char *string, int len)
 
 static inline int push_mark(const char *string, int len)
 {
-	const char *suffix[] = { "@{push}" };
+	const char *suffix[] = { "@{push}", "@{p}" };
 	return at_mark(string, len, suffix, ARRAY_SIZE(suffix));
 }
 

index 788cc91e45..db9aaf88f8 100755
@@ -31,6 +31,8 @@ test_expect_success '@{push} with default=nothing' '
 
 test_expect_success '@{push} with default=simple' '
 	test_config push.default simple &&
+	resolve master@{p} refs/remotes/origin/master &&
+	resolve master@{P} refs/remotes/origin/master &&
 	resolve master@{push} refs/remotes/origin/master &&
 	resolve master@{PUSH} refs/remotes/origin/master &&
 	resolve master@{pUSh} refs/remotes/origin/master

2.11.0

It _can_ be added, but it was intentionally avoided at the time because there was discussion of adding other p-words, including:

- @{pull} as a synonym for @{upstream} (and to better match @{push})

- @{publish}, which was some similar-ish system that was based on per-branch config, but the patches were never merged.

It's been a few years with neither of those things happening, so in a sense it may be safe to add it now. OTOH, if users are not clamoring for @{p} and it is just being added "because we can", maybe that is not a good reason.

Yeah let's just drop this.

Yup, my mistake.

Change the revision parsing logic to match ^{commit}, ^{tree}, ^{blob} etc. case-insensitively.

Before this change supplying anything except the lower-case forms emits an "unknown revision or path not in the working tree" error. This change makes upper-case & mixed-case versions equivalent to the lower-case versions.

The rationale for this change is the same as for making @{upstream} and related suffixes case-insensitive in "rev-parse: match @{upstream}, @{u} and @{push} case-insensitively", but unlike those suffixes this change introduces the potential confusion of accepting TREE or BLOB here, but not as an argument to e.g. "cat-file -t <type>" or "hash-object -t <type>".

---
 Documentation/revisions.txt |  5 +++++
 git-compat-util.h           |  1 +
 sha1_name.c                 | 10 +++++-----
 strbuf.c                    |  9 +++++++++
 t/t1450-fsck.sh             |  7 +++++++
 t/t1511-rev-parse-caret.sh  | 13 +++++++++++++
 6 files changed, 40 insertions(+), 5 deletions(-)

index 5fe90e411d..136e26c05d 100644
@@ -162,6 +162,11 @@ it does not have to be dereferenced even once to get to an object.
 +
 'rev{caret}\{tag\}' can be used to ensure that 'rev' identifies an
 existing tag object.
++
+The {caret}{<type>} part is matched case-insensitively. So
+e.g. '{caret}\{commit\}' can be equivalently specified as
+'{caret}\{COMMIT\}', '{caret}\{Commit\}' etc., '{caret}\{tree\}' as
+'{caret}\{TREE\}' and so forth.
 
 '<rev>{caret}{}', e.g. 'v0.99.8{caret}{}'::
   A suffix '{caret}' followed by an empty brace pair

index 8a4a3f85e7..4a03934ef3 100644
@@ -448,6 +448,7 @@ extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
 extern int starts_with(const char *str, const char *prefix);
+extern int starts_with_icase(const char *str, const char *prefix);
 
 /*
  * If the string "str" begins with the string found in "prefix", return 1.

index 2deb9bfdf6..107246bd2d 100644
@@ -821,15 +821,15 @@ static int peel_onion(const char *name, int len, unsigned char *sha1,
 		return -1;
 
 	sp++; /* beginning of type name, or closing brace for empty */
-	if (starts_with(sp, "commit}"))
+	if (starts_with_icase(sp, "commit}"))
 		expected_type = OBJ_COMMIT;
-	else if (starts_with(sp, "tag}"))
+	else if (starts_with_icase(sp, "tag}"))
 		expected_type = OBJ_TAG;
-	else if (starts_with(sp, "tree}"))
+	else if (starts_with_icase(sp, "tree}"))
 		expected_type = OBJ_TREE;
-	else if (starts_with(sp, "blob}"))
+	else if (starts_with_icase(sp, "blob}"))
 		expected_type = OBJ_BLOB;
-	else if (starts_with(sp, "object}"))
+	else if (starts_with_icase(sp, "object}"))
 		expected_type = OBJ_ANY;
 	else if (sp[0] == '}')
 		expected_type = OBJ_NONE;

index ace58e7367..7d4a59bca6 100644
@@ -11,6 +11,15 @@ int starts_with(const char *str, const char *prefix)
 			return 0;
 }
 
+int starts_with_icase(const char *str, const char *prefix)
+{
+	for (; ; str++, prefix++)
+		if (!*prefix)
+			return 1;
+		else if (tolower(*str) != tolower(*prefix))
+			return 0;
+}
+
 /*
  * Used as the default ->buf value, so that people can always assume
  * buf is non NULL and ->buf is NUL terminated even for a freshly

index 33a51c9a67..b6c1989671 100755
@@ -505,6 +505,13 @@ test_expect_success 'fsck notices missing tagged object' '
 	test_must_fail git -C missing fsck
 '
 
+test_expect_success 'fsck notices missing tagged object with case insensitive {blob}' '
+	create_repo_missing tag^{BLOB} &&
+	test_must_fail git -C missing fsck &&
+	create_repo_missing tag^{BloB} &&
+	test_must_fail git -C missing fsck
+'
+
 test_expect_success 'fsck notices ref pointing to missing commit' '
 	create_repo_missing HEAD &&
 	test_must_fail git -C missing fsck

index e0a49a651f..56750f99c6 100755
@@ -48,6 +48,10 @@ test_expect_success 'ref^{commit}' '
 	git rev-parse ref >expected &&
 	git rev-parse ref^{commit} >actual &&
 	test_cmp expected actual &&
+	git rev-parse ref^{COMMIT} >actual &&
+	test_cmp expected actual &&
+	git rev-parse ref^{CoMMiT} >actual &&
+	test_cmp expected actual &&
 	git rev-parse commit-tag^{commit} >actual &&
 	test_cmp expected actual &&
 	test_must_fail git rev-parse tree-tag^{commit} &&

@@ -58,6 +62,10 @@ test_expect_success 'ref^{tree}' '
 	echo $TREE_SHA1 >expected &&
 	git rev-parse ref^{tree} >actual &&
 	test_cmp expected actual &&
+	git rev-parse ref^{TREE} >actual &&
+	test_cmp expected actual &&
+	git rev-parse ref^{TrEe} >actual &&
+	test_cmp expected actual &&
 	git rev-parse commit-tag^{tree} >actual &&
 	test_cmp expected actual &&
 	git rev-parse tree-tag^{tree} >actual &&

@@ -67,8 +75,13 @@ test_expect_success 'ref^{tree}' '
 
 test_expect_success 'ref^{tag}' '
 	test_must_fail git rev-parse HEAD^{tag} &&
+	test_must_fail git rev-parse HEAD^{TAG} &&
 	git rev-parse commit-tag >expected &&
 	git rev-parse commit-tag^{tag} >actual &&
+	test_cmp expected actual &&
+	git rev-parse commit-tag^{TAG} >actual &&
+	test_cmp expected actual &&
+	git rev-parse commit-tag^{Tag} >actual &&
 	test_cmp expected actual
 '
 

2.11.0

It's not "potential confusion". This closes the door for us to introduce "TREE" as a separate object type in the future.

If we agree to make a declaration that all typenames are officially spelled in lowercase [*1*] and at the UI level we accept typenames spelled in any case, then we can adopt this change (and then we need to update "cat-file -t" etc. to match it).

I do not at all mind to see if the list concensus is to make such a declaration and permanently close the door for typenames that are not lowercase, and after seeing such a concensus I'd gladly appreciate this patch, but I do not want to see a change like this that decides the future of the system, pretending as an innocuous change, sneaked in without making sure that everybody is aware of its implications.

[Footnote]

*1* "officially spelled in lowercase" is necessary to avoid
confusion, because we are not making typenames case insensitive, allowing an object whose raw-bytes in its loose object representation before deflating begins with "COMMIT" and take it as an object of <commit> type. Instead, such a declaration will make such an object an invalid one (as opposed to "object of an unknown type", as it currently is).

FWIW, I cannot see us ever adding TREE (or Tree) as a separate type. It's too confusing for no gain. We'd call it "tree2" or something more obvious.

So I don't mind closing that door, but I'm not sure if a partial conversion (where some commands are case-insensitive but others aren't yet) might not leave us in a more confusing place.

I dunno. I guess I have never wanted to type "^{Tree}" in the first place, so I do not personally see the _benefit_. Which makes it easy to see even small negatives as a net loss.

-Peff

In case it was not clear, I didn't mean to say I _want_ to leave that door open. Well, I cannot imagine it was unclear, as I said I do not at all mind declaring that all object names will be lowercase to allow us freely downcase what we got at the UI level.

Exactly.

As to the potential _benefit_, I do not see much either myself, but we already are seeing somebody cared enough to throw us a patch, so to some people there are clearly perceived benefit. I do not think closing the door for typenames that are not lowercase is a negative change at all.

I just wanted the patch to make it clear that it is making such a system-wide design decision and casting it in stone. Which includes that "cat-file <type>" and "hash-object -t <type>" get the same case-insensitivity update and probably writing that design decision down somewhere in the documentation, perhaps in the glossary where we talk about the "object type".

No, I understood that. You just mentioned "list consensus" so I was trying to give my two cents. ;)

By negative, I just meant potential confusion when we are half-way there (e.g., "foo^{TREE}" works but "git cat-file TREE foo" does not).

Yes, I agree that that is the right path forward.

-Peff

I don't either, I think this patch should just be dropped.

As noted in the cover letter[1] I carved this off from the rest of the series just in case anyone wanted this, either now or in the future.

I originally implemented this for consistency with @{U} because it's another thing that can be made case-insensitive unambiguously as far as the rev-parse syntax is concerned, but as you/Junio note here the tree/blob/object etc. names exist in a lot more places, so just making this particular thing case insensitive wouldn't make sense, and has little benefit.

1. <[email protected]>

[Dropping Richard Hansen from CC, his E-Mail bounces, changed jobs probably...]

Yeah, static inside sha1_name.c is OK; people with newer needs can move it later if necessary.

I'd have called starts_with_icase(), though.

I was thinking that I had written this same function before (in a patch that didn't end up merged), but it was actually the related skip_prefix_icase(). I'm fine with it either next to starts_with() or as a static next to its callers.

-Peff