From c4e2c0d3ec7cec15b6bfdb386c5bb0ce88a3f570 Mon Sep 17 00:00:00 2001
From: Wayne Stambaugh <stambaughw@gmail.com>
Date: Thu, 19 Dec 2024 09:19:35 -0500
Subject: [PATCH] Save schematic symbol field and unit by ordinal sheet
 instance.

Use the ordinal symbol reference and unit when saving schematics to
prevent file changes instead of the first instance.  The first instance
would change when sheets were moved and the hierarchy order changed.
The ordinal instance is defined as the first sheet instance when the
hierarchy sorted by page number.
---
 .../sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp | 28 ++++++++++++++++---
 eeschema/schematic.cpp                        |  4 +--
 eeschema/schematic.h                          |  6 +++-
 eeschema/widgets/hierarchy_pane.cpp           |  6 ++--
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp
index 0678b2b99c..d5bef1d2f9 100644
--- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp
+++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp
@@ -691,10 +691,30 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
                       mirrorY ? "y" : "" );
     }
 
-    // The symbol unit is always set to the first instance regardless of the current sheet
+    // The symbol unit is always set to the ordianal instance regardless of the current sheet
     // instance to prevent file churn.
-    int unit = ( aSymbol->GetInstances().size() == 0 ) ? aSymbol->GetUnit()
-                                                       : aSymbol->GetInstances()[0].m_Unit;
+    SCH_SYMBOL_INSTANCE ordinalInstance;
+
+    ordinalInstance.m_Reference = aSymbol->GetPrefix();
+
+    const SCH_SCREEN* parentScreen = static_cast<const SCH_SCREEN*>( aSymbol->GetParent() );
+
+    wxASSERT( parentScreen );
+
+    if( parentScreen && m_schematic )
+    {
+        std::optional<SCH_SHEET_PATH> ordinalPath =
+                m_schematic->Hierarchy().GetOrdinalPath( parentScreen );
+
+        wxASSERT( ordinalPath );
+
+        if( ordinalPath )
+            aSymbol->GetInstance( ordinalInstance, ordinalPath->Path() );
+        else if( aSymbol->GetInstances().size() )
+            ordinalInstance = aSymbol->GetInstances()[0];
+    }
+
+    int unit = ordinalInstance.m_Unit;
 
     if( aForClipboard && aRelativePath )
     {
@@ -732,7 +752,7 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
             // sheet instance to prevent file churn.
             if( id == REFERENCE_FIELD )
             {
-                field.SetText( aSymbol->GetInstances()[0].m_Reference );
+                field.SetText( ordinalInstance.m_Reference );
             }
             else if( id == VALUE_FIELD )
             {
diff --git a/eeschema/schematic.cpp b/eeschema/schematic.cpp
index be84355169..8ba07e4eac 100644
--- a/eeschema/schematic.cpp
+++ b/eeschema/schematic.cpp
@@ -221,9 +221,7 @@ SCH_SHEET_LIST SCHEMATIC::Hierarchy() const
 
 void SCHEMATIC::RefreshHierarchy()
 {
-    wxLogDebug( wxS( "Refreshing schematic heirarchy." ) );
-
-    m_hierarchy = SCH_SHEET_LIST( m_rootSheet );
+    m_hierarchy = BuildSheetListSortedByPageNumbers();
 }
 
 
diff --git a/eeschema/schematic.h b/eeschema/schematic.h
index 31a7b7bf96..d101a1edb2 100644
--- a/eeschema/schematic.h
+++ b/eeschema/schematic.h
@@ -96,7 +96,11 @@ public:
 
     SCH_SHEET_LIST BuildSheetListSortedByPageNumbers() const override
     {
-        return SCH_SHEET_LIST( m_rootSheet );
+        SCH_SHEET_LIST hierarchy( m_rootSheet );
+
+        hierarchy.SortByPageNumbers();
+
+        return hierarchy;
     }
 
     SCH_SHEET_LIST BuildUnorderedSheetList() const
diff --git a/eeschema/widgets/hierarchy_pane.cpp b/eeschema/widgets/hierarchy_pane.cpp
index f2ee8eeb8a..d4e3d5d8c6 100644
--- a/eeschema/widgets/hierarchy_pane.cpp
+++ b/eeschema/widgets/hierarchy_pane.cpp
@@ -417,11 +417,11 @@ void HIERARCHY_PANE::onRightClick( wxTreeItemId aItem )
 
         if( dlg.ShowModal() == wxID_OK && dlg.GetValue() != itemData->m_SheetPath.GetPageNumber() )
         {
+            SCH_COMMIT commit( m_frame );
             SCH_SHEET_PATH parentPath = itemData->m_SheetPath;
             parentPath.pop_back();
 
-            m_frame->SaveCopyInUndoList( parentPath.LastScreen(), itemData->m_SheetPath.Last(),
-                                         UNDO_REDO::CHANGED, false );
+            commit.Modify( itemData->m_SheetPath.Last(), parentPath.LastScreen() );
 
             itemData->m_SheetPath.SetPageNumber( dlg.GetValue() );
 
@@ -431,7 +431,7 @@ void HIERARCHY_PANE::onRightClick( wxTreeItemId aItem )
                 m_frame->OnPageSettingsChange();
             }
 
-            m_frame->OnModify();
+            commit.Push( wxS( "Change sheet page number." ) );
 
             UpdateLabelsHierarchyTree();
         }