git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Constantine Plotnikov <constantine.plotnikov@gmail.com>
To: "Shawn O. Pearce" <spearce@spearce.org>
Cc: Robin Rosenberg <robin.rosenberg@dewire.com>, git@vger.kernel.org
Subject: Re: [JGIT PATCH 10/12] Match config subsection names using case  sensitive search
Date: Wed, 22 Jul 2009 15:11:07 +0400	[thread overview]
Message-ID: <85647ef50907220411w356000bcuda21e9318eab094@mail.gmail.com> (raw)
In-Reply-To: <1248207570-13880-11-git-send-email-spearce@spearce.org>

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

Regards,
Constantine

On Wed, Jul 22, 2009 at 12:19 AM, Shawn O. Pearce<spearce@spearce.org> wrote:
> The subsection name is case sensitive, and should be matched as such.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>  .../src/org/spearce/jgit/lib/Config.java           |   19 ++++++++++++++-----
>  1 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> index e379c37..974ffea 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> @@ -4,6 +4,7 @@
>  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
>  * Copyright (C) 2008, Thad Hughes <thadh@thad.corp.google.com>
>  * Copyright (C) 2009, JetBrains s.r.o.
> + * Copyright (C) 2009, Google, Inc.
>  *
>  * All rights reserved.
>  *
> @@ -1024,17 +1025,25 @@ private static String readValue(final BufferedReader r, boolean quote,
>
>                boolean match(final String aSection, final String aSubsection,
>                                final String aKey) {
> -                       return eq(section, aSection) && eq(subsection, aSubsection)
> -                                       && eq(name, aKey);
> +                       return eqIgnoreCase(section, aSection)
> +                                       && eqSameCase(subsection, aSubsection)
> +                                       && eqIgnoreCase(name, aKey);
>                }
>
> -               private static boolean eq(final String a, final String b) {
> +               private static boolean eqIgnoreCase(final String a, final String b) {
>                        if (a == null && b == null)
>                                return true;
>                        if (a == null || b == null)
>                                return false;
>                        return a.equalsIgnoreCase(b);
>                }
> -       }
>
> -}
> \ No newline at end of file
> +               private static boolean eqSameCase(final String a, final String b) {
> +                       if (a == null && b == null)
> +                               return true;
> +                       if (a == null || b == null)
> +                               return false;
> +                       return a.equals(b);
> +               }
> +       }
> +}
> --
> 1.6.4.rc1.186.g60aa0c
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

  parent reply	other threads:[~2009-07-22 11:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-21 20:19 [JGIT PATCH 00/12] Cleanup Config class Shawn O. Pearce
2009-07-21 20:19 ` [JGIT PATCH 01/12] Use NB.readFully(File) to slurp complete file contents Shawn O. Pearce
2009-07-21 20:19   ` [JGIT PATCH 02/12] Correct name of fileRead member of Config class Shawn O. Pearce
2009-07-21 20:19     ` [JGIT PATCH 03/12] Add setLong to Config Shawn O. Pearce
2009-07-21 20:19       ` [JGIT PATCH 04/12] Fix Config setInt(..., 0) to store "0" not "0 g" Shawn O. Pearce
2009-07-21 20:19         ` [JGIT PATCH 05/12] Rename Config.unsetString to just unset() Shawn O. Pearce
2009-07-21 20:19           ` [JGIT PATCH 06/12] Remove pointless null assignments in Config Shawn O. Pearce
2009-07-21 20:19             ` [JGIT PATCH 07/12] Clarify section and subsection values in Config code Shawn O. Pearce
2009-07-21 20:19               ` [JGIT PATCH 08/12] Don't subclass PrintWriter when writing the Config Shawn O. Pearce
2009-07-21 20:19                 ` [JGIT PATCH 09/12] Use a Java 5 style iteration over the Config entries list Shawn O. Pearce
2009-07-21 20:19                   ` [JGIT PATCH 10/12] Match config subsection names using case sensitive search Shawn O. Pearce
2009-07-21 20:19                     ` [JGIT PATCH 11/12] Cleanup Config's MAGIC_EMPTY_VALUE to be more safe Shawn O. Pearce
2009-07-21 20:19                       ` [JGIT PATCH 12/12] Remove unreferenced REMOTE_SECTION from RepositoryConfig Shawn O. Pearce
2009-07-21 21:51                       ` [JGIT PATCH 11/12] Cleanup Config's MAGIC_EMPTY_VALUE to be more safe Robin Rosenberg
2009-07-21 21:54                         ` Shawn O. Pearce
2009-07-22 11:11                     ` Constantine Plotnikov [this message]
2009-07-22 21:37                       ` [JGIT PATCH 10/12] Match config subsection names using case sensitive search Robin Rosenberg
2009-07-24 21:34                         ` [PATCH] Ensure Config readers handle case insensitive names correctly Shawn O. Pearce

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=85647ef50907220411w356000bcuda21e9318eab094@mail.gmail.com \
    --to=constantine.plotnikov@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=robin.rosenberg@dewire.com \
    --cc=spearce@spearce.org \
    /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).