From f0b2565f12240d74813c6581fcadc155a71fb100 Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Sun, 27 Oct 2013 14:21:53 -0400 Subject: [PATCH] Eeschema find/replace bug fixes and improvements (fixes 1208616). * Fix replace bug to handle case sensitivity properly. * Fix replace bug where the item index was getting updated incorrectly. * Fix replace infinite loop bug on replace all. * Make find/replace view update code a separate function. * Rearrange find/replace trace string to add tracing to EDA_ITEM::Replace(). * Add IsComplexHierarchy method to SCH_SHEET_LIST for future find/replace improvements. --- common/base_struct.cpp | 28 +++- common/sch_item_struct.cpp | 2 - eeschema/dialogs/dialog_schematic_find.h | 2 +- eeschema/find.cpp | 157 +++++++++++++---------- eeschema/sch_collectors.cpp | 13 +- eeschema/sch_collectors.h | 29 +++-- eeschema/sch_field.cpp | 9 +- eeschema/sch_sheet_path.cpp | 26 ++++ eeschema/sch_sheet_path.h | 10 ++ include/base_struct.h | 4 + include/sch_item_struct.h | 3 - include/wxEeschemaStruct.h | 2 + 12 files changed, 189 insertions(+), 96 deletions(-) diff --git a/common/base_struct.cpp b/common/base_struct.cpp index 4af6994e9b..06ce8c03c6 100644 --- a/common/base_struct.cpp +++ b/common/base_struct.cpp @@ -38,6 +38,10 @@ #include "../eeschema/dialogs/dialog_schematic_find.h" + +const wxString traceFindReplace( wxT( "KicadFindReplace" ) ); + + enum textbox { ID_TEXTBOX_LIST = 8010 }; @@ -190,8 +194,28 @@ bool EDA_ITEM::Replace( wxFindReplaceData& aSearchData, wxString& aText ) wxCHECK_MSG( IsReplaceable(), false, wxT( "Attempt to replace text in <" ) + GetClass() + wxT( "> item." ) ); - return aText.Replace( aSearchData.GetFindString(), - aSearchData.GetReplaceString(), false ) != 0; + wxString searchString = (aSearchData.GetFlags() & wxFR_MATCHCASE) ? aText.Upper() : aText; + + int result = searchString.Find( (aSearchData.GetFlags() & wxFR_MATCHCASE) ? + aSearchData.GetFindString() : + aSearchData.GetFindString().Upper() ); + + if( result == wxNOT_FOUND ) + return false; + + wxString prefix = aText.Left( result ); + wxString suffix; + + if( aSearchData.GetFindString().length() + result < aText.length() ) + suffix = aText.Right( aText.length() - ( aSearchData.GetFindString().length() + result ) ); + + wxLogTrace( traceFindReplace, wxT( "Replacing '%s', prefix '%s', replace '%s', suffix '%s'." ), + GetChars( aText ), GetChars( prefix ), GetChars( aSearchData.GetReplaceString() ), + GetChars( suffix ) ); + + aText = prefix + aSearchData.GetReplaceString() + suffix; + + return true; } diff --git a/common/sch_item_struct.cpp b/common/sch_item_struct.cpp index a0be20a0db..11f9058131 100644 --- a/common/sch_item_struct.cpp +++ b/common/sch_item_struct.cpp @@ -39,8 +39,6 @@ #include -const wxString traceFindReplace( wxT( "KicadFindReplace" ) ); - const wxString traceFindItem( wxT( "KicadFindItem" ) ); diff --git a/eeschema/dialogs/dialog_schematic_find.h b/eeschema/dialogs/dialog_schematic_find.h index 06b264d9a0..287d347cdc 100644 --- a/eeschema/dialogs/dialog_schematic_find.h +++ b/eeschema/dialogs/dialog_schematic_find.h @@ -48,7 +48,7 @@ */ enum SchematicFindReplaceFlags { - // The last wxFindReplaceFlag enum is wxFR_MATCHCASE. + // The last wxFindReplaceFlag enum is wxFR_MATCHCASE = 0x4. /// Search the current sheet only. FR_CURRENT_SHEET_ONLY = wxFR_MATCHCASE << 1, diff --git a/eeschema/find.cpp b/eeschema/find.cpp index e81ba6c3ff..ed52efe966 100644 --- a/eeschema/find.cpp +++ b/eeschema/find.cpp @@ -288,12 +288,11 @@ SCH_ITEM* SCH_EDIT_FRAME::FindComponentAndItem( const wxString& aReference, void SCH_EDIT_FRAME::OnFindSchematicItem( wxFindDialogEvent& aEvent ) { - static wxPoint itemPosition; // the actual position of the matched item. + static wxPoint itemPosition; // the actual position of the matched item. - SCH_SHEET_LIST schematic; - wxString msg; - SCH_FIND_REPLACE_DATA searchCriteria; - bool warpCursor = !( aEvent.GetFlags() & FR_NO_WARP_CURSOR ); + SCH_SHEET_LIST schematic; + wxString msg; + SCH_FIND_REPLACE_DATA searchCriteria; SCH_FIND_COLLECTOR_DATA data; searchCriteria.SetFlags( aEvent.GetFlags() ); @@ -326,6 +325,88 @@ void SCH_EDIT_FRAME::OnFindSchematicItem( wxFindDialogEvent& aEvent ) m_foundItems.UpdateIndex(); } + updateFindReplaceView( aEvent ); +} + + +void SCH_EDIT_FRAME::OnFindReplace( wxFindDialogEvent& aEvent ) +{ + SCH_ITEM* item; + SCH_SHEET_PATH* sheet; + SCH_SHEET_LIST schematic; + SCH_FIND_COLLECTOR_DATA data; + + if( aEvent.GetEventType() == wxEVT_COMMAND_FIND_REPLACE_ALL ) + { + while( ( item = (SCH_ITEM*) m_foundItems.GetItem( data ) ) != NULL ) + { + SCH_ITEM* undoItem = data.GetParent(); + + // Don't save child items in undo list. + if( undoItem == NULL ) + undoItem = item; + + SetUndoItem( undoItem ); + + sheet = schematic.GetSheet( data.GetSheetPath() ); + + wxCHECK_RET( sheet != NULL, wxT( "Could not find sheet path " ) + data.GetSheetPath() ); + + if( m_foundItems.ReplaceItem( sheet ) ) + { + OnModify(); + SaveUndoItemInUndoList( undoItem ); + updateFindReplaceView( aEvent ); + } + + m_foundItems.IncrementIndex(); + + if( m_foundItems.PassedEnd() ) + break; + } + } + else + { + SCH_ITEM* item = (SCH_ITEM*) m_foundItems.GetItem( data ); + + wxCHECK_RET( item != NULL, wxT( "Invalid replace item in find collector list." ) ); + + SCH_ITEM* undoItem = data.GetParent(); + + if( undoItem == NULL ) + undoItem = item; + + SetUndoItem( undoItem ); + + sheet = schematic.GetSheet( data.GetSheetPath() ); + + wxCHECK_RET( sheet != NULL, wxT( "Could not find sheet path " ) + data.GetSheetPath() ); + + if( m_foundItems.ReplaceItem( sheet ) ) + { + OnModify(); + SaveUndoItemInUndoList( undoItem ); + updateFindReplaceView( aEvent ); + } + + m_foundItems.IncrementIndex(); + } + + // End the replace if we are at the end if the list. This prevents an infinite loop if + // wrap search is selected and all of the items have been replaced with a value that + // still satisfies the search criteria. + if( m_foundItems.PassedEnd() ) + aEvent.SetFlags( aEvent.GetFlags() & ~FR_REPLACE_ITEM_FOUND ); +} + + +void SCH_EDIT_FRAME::updateFindReplaceView( wxFindDialogEvent& aEvent ) +{ + wxString msg; + SCH_SHEET_LIST schematic; + SCH_FIND_COLLECTOR_DATA data; + bool warpCursor = !( aEvent.GetFlags() & FR_NO_WARP_CURSOR ); + if( m_foundItems.GetItem( data ) != NULL ) { wxLogTrace( traceFindReplace, wxT( "Found " ) + m_foundItems.GetText() ); @@ -335,13 +416,15 @@ void SCH_EDIT_FRAME::OnFindSchematicItem( wxFindDialogEvent& aEvent ) wxCHECK_RET( sheet != NULL, wxT( "Could not find sheet path " ) + data.GetSheetPath() ); + SCH_ITEM* item = (SCH_ITEM*)m_foundItems.GetItem( data ); + // Make the item temporarily visible just in case it's hide flag is set. This // has no effect on objects that don't support hiding. If this is a close find // dialog event, clear the temporary visibility flag. if( aEvent.GetEventType() == wxEVT_COMMAND_FIND_CLOSE ) - m_foundItems.GetItem( data )->SetForceVisible( false ); - else - m_foundItems.GetItem( data )->SetForceVisible( true ); + item->SetForceVisible( false ); + else if( item->Type() == SCH_FIELD_T && !( (SCH_FIELD*) item )->IsVisible() ) + item->SetForceVisible( true ); if( sheet->PathHumanReadable() != m_CurrentSheet->PathHumanReadable() ) { @@ -371,61 +454,3 @@ void SCH_EDIT_FRAME::OnFindSchematicItem( wxFindDialogEvent& aEvent ) SetStatusText( msg ); } - - -void SCH_EDIT_FRAME::OnFindReplace( wxFindDialogEvent& aEvent ) -{ - SCH_FIND_COLLECTOR_DATA data; - - bool warpCursor = !( aEvent.GetFlags() & FR_NO_WARP_CURSOR ); - SCH_ITEM* item = (SCH_ITEM*) m_foundItems.GetItem( data ); - - wxCHECK_RET( item != NULL, wxT( "Invalid replace item in find collector list." ) ); - - wxLogTrace( traceFindReplace, wxT( "Replacing %s with %s in item %s" ), - GetChars( aEvent.GetFindString() ), GetChars( aEvent.GetReplaceString() ), - GetChars( m_foundItems.GetText() ) ); - - SCH_ITEM* undoItem = data.GetParent(); - - if( undoItem == NULL ) - undoItem = item; - - SetUndoItem( undoItem ); - - if( m_foundItems.ReplaceItem() ) - { - OnModify(); - SaveUndoItemInUndoList( undoItem ); - RedrawScreen( data.GetPosition(), warpCursor ); - } - - OnFindSchematicItem( aEvent ); - - if( aEvent.GetEventType() == wxEVT_COMMAND_FIND_REPLACE_ALL ) - { - while( ( item = (SCH_ITEM*) m_foundItems.GetItem( data ) ) != NULL ) - { - wxLogTrace( traceFindReplace, wxT( "Replacing %s with %s in item %s" ), - GetChars( aEvent.GetFindString() ), GetChars( aEvent.GetReplaceString() ), - GetChars( m_foundItems.GetText() ) ); - - SCH_ITEM* undoItem = data.GetParent(); - - // Don't save child items in undo list. - if( undoItem == NULL ) - undoItem = item; - - SetUndoItem( undoItem ); - - if( m_foundItems.ReplaceItem() ) - { - OnModify(); - SaveUndoItemInUndoList( undoItem ); - RedrawScreen( data.GetPosition(), warpCursor ); - } - - OnFindSchematicItem( aEvent ); - } - } -} diff --git a/eeschema/sch_collectors.cpp b/eeschema/sch_collectors.cpp index 6313b1dd52..6f8562091e 100644 --- a/eeschema/sch_collectors.cpp +++ b/eeschema/sch_collectors.cpp @@ -347,7 +347,7 @@ bool SCH_FIND_COLLECTOR::PassedEnd() const if( GetCount() == 0 ) return true; - if( !(flags & FR_SEARCH_WRAP) ) + if( !(flags & FR_SEARCH_WRAP) || (flags & FR_SEARCH_REPLACE) ) { if( flags & wxFR_DOWN ) { @@ -454,7 +454,7 @@ EDA_ITEM* SCH_FIND_COLLECTOR::GetItem( SCH_FIND_COLLECTOR_DATA& aData ) } -bool SCH_FIND_COLLECTOR::ReplaceItem() +bool SCH_FIND_COLLECTOR::ReplaceItem( SCH_SHEET_PATH* aSheetPath ) { if( PassedEnd() ) return false; @@ -464,15 +464,10 @@ bool SCH_FIND_COLLECTOR::ReplaceItem() EDA_ITEM* item = m_List[ m_foundIndex ]; - bool replaced = item->Replace( m_findReplaceData ); + bool replaced = item->Replace( m_findReplaceData, aSheetPath ); - // If the replace was successful, remove the item from the find list to prevent - // iterating back over it again. if( replaced ) - { - Remove( m_foundIndex ); - m_data.erase( m_data.begin() + m_foundIndex ); - } + m_forceSearch = true; return replaced; } diff --git a/eeschema/sch_collectors.h b/eeschema/sch_collectors.h index d8851e0d89..299ea9ec62 100644 --- a/eeschema/sch_collectors.h +++ b/eeschema/sch_collectors.h @@ -237,15 +237,6 @@ class SCH_FIND_COLLECTOR : public COLLECTOR /// performed even if the search criteria hasn't changed. bool m_forceSearch; - /** - * Function PassedEnd - * tests if #m_foundIndex is beyond the end of the list give the current - * find/replace criterial in #m_findReplaceData. - * - * @return True if #m_foundIndex has crossed the end of the found item list. - */ - bool PassedEnd() const; - /** * Function dump * is a helper to dump the items in the find list for debugging purposes. @@ -266,8 +257,24 @@ public: m_forceSearch = false; } + void Empty() + { + m_foundIndex = 0; + COLLECTOR::Empty(); + m_data.clear(); + } + void SetForceSearch() { m_forceSearch = true; } + /** + * Function PassedEnd + * tests if #m_foundIndex is beyond the end of the list give the current + * find/replace criterial in #m_findReplaceData. + * + * @return True if #m_foundIndex has crossed the end of the found item list. + */ + bool PassedEnd() const; + /** * Function UpdateIndex * updates the list index according to the current find and replace criteria. @@ -326,7 +333,7 @@ public: * * @return True if the text replace occurred otherwise false. */ - bool ReplaceItem(); + bool ReplaceItem( SCH_SHEET_PATH* aSheetPath = NULL ); SEARCH_RESULT Inspect( EDA_ITEM* aItem, const void* aTestData = NULL ); @@ -340,6 +347,8 @@ public: * value searches the entire schematic hierarchy. */ void Collect( SCH_FIND_REPLACE_DATA& aFindReplaceData, SCH_SHEET_PATH* aSheetPath = NULL ); + + void IncrementIndex() { m_foundIndex += 1; } }; diff --git a/eeschema/sch_field.cpp b/eeschema/sch_field.cpp index 9ed6e461d1..67256b5653 100644 --- a/eeschema/sch_field.cpp +++ b/eeschema/sch_field.cpp @@ -431,8 +431,11 @@ bool SCH_FIELD::Replace( wxFindReplaceData& aSearchData, void* aAuxData ) bool isReplaced; wxString text = GetFullyQualifiedText(); - if( m_id == REFERENCE && aAuxData != NULL ) + if( m_id == REFERENCE ) { + wxCHECK_MSG( aAuxData != NULL, false, + wxT( "Cannot replace reference designator without valid sheet path." ) ); + wxCHECK_MSG( aSearchData.GetFlags() & FR_REPLACE_REFERENCES, false, wxT( "Invalid replace component reference field call." ) ) ; @@ -443,8 +446,8 @@ bool SCH_FIELD::Replace( wxFindReplaceData& aSearchData, void* aAuxData ) text = component->GetRef( (SCH_SHEET_PATH*) aAuxData ); - if( component->GetPartCount() > 1 ) - text << LIB_COMPONENT::ReturnSubReference( component->GetUnit() ); + // if( component->GetPartCount() > 1 ) + // text << LIB_COMPONENT::ReturnSubReference( component->GetUnit() ); isReplaced = EDA_ITEM::Replace( aSearchData, text ); diff --git a/eeschema/sch_sheet_path.cpp b/eeschema/sch_sheet_path.cpp index 71f777dc22..23ef5272c3 100644 --- a/eeschema/sch_sheet_path.cpp +++ b/eeschema/sch_sheet_path.cpp @@ -438,6 +438,7 @@ SCH_SHEET_LIST::SCH_SHEET_LIST( SCH_SHEET* aSheet ) m_index = 0; m_count = 0; m_List = NULL; + m_isRootSheet = false; if( aSheet == NULL ) aSheet = g_RootSheet; @@ -518,6 +519,9 @@ SCH_SHEET_PATH* SCH_SHEET_LIST::GetSheet( const wxString aPath, bool aHumanReada void SCH_SHEET_LIST::BuildSheetList( SCH_SHEET* aSheet ) { + if( aSheet == g_RootSheet ) + m_isRootSheet = true; + if( m_List == NULL ) { int count = aSheet->CountSheets(); @@ -702,3 +706,25 @@ bool SCH_SHEET_LIST::SetComponentFootprint( const wxString& aReference, return found; } + + +bool SCH_SHEET_LIST::IsComplexHierarchy() +{ + wxString fileName; + + for( int i = 0; i < GetCount(); i++ ) + { + fileName = GetSheet( i )->Last()->GetFileName(); + + for( int j = 0; j < GetCount(); j++ ) + { + if( i == j ) + continue; + + if( fileName == GetSheet( j )->Last()->GetFileName() ) + return true; + } + } + + return false; +} diff --git a/eeschema/sch_sheet_path.h b/eeschema/sch_sheet_path.h index 4ab4237064..dcfe9b690a 100644 --- a/eeschema/sch_sheet_path.h +++ b/eeschema/sch_sheet_path.h @@ -292,6 +292,7 @@ private: * returning the next item in m_List. Also used for * internal calculations in BuildSheetList() */ + bool m_isRootSheet; SCH_SHEET_PATH m_currList; public: @@ -442,6 +443,15 @@ public: bool SetComponentFootprint( const wxString& aReference, const wxString& aFootPrint, bool aSetVisible ); + /** + * Function IsComplexHierarchy + * searches all of the sheets for duplicate files names which indicates a complex + * hierarchy. + * + * @return true if the #SCH_SHEET_LIST is a complex hierarchy. + */ + bool IsComplexHierarchy(); + private: /** diff --git a/include/base_struct.h b/include/base_struct.h index 2fce8196a3..eb743d07fe 100644 --- a/include/base_struct.h +++ b/include/base_struct.h @@ -46,6 +46,10 @@ extern std::ostream& operator <<( std::ostream& out, const wxPoint& pt ); #endif +/// Flag to enable find and replace tracing using the WXTRACE environment variable. +extern const wxString traceFindReplace; + + /** * Enum KICAD_T * is the set of class identification values, stored in EDA_ITEM::m_StructType diff --git a/include/sch_item_struct.h b/include/sch_item_struct.h index 6d4d690235..788ef66160 100644 --- a/include/sch_item_struct.h +++ b/include/sch_item_struct.h @@ -56,9 +56,6 @@ typedef vector< SCH_ITEMS_ITR > SCH_ITEMS_ITRS; #define FMT_ANGLE SCH_ITEM::FormatAngle -/// Flag to enable find and replace tracing using the WXTRACE environment variable. -extern const wxString traceFindReplace; - /// Flag to enable find item tracing using the WXTRACE environment variable. This /// flag generates a lot of debug output. extern const wxString traceFindItem; diff --git a/include/wxEeschemaStruct.h b/include/wxEeschemaStruct.h index 8615aae6c4..4b4ca81566 100644 --- a/include/wxEeschemaStruct.h +++ b/include/wxEeschemaStruct.h @@ -194,6 +194,8 @@ protected: */ void addCurrentItemToList( wxDC* aDC ); + void updateFindReplaceView( wxFindDialogEvent& aEvent ); + public: SCH_EDIT_FRAME( wxWindow* aParent, const wxString& aTitle, const wxPoint& aPosition, const wxSize& aSize,