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

@@ -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(),
+    wxString searchString = (aSearchData.GetFlags() & wxFR_MATCHCASE) ? aText.Upper() : aText;
-                          aSearchData.GetReplaceString(), false ) != 0;
+
+    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;
 }
 
 

common/sch_item_struct.cpp

@@ -39,8 +39,6 @@
 #include <protos.h>
 
 
-const wxString traceFindReplace( wxT( "KicadFindReplace" ) );
-
 const wxString traceFindItem( wxT( "KicadFindItem" ) );
 
 

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,

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;
+    SCH_SHEET_LIST          schematic;
-    wxString msg;
+    wxString                msg;
-    SCH_FIND_REPLACE_DATA searchCriteria;
+    SCH_FIND_REPLACE_DATA   searchCriteria;
-    bool warpCursor = !( aEvent.GetFlags() & FR_NO_WARP_CURSOR );
     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 );
+            item->SetForceVisible( false );
-        else
+        else if( item->Type() == SCH_FIELD_T && !( (SCH_FIELD*) item )->IsVisible() )
-            m_foundItems.GetItem( data )->SetForceVisible( true );
+            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 );
-        }
-    }
-}

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 )
-    {
+        m_forceSearch = true;
-        Remove( m_foundIndex );
-        m_data.erase( m_data.begin() + m_foundIndex );
-    }
 
     return replaced;
 }

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; }
 };
 
 

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 )
+        // if( component->GetPartCount() > 1 )
-            text << LIB_COMPONENT::ReturnSubReference( component->GetUnit() );
+        //     text << LIB_COMPONENT::ReturnSubReference( component->GetUnit() );
 
         isReplaced = EDA_ITEM::Replace( aSearchData, text );
 

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;
+}

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:
 
     /**

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

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;

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,