Skip to content

Commit

Permalink
Do not use wxRegEx in wxCmpNaturalGeneric()
Browse files Browse the repository at this point in the history
Using wxRegEx in wxCmpNaturalGeneric() introduced a dependency of the
base library on the regex library.

Replace wxRegEx with character classification functions wxIsspace(),
wxIspunct(), and wxIsdigit() to remove this rather unnecessary
dependency.

Closes #2014
  • Loading branch information
PBfordev authored and vadz committed Aug 14, 2020
1 parent 8e2aad2 commit a464782
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 49 deletions.
3 changes: 0 additions & 3 deletions interface/wx/arrstr.h
Expand Up @@ -486,9 +486,6 @@ int wxCmpNatural(const wxString& s1, const wxString& s2);
/**
This is wxWidgets' own implementation of the natural sort comparison function.
Requires wxRegEx, if it is unavailable numbers within strings are not
recognised and only case-insensitive collation is performed.
@see wxCmpNatural()
@since 3.1.4
Expand Down
77 changes: 35 additions & 42 deletions src/common/arrstr.cpp
Expand Up @@ -20,7 +20,6 @@
#endif

#include "wx/arrstr.h"
#include "wx/regex.h"
#include "wx/scopedarray.h"
#include "wx/wxcrt.h"

Expand Down Expand Up @@ -729,8 +728,6 @@ wxArrayString wxSplit(const wxString& str, const wxChar sep, const wxChar escape
return ret;
}

#if wxUSE_REGEX

namespace // helpers needed by wxCmpNaturalGeneric()
{
// Used for comparison of string parts
Expand Down Expand Up @@ -759,47 +756,52 @@ struct wxStringFragment

wxStringFragment GetFragment(wxString& text)
{
static const wxRegEx reSpaceOrPunct(wxS("^([[:space:]]|[[:punct:]])+"));
// Limit the length to make sure the value will fit into a wxUint64
static const wxRegEx reDigit(wxS("^[[:digit:]]{1,19}"));
static const wxRegEx reLetterOrSymbol("^[^[:space:]|[:punct:]|[:digit:]]+");

if ( text.empty() )
return wxStringFragment();

wxStringFragment fragment;
size_t length = 0;
// the maximum length of a sequence of digits that
// can fit into wxUint64 when converted to a number
static const ptrdiff_t maxDigitSequenceLength = 19;

// In attempt to minimize the number of wxRegEx.Matches() calls,
// try to do them from the most expected to the least expected
// string fragment type.
if ( reLetterOrSymbol.Matches(text) )
{
if ( reLetterOrSymbol.GetMatch(NULL, &length) )
{
fragment.type = wxStringFragment::LetterOrSymbol;
fragment.text = text.Left(length);
}
}
else if ( reDigit.Matches(text) )
wxStringFragment fragment;
wxString::const_iterator it;

for ( it = text.cbegin(); it != text.cend(); ++it )
{
if ( reDigit.GetMatch(NULL, &length) )
const wxUniChar& ch = *it;
wxStringFragment::Type chType = wxStringFragment::Empty;

if ( wxIsspace(ch) || wxIspunct(ch) )
chType = wxStringFragment::SpaceOrPunct;
else if ( wxIsdigit(ch) )
chType = wxStringFragment::Digit;
else
chType = wxStringFragment::LetterOrSymbol;

// check if evaluating the first character
if ( fragment.type == wxStringFragment::Empty )
{
fragment.type = wxStringFragment::Digit;
fragment.text = text.Left(length);
fragment.text.ToULongLong(&fragment.value);
fragment.type = chType;
continue;
}
}
else if ( reSpaceOrPunct.Matches(text) )
{
if ( reSpaceOrPunct.GetMatch(NULL, &length) )

// stop processing when the current character has a different
// string fragment type than the previously processed characters had
// or a sequence of digits is too long
if ( fragment.type != chType
|| (fragment.type == wxStringFragment::Digit
&& it - text.cbegin() > maxDigitSequenceLength) )
{
fragment.type = wxStringFragment::SpaceOrPunct;
fragment.text = text.Left(length);
break;
}
}

text.erase(0, length);
fragment.text.assign(text.cbegin(), it);
if ( fragment.type == wxStringFragment::Digit )
fragment.text.ToULongLong(&fragment.value);

text.erase(0, it - text.cbegin());

return fragment;
}

Expand Down Expand Up @@ -893,15 +895,6 @@ int wxCMPFUNC_CONV wxCmpNaturalGeneric(const wxString& s1, const wxString& s2)
return comparison;
}

#else

int wxCMPFUNC_CONV wxCmpNaturalGeneric(const wxString& s1, const wxString& s2)
{
return wxStrcoll_String(s1.Lower(), s2.Lower());
}

#endif // #if wxUSE_REGEX

// ----------------------------------------------------------------------------
// Declaration of StrCmpLogicalW()
// ----------------------------------------------------------------------------
Expand Down
4 changes: 0 additions & 4 deletions tests/arrays/arrays.cpp
Expand Up @@ -784,9 +784,6 @@ void ArraysTestCase::IndexFromEnd()

TEST_CASE("wxNaturalStringComparisonGeneric()", "[wxString][compare]")
{
#if !wxUSE_REGEX
WARN("Skipping wxCmpNaturalGeneric() tests: wxRegEx not available");
#else
// simple string comparison
CHECK(wxCmpNaturalGeneric("a", "a") == 0);
CHECK(wxCmpNaturalGeneric("a", "z") < 0);
Expand Down Expand Up @@ -858,6 +855,5 @@ TEST_CASE("wxNaturalStringComparisonGeneric()", "[wxString][compare]")
CHECK(wxCmpNaturalGeneric("a5th5", "a10th10") < 0);
CHECK(wxCmpNaturalGeneric("a 10th 10", "a5th 5") < 0);
CHECK(wxCmpNaturalGeneric("a5th 5", "a 10th 10") > 0);
#endif // #if !wxUSE_REGEX
}

0 comments on commit a464782

Please sign in to comment.