Implement wxUILocale::CompareStrings() for Unix systems

This required changing CompareStrings() to be a method of wxUILocale
object, rather than just as a static function, as we must only allocate
the locale_t object once, and not during each to this function, as this
could make it unusably slow when using it as a comparison function when
sorting a large list of strings.

This is also more efficient under Mac, where we can similarly allocate
NSLocale only once and even marginally more efficient under MSW, where
we don't have to construct the locale string during each call. And,
under all platforms, it also simplifies code by separating this function
implementation from the initialization of wxUILocaleImpl.

Also document that case-insensitive comparison is not available under
Unix and adjust the tests accordingly.

include/wx/private/uilocale.h

@@ -60,6 +60,8 @@ public:
     // Functions corresponding to wxUILocale ones.
     virtual wxString GetName() const = 0;
     virtual wxString GetInfo(wxLocaleInfo index, wxLocaleCategory cat) const = 0;
+    virtual int CompareStrings(const wxString& lhs, const wxString& rhs,
+                               int flags) const = 0;
 
     virtual ~wxUILocaleImpl() { }
 };

include/wx/uilocale.h

@@ -55,10 +55,9 @@ public:
     wxString GetInfo(wxLocaleInfo index,
                      wxLocaleCategory cat = wxLOCALE_CAT_DEFAULT) const;
 
-    // Compares two strings, for a locale specified by wxLocaleIdent.
+    // Compares two strings in the order defined by this locale.
-    static int CompareStrings(const wxString& lhs, const wxString& rhs,
+    int CompareStrings(const wxString& lhs, const wxString& rhs,
-                              const wxLocaleIdent& localeId = wxLocaleIdent(),
+                       int flags = wxCompare_CaseSensitive) const;
-                              int flags = wxCompare_CaseSensitive);
 
     // Note that this class is not supposed to be used polymorphically, hence
     // its dtor is not virtual.

interface/wx/uilocale.h

@@ -17,7 +17,12 @@ enum
     /// Compare strings case-sensitively, this is the default.
     wxCompare_CaseSensitive   = 0,
 
-    /// Ignore strings case when comparing.
+    /**
+        Ignore strings case when comparing.
+
+        Note that this flag is not supported under POSIX systems, where it is
+        simply ignored.
+     */
     wxCompare_CaseInsensitive = 1
 };
 
@@ -99,10 +104,9 @@ public:
 
         In the simplest case, this can be used as following:
         @code
-            const wxUILocale loc("fr");
+            const wxUILocale loc(wxLocaleIdent("fr"));
         @endcode
-        but more precise locale identifiers can be used, see wxLocaleIdent
+        see wxLocaleIdent description for more details.
-        description for more details.
 
         If @a localeId is not recognized or not supported, default ("C") locale
         is used instead.
@@ -110,26 +114,24 @@ public:
     explicit wxUILocale(const wxLocaleIdent& localeId);
 
     /**
-        Compares two strings using comparison rules of the given locale.
+        Compares two strings using comparison rules of this locale.
 
         @param lhs
             First comparing string.
         @param rhs
             Second comparing string.
-        @param localeId
-            Represents platform dependent language name.
-            @see wxLocaleIdent for details.
         @param flags
             Can be used to specify whether to compare strings case-sensitively
-            (default) or not, by specifying ::wxCompare_CaseInsensitive.
+            (default) or not, by specifying ::wxCompare_CaseInsensitive (note
+            that this flag only works under MSW and Mac and is simply ignored
+            under the other platforms).
         @return
             -1 if lhs less than rhs.
             0 if lhs equal to rhs.
             1 if lhs greater than rhs.
      */
-    static int CompareStrings(const wxString& lhs, const wxString& rhs,
+    int CompareStrings(const wxString& lhs, const wxString& rhs,
-                              const wxLocaleIdent& localeId = wxLocaleIdent(),
+                       int flags = wxCompare_CaseSensitive) const;
-                              int flags = wxCompare_CaseSensitive);
 
     /**
         Get the platform-dependent name of the current locale.

src/common/uilocale.cpp

@@ -133,6 +133,14 @@ wxString wxUILocale::GetInfo(wxLocaleInfo index, wxLocaleCategory cat) const
     return m_impl->GetInfo(index, cat);
 }
 
+int
+wxUILocale::CompareStrings(const wxString& lhs,
+                           const wxString& rhs,
+                           int flags) const
+{
+    return m_impl->CompareStrings(lhs, rhs, flags);
+}
+
 wxUILocale::~wxUILocale()
 {
     delete m_impl;

src/msw/uilocale.cpp

@@ -64,43 +64,6 @@ static void wxMSWSetThreadUILanguage(LANGID langid)
     }
 }
 
-// Trivial wrapper for ::CompareStringEx().
-//
-// TODO-XP: Drop this when we don't support XP any longer.
-static int wxMSWCompareStringEx(LPCWSTR lpLocaleName,
-                                 DWORD dwCmpFlags,
-                                 LPCWSTR lpString1, //_In_NLS_string_(cchCount1)LPCWCH lpString1,
-                                 int cchCount1,
-                                 LPCWSTR lpString2, //_In_NLS_string_(cchCount2)LPCWCH lpString2,
-                                 int cchCount2,
-                                 LPNLSVERSIONINFO lpVersionInformation,
-                                 LPVOID lpReserved,
-                                 LPARAM lParam)
-{
-    typedef int(WINAPI *CompareStringEx_t)(LPCWSTR,DWORD,LPCWSTR,int,LPCWSTR,int,LPNLSVERSIONINFO,LPVOID,LPARAM);
-    static const CompareStringEx_t INVALID_FUNC_PTR = (CompareStringEx_t)-1;
-
-    static CompareStringEx_t pfnCompareStringEx = INVALID_FUNC_PTR;
-
-    if (pfnCompareStringEx == INVALID_FUNC_PTR)
-    {
-        // Avoid calling CompareStringEx() on XP.
-        if (wxGetWinVersion() >= wxWinVersion_Vista)
-        {
-            wxLoadedDLL dllKernel32(wxS("kernel32.dll"));
-            wxDL_INIT_FUNC(pfn, CompareStringEx, dllKernel32);
-        }
-    }
-
-    if (pfnCompareStringEx)
-    {
-        return pfnCompareStringEx(lpLocaleName, dwCmpFlags, lpString1, cchCount1, lpString2,
-                                    cchCount2, lpVersionInformation, lpReserved, lParam);
-    }
-
-    return 0;
-}
-
 } // anonymous namespace
 
 void wxUseLCID(LCID lcid)
@@ -185,6 +148,16 @@ public:
         return wxGetInfoFromLCID(m_lcid, index, cat);
     }
 
+    int CompareStrings(const wxString& lhs, const wxString& rhs,
+                       int flags) const wxOVERRIDE
+    {
+        // Can't be really implemented on the OS versions where this class is
+        // used.
+        return flags & wxCompare_CaseInsensitive
+                ? lhs.CmpNoCase(rhs)
+                : lhs.Cmp(rhs) ;
+    }
+
 private:
     const LCID m_lcid;
 
@@ -217,6 +190,10 @@ public:
             if ( !ms_SetThreadPreferredUILanguages )
                 return false;
 
+            wxDL_INIT_FUNC(ms_, CompareStringEx, dllKernel32);
+            if ( !ms_CompareStringEx )
+                return false;
+
             s_canUse = 1;
         }
 
@@ -301,6 +278,39 @@ public:
         return str;
     }
 
+    int CompareStrings(const wxString& lhs, const wxString& rhs,
+                       int flags) const wxOVERRIDE
+    {
+        DWORD dwFlags = 0;
+
+        if ( flags & wxCompare_CaseInsensitive )
+            dwFlags |= NORM_IGNORECASE;
+
+        const int ret = ms_CompareStringEx
+            (
+                m_name,
+                dwFlags,
+                lhs.wc_str(), -1,
+                rhs.wc_str(), -1,
+                NULL,               // [out] version information -- not needed
+                wxRESERVED_PARAM,
+                wxRESERVED_PARAM
+            );
+
+        switch ( ret )
+        {
+            case CSTR_LESS_THAN:
+                return -1;
+            case CSTR_EQUAL:
+                return 0;
+            case CSTR_GREATER_THAN:
+                return 1;
+        }
+
+        wxFAIL_MSG(wxS("Unreachable"));
+        return 0;
+    }
+
 private:
     typedef int (WINAPI *GetLocaleInfoEx_t)(LPCWSTR, LCTYPE, LPWSTR, int);
     static GetLocaleInfoEx_t ms_GetLocaleInfoEx;
@@ -308,6 +318,19 @@ private:
     typedef BOOL (WINAPI *SetThreadPreferredUILanguages_t)(DWORD, CONST WCHAR*, ULONG*);
     static SetThreadPreferredUILanguages_t ms_SetThreadPreferredUILanguages;
 
+    // Note: we currently don't use NLSVERSIONINFO output parameter and so we
+    // don't bother dealing with the different sizes of this struct under
+    // different OS versions and define the function type as using "void*" to
+    // avoid using this parameter accidentally. If we ever really need to use
+    // it, we'd need to check the OS version/struct size during run-time.
+    typedef int (WINAPI *CompareStringEx_t)(LPCWSTR, DWORD,
+                                            CONST WCHAR*, int,
+                                            CONST WCHAR*, int,
+                                            void*, // actually LPNLSVERSIONINFO
+                                            void*,
+                                            LPARAM);
+    static CompareStringEx_t ms_CompareStringEx;
+
     wxString DoGetInfo(LCTYPE lctype) const
     {
         wchar_t buf[256];
@@ -327,6 +350,7 @@ private:
 
 wxUILocaleImplName::GetLocaleInfoEx_t wxUILocaleImplName::ms_GetLocaleInfoEx;
 wxUILocaleImplName::SetThreadPreferredUILanguages_t wxUILocaleImplName::ms_SetThreadPreferredUILanguages;
+wxUILocaleImplName::CompareStringEx_t wxUILocaleImplName::ms_CompareStringEx;
 
 // ----------------------------------------------------------------------------
 // wxUILocaleImpl implementation
@@ -382,39 +406,4 @@ wxUILocaleImpl* wxUILocaleImpl::CreateForLocale(const wxLocaleIdent& locId)
     return new wxUILocaleImplName(locId.GetName());
 }
 
-/* static */
-int
-wxUILocale::CompareStrings(const wxString& lhs,
-                           const wxString& rhs,
-                           const wxLocaleIdent& localeId,
-                           int flags)
-{
-    DWORD dwFlags = 0;
-
-    if ( flags & wxCompare_CaseInsensitive )
-        dwFlags |= NORM_IGNORECASE;
-
-    int ret = wxMSWCompareStringEx(
-        localeId.IsDefault() ? LOCALE_NAME_USER_DEFAULT
-                             : static_cast<LPCWSTR>(localeId.GetName().wc_str()),
-        dwFlags,
-        static_cast<LPCWSTR>(lhs.wc_str()), -1,
-        static_cast<LPCWSTR>(rhs.wc_str()), -1,
-        NULL,               // [out] version information -- not needed
-        wxRESERVED_PARAM,
-        wxRESERVED_PARAM);
-
-    switch (ret)
-    {
-    case CSTR_LESS_THAN:
-        return -1;
-    case CSTR_EQUAL:
-        return 0;
-    case CSTR_GREATER_THAN:
-        return 1;
-    }
-
-    return 0;
-}
-
 #endif // wxUSE_INTL

src/osx/core/uilocale.mm

@@ -91,6 +91,8 @@ public:
     bool Use() wxOVERRIDE;
     wxString GetName() const wxOVERRIDE;
     wxString GetInfo(wxLocaleInfo index, wxLocaleCategory cat) const wxOVERRIDE;
+    int CompareStrings(const wxString& lhs, const wxString& rhs,
+                       int flags) const wxOVERRIDE;
 
 private:
     wxCFRef<CFLocaleRef> m_cfloc;
@@ -141,29 +143,22 @@ wxUILocaleImpl* wxUILocaleImpl::CreateForLocale(const wxLocaleIdent& locId)
     return wxUILocaleImplCF::Create(locId);
 }
 
-/* static */
 int
-wxUILocale::CompareStrings(const wxString& lhs,
+wxUILocaleImplCF::CompareStrings(const wxString& lhs, const wxString& rhs,
-                           const wxString& rhs,
+                                 int flags) const
-                           const wxLocaleIdent& localeId,
-                           int flags)
 {
     NSString *ns_lhs = [NSString stringWithCString:lhs.ToStdString(wxConvUTF8).c_str()
                                  encoding:NSUTF8StringEncoding];
     NSString *ns_rhs = [NSString stringWithCString:rhs.ToStdString(wxConvUTF8).c_str()
                                  encoding:NSUTF8StringEncoding];
-    NSString *ns_locale_id = [NSString stringWithCString:localeId.GetName().ToStdString(wxConvUTF8).c_str()
-                                       encoding:NSUTF8StringEncoding];
     NSInteger options = 0;
     if ( flags & wxCompare_CaseInsensitive )
         options |= NSCaseInsensitiveSearch;
 
-    NSLocale *locale = [[NSLocale alloc] initWithLocaleIdentifier:ns_locale_id];
-
     NSComparisonResult ret = [ns_lhs compare:ns_rhs
                                      options:options
                                      range:(NSRange){0, [ns_lhs length]}
-                                     locale:locale];
+                                     locale:(id)(CFLocaleRef)m_cfloc];
 
     switch (ret)
     {

src/unix/uilocale.cpp

@@ -47,6 +47,8 @@ public:
     bool Use() wxOVERRIDE;
     wxString GetName() const wxOVERRIDE;
     wxString GetInfo(wxLocaleInfo index, wxLocaleCategory cat) const wxOVERRIDE;
+    int CompareStrings(const wxString& lhs, const wxString& rhs,
+                       int flags) const wxOVERRIDE;
 
 private:
 #ifdef HAVE_LANGINFO_H
@@ -326,6 +328,28 @@ wxUILocaleImplUnix::GetInfo(wxLocaleInfo index, wxLocaleCategory cat) const
 #endif // HAVE_LANGINFO_H/!HAVE_LANGINFO_H
 }
 
+int
+wxUILocaleImplUnix::CompareStrings(const wxString& lhs, const wxString& rhs,
+                                   int WXUNUSED(flags)) const
+{
+    int rc;
+
+#ifdef HAVE_LOCALE_T
+    if ( m_locale )
+        rc = wcscoll_l(lhs.wc_str(), rhs.wc_str(), m_locale);
+    else
+#endif // HAVE_LOCALE_T
+        rc = wcscoll(lhs.wc_str(), rhs.wc_str());
+
+    if ( rc < 0 )
+        return -1;
+
+    if ( rc > 0 )
+        return 1;
+
+    return 0;
+}
+
 /* static */
 wxUILocaleImpl* wxUILocaleImpl::CreateStdC()
 {

tests/intl/intltest.cpp

@@ -260,44 +260,49 @@ TEST_CASE("wxUILocale::CompareStrings", "[uilocale]")
 {
     SECTION("English")
     {
-        const wxLocaleIdent l("en");
+        const wxUILocale l("en");
 
         // This is not very interesting, but check that comparison works at all.
-        CHECK( wxUILocale::CompareStrings("x", "x", l) ==  0 );
+        CHECK( l.CompareStrings("x", "x") ==  0 );
-        CHECK( wxUILocale::CompareStrings("x", "y", l) == -1 );
+        CHECK( l.CompareStrings("x", "y") == -1 );
-        CHECK( wxUILocale::CompareStrings("z", "y", l) == +1 );
+        CHECK( l.CompareStrings("z", "y") == +1 );
 
         // Also check for some degenerate cases.
-        CHECK( wxUILocale::CompareStrings("", "",  l) ==  0 );
+        CHECK( l.CompareStrings("", "")  ==  0 );
-        CHECK( wxUILocale::CompareStrings("", "a", l) == -1 );
+        CHECK( l.CompareStrings("", "a") == -1 );
 
         // And for case handling.
-        CHECK( wxUILocale::CompareStrings("a", "A",  l) == -1 );
+        CHECK( l.CompareStrings("a", "A") == -1 );
-        CHECK( wxUILocale::CompareStrings("a", "A",  l,
+        CHECK( l.CompareStrings("b", "A") ==  1 );
-                                          wxCompare_CaseInsensitive) == 0 );
+        CHECK( l.CompareStrings("B", "a") ==  1 );
-        CHECK( wxUILocale::CompareStrings("b", "A",  l) ==  1 );
+
-        CHECK( wxUILocale::CompareStrings("B", "a",  l) ==  1 );
+        // Case insensitive comparison is not supported with POSIX APIs.
+#if defined(__WINDOWS__) || defined(__WXOSX__)
+        CHECK( l.CompareStrings("a", "A", wxCompare_CaseInsensitive) == 0 );
+#endif
     }
 
     SECTION("German")
     {
-        const wxLocaleIdent l = wxLocaleIdent("de").Region("DE");
+        const wxUILocale l(wxLocaleIdent("de").Region("DE"));
 
         // This is more interesting and shows that CompareStrings() uses German
         // dictionary rules (DIN 5007-1 variant 1).
-        CHECK( wxUILocale::CompareStrings("a",  u8("ä"), l) == -1 );
+        CHECK( l.CompareStrings("a",  u8("ä")) == -1 );
-        CHECK( wxUILocale::CompareStrings(u8("ä"), "ae", l) == -1 );
+        CHECK( l.CompareStrings(u8("ä"), "ae") == -1 );
-        CHECK( wxUILocale::CompareStrings(u8("ß"), "ss", l,
+
-                                          wxCompare_CaseInsensitive) == 0 );
+#if defined(__WINDOWS__) || defined(__WXOSX__)
+        CHECK( l.CompareStrings(u8("ß"), "ss", wxCompare_CaseInsensitive) == 0 );
+#endif
     }
 
     SECTION("Swedish")
     {
-        const wxLocaleIdent l("sv");
+        const wxUILocale l("sv");
 
         // And this shows that sort order really depends on the language.
-        CHECK( wxUILocale::CompareStrings(u8("ä"), "ae", l) == 1 );
+        CHECK( l.CompareStrings(u8("ä"), "ae") == 1 );
-        CHECK( wxUILocale::CompareStrings(u8("ö"), "z" , l) == 1 );
+        CHECK( l.CompareStrings(u8("ö"), "z" ) == 1 );
     }
 }