diff --git a/common/displlst.cpp b/common/displlst.cpp index 5c99696c89..7b046cdd5f 100644 --- a/common/displlst.cpp +++ b/common/displlst.cpp @@ -250,7 +250,7 @@ static int wxCALLBACK myCompareFunction( wxIntPtr aItem1, wxIntPtr aItem2, wxString* component1Name = (wxString*) aItem1; wxString* component2Name = (wxString*) aItem2; - return StrNumCmp( *component1Name, *component2Name, INT_MAX, true ); + return StrNumCmp( *component1Name, *component2Name, true ); } diff --git a/common/lib_tree_model.cpp b/common/lib_tree_model.cpp index fcbb460085..71fe74c7dc 100644 --- a/common/lib_tree_model.cpp +++ b/common/lib_tree_model.cpp @@ -77,7 +77,7 @@ void LIB_TREE_NODE::AssignIntrinsicRanks( bool presorted ) std::sort( sort_buf.begin(), sort_buf.end(), []( LIB_TREE_NODE* a, LIB_TREE_NODE* b ) -> bool - { return StrNumCmp( a->Name, b->Name, INT_MAX, true ) > 0; } ); + { return StrNumCmp( a->Name, b->Name, true ) > 0; } ); for( int i = 0; i < (int) sort_buf.size(); ++i ) sort_buf[i]->IntrinsicRank = i; diff --git a/common/string.cpp b/common/string.cpp index d9154fa788..d95257996d 100644 --- a/common/string.cpp +++ b/common/string.cpp @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2004-2017 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2004-2019 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -39,13 +39,7 @@ */ static const char illegalFileNameChars[] = "\\/:\"<>|"; -/** - * These Escape/Unescape routines use HTML-entity-reference-style encoding to handle - * characters which are: - * (a) not legal in filenames - * (b) used as control characters in LIB_IDs - * (c) used to delineate hierarchical paths - */ + wxString EscapeString( const wxString& aSource ) { #if 1 @@ -316,62 +310,78 @@ wxString DateAndTime() } -int StrNumCmp( const wxString& aString1, const wxString& aString2, int aLength, bool aIgnoreCase ) +int StrNumCmp( const wxString& aString1, const wxString& aString2, bool aIgnoreCase ) { - int i; int nb1 = 0, nb2 = 0; wxString::const_iterator str1 = aString1.begin(), str2 = aString2.begin(); - for( i = 0; i < aLength; i++ ) + while( str1 != aString1.end() && str2 != aString2.end() ) { wxUniChar c1 = *str1; wxUniChar c2 = *str2; - if( isdigit( c1 ) && isdigit( c2 ) ) /* digit found */ + if( isdigit( c1 ) && isdigit( c2 ) ) // Both characters are digits, do numeric compare. { nb1 = 0; nb2 = 0; - while( isdigit( c1 ) ) + do { + c1 = *(str1); nb1 = nb1 * 10 + (int) c1 - '0'; - c1 = *(++str1); - } + ++str1; + } while( str1 != aString1.end() && isdigit( *(str1) ) ); - while( isdigit( c2 ) ) + do { + c2 = *(str2); nb2 = nb2 * 10 + (int) c2 - '0'; - c2 = *(++str2); - } + ++str2; + } while( str2 != aString2.end() && isdigit( *(str2) ) ); if( nb1 < nb2 ) return -1; - else if( nb1 > nb2 ) + + if( nb1 > nb2 ) return 1; + + c1 = ( str1 != aString1.end() ) ? *(str1) : (wxUniChar) '\0'; + c2 = ( str2 != aString2.end() ) ? *(str2) : (wxUniChar) '\0'; } + // Any numerical comparisons to here are identical. if( aIgnoreCase ) { - if( toupper( c1 ) < toupper(c2 ) ) + if( toupper( c1 ) < toupper( c2 ) ) return -1; - else if( toupper( c1 ) > toupper( c2 ) ) + + if( toupper( c1 ) > toupper( c2 ) ) return 1; - else if( ( c1 == 0 ) && ( c2 == 0 ) ) - return 0; } else { if( c1 < c2 ) return -1; - else if( c1 > c2 ) + + if( c1 > c2 ) return 1; - else if( ( c1 == 0 ) && ( c2 == 0 ) ) - return 0; } - ++str1; - ++str2; + if( str1 != aString1.end() ) + ++str1; + + if( str2 != aString2.end() ) + ++str2; + } + + if( str1 == aString1.end() && str2 != aString2.end() ) + { + return -1; // Identical to here but aString1 is longer. + } + else if( str1 != aString1.end() && str2 == aString2.end() ) + { + return 1; // Identical to here but aString2 is longer. } return 0; @@ -486,15 +496,6 @@ bool ApplyModifier( double& value, const wxString& aString ) } -// Should handle: -// a) Purely numerical e.g. '22' -// b) Numerical with included units e.g. '15uF' -// c) Numerical with included prefix but no units e.g. '20n' -// d) Numerical with prefix inside number e.g. '4K7' -// e) Other, e.g. 'MAX232' -// -// TODO: case (d) unimplemented !!! -// int ValueStringCompare( wxString strFWord, wxString strSWord ) { // Compare unescaped text @@ -611,6 +612,7 @@ int GetTrailingInt( const wxString& aStr ) // Trim and extract the trailing numeric part int index = aStr.Len() - 1; + while( index >= 0 ) { const char chr = aStr.GetChar( index ); diff --git a/include/footprint_info.h b/include/footprint_info.h index 9bf70f9168..0925dbd086 100644 --- a/include/footprint_info.h +++ b/include/footprint_info.h @@ -168,12 +168,12 @@ protected: /// FOOTPRINT object list sort function. inline bool operator<( const FOOTPRINT_INFO& item1, const FOOTPRINT_INFO& item2 ) { - int retv = StrNumCmp( item1.m_nickname, item2.m_nickname, INT_MAX, true ); + int retv = StrNumCmp( item1.m_nickname, item2.m_nickname, true ); if( retv != 0 ) return retv < 0; - return StrNumCmp( item1.m_fpname, item2.m_fpname, INT_MAX, true ) < 0; + return StrNumCmp( item1.m_fpname, item2.m_fpname, true ) < 0; } diff --git a/include/kicad_string.h b/include/kicad_string.h index 9044318d95..d0db7a13bb 100644 --- a/include/kicad_string.h +++ b/include/kicad_string.h @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2004 KiCad Developers, see change_log.txt for contributors. + * Copyright (C) 2004, 2019 KiCad Developers, see change_log.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -48,8 +48,8 @@ wxString EscapeString( const wxString& aSource ); wxString UnescapeString( const wxString& aSource ); /** - * Function ReadDelimitedText - * copies bytes from @a aSource delimited string segment to @a aDest buffer. + * Copy bytes from @a aSource delimited string segment to @a aDest buffer. + * * The extracted string will be null terminated even if truncation is necessary * because aDestSize was not large enough. * @@ -63,8 +63,7 @@ wxString UnescapeString( const wxString& aSource ); int ReadDelimitedText( char* aDest, const char* aSource, int aDestSize ); /** - * Function ReadDelimitedText - * copies bytes from @a aSource delimited string segment to @a aDest wxString. + * Copy bytes from @a aSource delimited string segment to @a aDest wxString. * * @param aDest is the destination wxString * @param aSource is the source C string holding utf8 encoded bytes. @@ -90,48 +89,43 @@ std::string EscapedUTF8( const wxString& aString ); wxString EscapedHTML( const wxString& aString ); /** - * Function GetLine - * reads one line line from \a aFile. - * @return A pointer the first useful line read by eliminating blank lines and comments. + * Read one line line from \a aFile. + * + * @return a pointer the first useful line read by eliminating blank lines and comments. */ char* GetLine( FILE* aFile, char* Line, int* LineNum = NULL, int SizeLine = 255 ); /** - * Function StrPurge - * removes leading and training spaces, tabs and end of line chars in \a text - * return a pointer on the first n char in text + * Remove leading and training spaces, tabs and end of line chars in \a text + * + * @return a pointer on the first n char in text */ char* StrPurge( char* text ); /** - * Function DateAndTime * @return a string giving the current date and time. */ wxString DateAndTime(); /** - * Function StrNumCmp - * is a routine compatible with qsort() to sort by alphabetical order. + * Compare two strings with alphanumerical content. * * This function is equivalent to strncmp() or strncasecmp() if \a aIgnoreCase is true * except that strings containing numbers are compared by their integer value not - * by their ASCII code. + * by their ASCII code. In other words U10 would be greater than U2. * * @param aString1 A wxString reference to the reference string. * @param aString2 A wxString reference to the comparison string. - * @param aLength The number of characters to compare. Set to -1 to compare - * the entire string. * @param aIgnoreCase Use true to make the comparison case insensitive. * @return An integer value of -1 if \a aString1 is less than \a aString2, 0 if * \a aString1 is equal to \a aString2, or 1 if \a aString1 is greater * than \a aString2. */ -int StrNumCmp( const wxString& aString1, const wxString& aString2, int aLength = INT_MAX, - bool aIgnoreCase = false ); +int StrNumCmp( const wxString& aString1, const wxString& aString2, bool aIgnoreCase = false ); /** - * Function WildCompareString - * compares a string against wild card (* and ?) pattern using the usual rules. + * Compare a string against wild card (* and ?) pattern using the usual rules. + * * @return true if pattern matched otherwise false. */ bool WildCompareString( const wxString& pattern, @@ -139,21 +133,18 @@ bool WildCompareString( const wxString& pattern, bool case_sensitive = true ); /** - * Function ValueStringCompare - * acts just like the strcmp function but handles numbers and modifiers within the + * Compare strings like the strcmp function but handle numbers and modifiers within the * string text correctly for sorting. eg. 1mF > 55uF - * return -1 if first string is less than the second - * return 0 if the strings are equal - * return 1 if the first string is greater than the second + * + * @return -1 if first string is less than the second, 0 if the strings are equal, or + * 1 if the first string is greater than the second. */ int ValueStringCompare( wxString strFWord, wxString strSWord ); /** - * Function SplitString - * breaks a string into three parts. - * The alphabetic preamble - * The numeric part - * Any alphabetic ending + * Breaks a string into three parts: he alphabetic preamble, the numeric part, and any + * alphabetic ending. + * * For example C10A is split to C 10 A */ int SplitString( wxString strToSplit, @@ -163,26 +154,25 @@ int SplitString( wxString strToSplit, /** * Gets the trailing int, if any, from a string. + * * @param aStr the string to check * @return the trailing int or 0 if none found */ int GetTrailingInt( const wxString& aStr ); /** - * Function GetIllegalFileNameWxChars - * @return a wString object containing the illegal file name characters for all platforms. + * @return a wxString object containing the illegal file name characters for all platforms. */ wxString GetIllegalFileNameWxChars(); /** - * Function ReplaceIllegalFileNameChars - * checks \a aName for illegal file name characters. + * Checks \a aName for illegal file name characters. * * The Windows (DOS) file system forbidden characters already include the forbidden file * name characters for both Posix and OSX systems. The characters \/?*|"\<\> are illegal * and are replaced with %xx where xx the hexadecimal equivalent of the replaced character. * This replacement may not be as elegant as using an underscore ('_') or hyphen ('-') but - * it guarentees that there will be no naming conflicts when fixing footprint library names. + * it guarantees that there will be no naming conflicts when fixing footprint library names. * however, if aReplaceChar is given, it will replace the illegal chars * * @param aName is a point to a std::string object containing the footprint name to verify. @@ -199,7 +189,7 @@ extern "C" char* strtok_r( char* str, const char* delim, char** nextp ); /** * A helper for sorting strings from the rear. Useful for things like 3d model names - * where they tend to be largely repititous at the front. + * where they tend to be largely repetitious at the front. */ struct rsort_wxString { diff --git a/pcbnew/exporters/gen_footprints_placefile.cpp b/pcbnew/exporters/gen_footprints_placefile.cpp index f88108dcd9..4f6e495036 100644 --- a/pcbnew/exporters/gen_footprints_placefile.cpp +++ b/pcbnew/exporters/gen_footprints_placefile.cpp @@ -388,7 +388,7 @@ static wxPoint File_Place_Offset; // Offset coordinates for generated file. static bool sortFPlist( const LIST_MOD& ref, const LIST_MOD& tst ) { if( ref.m_Layer == tst.m_Layer ) - return StrNumCmp( ref.m_Reference, tst.m_Reference, 16 ) < 0; + return StrNumCmp( ref.m_Reference, tst.m_Reference ) < 0; return ref.m_Layer > tst.m_Layer; } diff --git a/pcbnew/fp_tree_model_adapter.cpp b/pcbnew/fp_tree_model_adapter.cpp index e0291c62d2..998de146b3 100644 --- a/pcbnew/fp_tree_model_adapter.cpp +++ b/pcbnew/fp_tree_model_adapter.cpp @@ -68,7 +68,7 @@ std::vector FP_TREE_MODEL_ADAPTER::getFootprints( const wxString auto libBounds = std::equal_range( fullListStart, fullListEnd, dummy, []( const std::unique_ptr& a, const std::unique_ptr& b ) { - return StrNumCmp( a->GetLibNickname(), b->GetLibNickname(), INT_MAX, true ) < 0; + return StrNumCmp( a->GetLibNickname(), b->GetLibNickname(), true ) < 0; } ); for( auto i = libBounds.first; i != libBounds.second; ++i ) diff --git a/pcbnew/fp_tree_synchronizing_adapter.cpp b/pcbnew/fp_tree_synchronizing_adapter.cpp index 67e6e3c655..55258e0502 100644 --- a/pcbnew/fp_tree_synchronizing_adapter.cpp +++ b/pcbnew/fp_tree_synchronizing_adapter.cpp @@ -109,7 +109,7 @@ void FP_TREE_SYNCHRONIZING_ADAPTER::updateLibrary( LIB_TREE_NODE_LIB& aLibNode ) auto footprintIt = std::lower_bound( footprints.begin(), footprints.end(), &dummy, []( LIB_TREE_ITEM* a, LIB_TREE_ITEM* b ) { - return StrNumCmp( a->GetName(), b->GetName(), INT_MAX, true ) < 0; + return StrNumCmp( a->GetName(), b->GetName(), true ) < 0; } ); if( footprintIt != footprints.end() && dummy.GetName() == (*footprintIt)->GetName() ) diff --git a/qa/common/test_kicad_string.cpp b/qa/common/test_kicad_string.cpp index 7571510c9e..1054da11db 100644 --- a/qa/common/test_kicad_string.cpp +++ b/qa/common/test_kicad_string.cpp @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE( NaturalNumberCompare ) c.first.first + " AND " + c.first.second + " failed for case sensitive" ); BOOST_CHECK_MESSAGE( - StrNumCmp( c.first.first, c.first.second, INT_MAX, true ) == c.second.second, + StrNumCmp( c.first.first, c.first.second, true ) == c.second.second, c.first.first + " AND " + c.first.second + " failed for case insensitive" ); } }