haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove broken RefDesStringCompare 

This function attempted a poor-man's natural compare but it assumed
specific structure of the string.  This broke for strings with
numberings that looked like decimals.

Instead, we use our natural string comparison function and remove the
references to this older function and its similar elements.

Fixes https://gitlab.com/kicad/code/kicad/issues/9067
This commit is contained in:
Seth Hillbrand 2021-09-01 11:45:10 -07:00
parent bc0302fafe
commit 8a305eec32
10 changed files with 26 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
common/refdes_utils.cpp
View File

@ -70,46 +70,4 @@ int GetRefDesNumber( const wxString& aRefDes )
return retval;
}
int RefDesStringCompare( const wxString& aFirst, const wxString& aSecond )
{
// Compare unescaped text
wxString strFWord = UnescapeString( aFirst );
wxString strSWord = UnescapeString( aSecond );
// The different sections of the two strings
wxString strFWordBeg, strFWordMid, strFWordEnd;
wxString strSWordBeg, strSWordMid, strSWordEnd;
// Split the two strings into separate parts
SplitString( strFWord, &strFWordBeg, &strFWordMid, &strFWordEnd );
SplitString( strSWord, &strSWordBeg, &strSWordMid, &strSWordEnd );
// Compare the Beginning section of the strings
int isEqual = strFWordBeg.Cmp( strSWordBeg );
if( isEqual > 0 )
return 1;
else if( isEqual < 0 )
return -1;
else
{
// If the first sections are equal compare their digits
long lFirstDigit = 0;
long lSecondDigit = 0;
strFWordMid.ToLong( &lFirstDigit );
strSWordMid.ToLong( &lSecondDigit );
if( lFirstDigit > lSecondDigit )
return 1;
else if( lFirstDigit < lSecondDigit )
return -1;
// If the first two sections are equal compare the endings
else
return strFWordEnd.Cmp( strSWordEnd );
}
}
} // namespace UTIL

13
eeschema/dialogs/dialog_symbol_fields_table.cpp
View File

@ -33,7 +33,6 @@
#include <grid_tricks.h>
#include <string_utils.h>
#include <kiface_i.h>
#include <refdes_utils.h>
#include <sch_edit_frame.h>
#include <sch_reference_list.h>
#include <schematic.h>
@ -302,9 +301,9 @@ public:
std::sort( references.begin(), references.end(),
[]( const SCH_REFERENCE& l, const SCH_REFERENCE& r ) -> bool
{
wxString l_ref( l.GetRef() << l.GetRefNumber() );
wxString r_ref( r.GetRef() << r.GetRefNumber() );
return UTIL::RefDesStringCompare( l_ref, r_ref ) < 0;
wxString l_ref( l.GetFullRef() );
wxString r_ref( r.GetFullRef() );
return StrNumCmp( l_ref, r_ref, true ) < 0;
} );
auto logicalEnd = std::unique( references.begin(), references.end(),
@ -372,9 +371,9 @@ public:
if( lhs == rhs || sortCol == REFERENCE_FIELD )
{
wxString lhRef = lhGroup.m_Refs[ 0 ].GetRef() + lhGroup.m_Refs[ 0 ].GetRefNumber();
wxString rhRef = rhGroup.m_Refs[ 0 ].GetRef() + rhGroup.m_Refs[ 0 ].GetRefNumber();
return local_cmp( UTIL::RefDesStringCompare( lhRef, rhRef ), 0 );
wxString lhRef = lhGroup.m_Refs[ 0 ].GetFullRef();
wxString rhRef = rhGroup.m_Refs[ 0 ].GetFullRef();
return local_cmp( StrNumCmp( lhRef, rhRef, true ), 0 );
}
else
{

4
eeschema/netlist_exporters/netlist_exporter_base.cpp
View File

@ -25,7 +25,7 @@
#include <netlist_exporter_base.h>
#include <refdes_utils.h>
#include <string_utils.h>
#include <symbol_library.h>
#include <connection_graph.h>
@ -117,7 +117,7 @@ SCH_SYMBOL* NETLIST_EXPORTER_BASE::findNextSymbol( EDA_ITEM* aItem, SCH_SHEET_PA
static bool sortPinsByNum( PIN_INFO& aPin1, PIN_INFO& aPin2 )
{
// return "lhs < rhs"
return UTIL::RefDesStringCompare( aPin1.num, aPin2.num ) < 0;
return StrNumCmp( aPin1.num, aPin2.num, true ) < 0;
}

8
eeschema/netlist_exporters/netlist_exporter_xml.cpp
View File

@ -31,7 +31,7 @@
#include <symbol_library.h>
#include <string_utils.h>
#include <connection_graph.h>
#include <refdes_utils.h>
#include <string_utils.h>
#include <wx/wfstream.h>
#include <xnode.h> // also nests: <wx/xml/xml.h>
@ -237,8 +237,8 @@ XNODE* NETLIST_EXPORTER_XML::makeSymbols( unsigned aCtl )
auto cmp = [sheet]( SCH_SYMBOL* a, SCH_SYMBOL* b )
{
return ( UTIL::RefDesStringCompare( a->GetRef( &sheet ),
b->GetRef( &sheet ) ) < 0 );
return ( StrNumCmp( a->GetRef( &sheet, false ),
b->GetRef( &sheet, false ), true ) < 0 );
};
std::set<SCH_SYMBOL*, decltype( cmp )> ordered_symbols( cmp );
@ -779,5 +779,5 @@ XNODE* NETLIST_EXPORTER_XML::node( const wxString& aName,
static bool sortPinsByNumber( LIB_PIN* aPin1, LIB_PIN* aPin2 )
{
// return "lhs < rhs"
return UTIL::RefDesStringCompare( aPin1->GetShownNumber(), aPin2->GetShownNumber() ) < 0;
return StrNumCmp( aPin1->GetShownNumber(), aPin2->GetShownNumber(), true ) < 0;
}

2
eeschema/sch_reference_list.h
View File

@ -104,7 +104,7 @@ public:
const char* GetRefStr() const { return m_ref.c_str(); }
///< Return reference name with unit altogether
wxString GetFullRef()
wxString GetFullRef() const
{
if( GetSymbol()->GetUnitCount() > 1 )
return GetRef() + LIB_SYMBOL::SubReference( GetUnit() );

9
include/refdes_utils.h
View File

@ -64,15 +64,6 @@ wxString GetRefDesUnannotated( const wxString& aRefDes );
*/
int GetRefDesNumber( const wxString& aRefDes );
/**
* Acts just like the strcmp function but treats numbers within the string text
* correctly for sorting. eg. A10 > A2
* 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
*/
int RefDesStringCompare( const wxString& lhs, const wxString& rhs );
} // namespace UTIL
#endif // REFDES_UTILS__H

4
pcbnew/netlist_reader/pcb_netlist.cpp
View File

@ -24,7 +24,7 @@
*/
#include <refdes_utils.h>
#include <string_utils.h>
#include "pcb_netlist.h"
#include <footprint.h>
@ -211,7 +211,7 @@ void NETLIST::SortByFPID()
*/
bool operator < ( const COMPONENT& item1, const COMPONENT& item2 )
{
return UTIL::RefDesStringCompare( item1.GetReference(), item2.GetReference() ) < 0;
return StrNumCmp( item1.GetReference(), item2.GetReference(), true ) < 0;
}

3
pcbnew/tools/board_reannotate_tool.cpp
View File

@ -24,6 +24,7 @@
*/
#include <refdes_utils.h>
#include <string_utils.h>
#include <tool/tool_manager.h>
#include <wx/filedlg.h>
#include <tools/board_reannotate_tool.h>
@ -86,7 +87,7 @@ int BOARD_REANNOTATE_TOOL::ReannotateDuplicatesInSelection()
std::sort( fpInSelection.begin(), fpInSelection.end(),
[]( const FOOTPRINT* aA, const FOOTPRINT* aB ) -> bool
{
int ii = UTIL::RefDesStringCompare( aA->GetReference(), aB->GetReference() );
int ii = StrNumCmp( aA->GetReference(), aB->GetReference(), true );
if( ii == 0 )
ii = aA->m_Uuid < aB->m_Uuid; // ensure a deterministic sort

8
qa/common/test_kicad_string.cpp
View File

@ -93,7 +93,13 @@ BOOST_AUTO_TEST_CASE( NaturalNumberCompare )
{ { "u10", "u10" }, { 0, 0 } },
{ { "U10", "U10" }, { 0, 0 } },
{ { "u10", "U10" }, { 1, 0 } },
{ { "U10", "u10" }, { -1, 0 } }
{ { "U10", "u10" }, { -1, 0 } },
{ { "U10.1", "U10.10" }, { -1, -1 } },
{ { "U10-1", "U10-10" }, { -1, -1 } },
{ { "U10,1", "U10,10" }, { -1, -1 } },
{ { "U10.A", "U10.a" }, { -1, 0 } },
{ { "U10-A", "U10-a" }, { -1, 0 } },
{ { "U10,A", "U10,a" }, { -1, 0 } },
};
for( const auto& c : cases )

32
qa/common/test_refdes_utils.cpp
View File

@ -66,36 +66,4 @@ BOOST_AUTO_TEST_CASE( GetPrefix )
}
struct REF_DES_COMP_CASE
{
std::string m_refdes_a;
std::string m_refdes_b;
int m_exp_res;
};
/**
* Test the #UTIL::RefDesStringCompare function
*/
BOOST_AUTO_TEST_CASE( RefDesComp )
{
const int SAME = 0;
const int LESS = -1;
const int MORE = 1;
const std::vector<REF_DES_COMP_CASE> cases = {
{ "", "", SAME },
{ "U", "U", SAME },
{ "U1", "U1", SAME },
{ "U1", "U2", LESS },
{ "U2", "U1", MORE },
{ "U1000", "U2000", LESS },
};
for( const auto& c : cases )
{
BOOST_CHECK_EQUAL( UTIL::RefDesStringCompare( c.m_refdes_a, c.m_refdes_b ), c.m_exp_res );
}
}
BOOST_AUTO_TEST_SUITE_END()