Subject: Re: [JGIT PATCH 10/12] Match config subsection names using case sensitive search

From: Robin Rosenberg <[email-masked]>
Date: Wed, 22 Jul 2009 23:37:55 +0200

onsdag 22 juli 2009 13:11:07 skrev Constantine Plotnikov <[email-masked]>:
> This patch is incomplete. The method getRawEntry(...) and
> setStringList(...) should be fixed as part of this patch too. There is
> subsection is converted to lowercase. I was planning to submit it as
> separate patch.
> 
> Also I'm somewhat bothered by usage of toLowerCase() without locale
> specified and equalsIgnoreCase(). When turkish locale is default one
> there could be surprising results with the letter "I".  The program:
> 
> import java.util.Locale;
> public class Test {
> 	public static void main(String[] args) {
> 		Locale tr_TR = new Locale("tr", "TR");
> 		System.out.printf("i = U+%04x LC(I, tr_TR) = U+%04x\n", (int)'i',
> (int)"I".toLowerCase(tr_TR).charAt(0));
> 		System.out.printf("I = U+%04x UC(i, tr_TR) = U+%04x\n", (int)'I',
> (int)"i".toUpperCase(tr_TR).charAt(0));
> 	}
> }
> 
> Gives the following output:
> 
> i = U+0069 LC(I, tr_TR) = U+0131
> I = U+0049 UC(i, tr_TR) = U+0130
> 
> So I suggest to explicitly use Locale.US for all toLowerCase()
> invocation in Config class just in case and to replace
> equalsIgnoreCase() with something else. But this possibly should be
> some other patch series. I do not know what C git doing in case
> turkish locale and whether it is a bug or "feature".

Good catch. C Git operates in "C" locale (default for C/C++) so it should
not be a problem. We might have other places where this could be a problem.
Using Locale.US should be the right choice here since unicode is not allowed
in sections and keys anyway.  (Btw, java.lang.String.regionMatches has some info
on case insensitive matching for those interested, serving a reminder for
why home-brewed logic isn't always the best.)

As you say we should make this a separate patch.

--  robin