From ae81d9d2078bce7e75576d4908d167c034803db1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Aug 2021 18:57:30 +0200 Subject: [PATCH] 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 | 2 + include/wx/uilocale.h | 7 +- interface/wx/uilocale.h | 26 ++++--- src/common/uilocale.cpp | 8 ++ src/msw/uilocale.cpp | 133 ++++++++++++++++------------------ src/osx/core/uilocale.mm | 15 ++-- src/unix/uilocale.cpp | 24 ++++++ tests/intl/intltest.cpp | 43 ++++++----- 8 files changed, 141 insertions(+), 117 deletions(-) diff --git a/include/wx/private/uilocale.h b/include/wx/private/uilocale.h index 6b0e7fbe48..c5443e6b4b 100644 --- a/include/wx/private/uilocale.h +++ b/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() { } }; diff --git a/include/wx/uilocale.h b/include/wx/uilocale.h index cb27bb0a70..238075805c 100644 --- a/include/wx/uilocale.h +++ b/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. - static int CompareStrings(const wxString& lhs, const wxString& rhs, - const wxLocaleIdent& localeId = wxLocaleIdent(), - int flags = wxCompare_CaseSensitive); + // Compares two strings in the order defined by this locale. + int CompareStrings(const wxString& lhs, const wxString& rhs, + int flags = wxCompare_CaseSensitive) const; // Note that this class is not supposed to be used polymorphically, hence // its dtor is not virtual. diff --git a/interface/wx/uilocale.h b/interface/wx/uilocale.h index a3ee7ec65e..3f722722d9 100644 --- a/interface/wx/uilocale.h +++ b/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 - description for more details. + see wxLocaleIdent 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, - const wxLocaleIdent& localeId = wxLocaleIdent(), - int flags = wxCompare_CaseSensitive); + int CompareStrings(const wxString& lhs, const wxString& rhs, + int flags = wxCompare_CaseSensitive) const; /** Get the platform-dependent name of the current locale. diff --git a/src/common/uilocale.cpp b/src/common/uilocale.cpp index 7dbdc63be5..0084d7058b 100644 --- a/src/common/uilocale.cpp +++ b/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; diff --git a/src/msw/uilocale.cpp b/src/msw/uilocale.cpp index d3c7079ff7..a023c01bab 100644 --- a/src/msw/uilocale.cpp +++ b/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(localeId.GetName().wc_str()), - dwFlags, - static_cast(lhs.wc_str()), -1, - static_cast(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 diff --git a/src/osx/core/uilocale.mm b/src/osx/core/uilocale.mm index 6fb591dde1..71dfab41af 100644 --- a/src/osx/core/uilocale.mm +++ b/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 m_cfloc; @@ -141,29 +143,22 @@ wxUILocaleImpl* wxUILocaleImpl::CreateForLocale(const wxLocaleIdent& locId) return wxUILocaleImplCF::Create(locId); } -/* static */ int -wxUILocale::CompareStrings(const wxString& lhs, - const wxString& rhs, - const wxLocaleIdent& localeId, - int flags) +wxUILocaleImplCF::CompareStrings(const wxString& lhs, const wxString& rhs, + int flags) const { 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) { diff --git a/src/unix/uilocale.cpp b/src/unix/uilocale.cpp index 3889e6271a..e7403d6173 100644 --- a/src/unix/uilocale.cpp +++ b/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() { diff --git a/tests/intl/intltest.cpp b/tests/intl/intltest.cpp index 7fa9e1bb30..50f3ae8eb7 100644 --- a/tests/intl/intltest.cpp +++ b/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( wxUILocale::CompareStrings("x", "y", l) == -1 ); - CHECK( wxUILocale::CompareStrings("z", "y", l) == +1 ); + CHECK( l.CompareStrings("x", "x") == 0 ); + CHECK( l.CompareStrings("x", "y") == -1 ); + CHECK( l.CompareStrings("z", "y") == +1 ); // Also check for some degenerate cases. - CHECK( wxUILocale::CompareStrings("", "", l) == 0 ); - CHECK( wxUILocale::CompareStrings("", "a", l) == -1 ); + CHECK( l.CompareStrings("", "") == 0 ); + CHECK( l.CompareStrings("", "a") == -1 ); // And for case handling. - CHECK( wxUILocale::CompareStrings("a", "A", l) == -1 ); - CHECK( wxUILocale::CompareStrings("a", "A", l, - wxCompare_CaseInsensitive) == 0 ); - CHECK( wxUILocale::CompareStrings("b", "A", l) == 1 ); - CHECK( wxUILocale::CompareStrings("B", "a", l) == 1 ); + CHECK( l.CompareStrings("a", "A") == -1 ); + CHECK( l.CompareStrings("b", "A") == 1 ); + CHECK( l.CompareStrings("B", "a") == 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( wxUILocale::CompareStrings(u8("ä"), "ae", l) == -1 ); - CHECK( wxUILocale::CompareStrings(u8("ß"), "ss", l, - wxCompare_CaseInsensitive) == 0 ); + CHECK( l.CompareStrings("a", u8("ä")) == -1 ); + CHECK( l.CompareStrings(u8("ä"), "ae") == -1 ); + +#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( wxUILocale::CompareStrings(u8("ö"), "z" , l) == 1 ); + CHECK( l.CompareStrings(u8("ä"), "ae") == 1 ); + CHECK( l.CompareStrings(u8("ö"), "z" ) == 1 ); } }