From 2c75911669eeb96521116886f1ceb39fabc415a3 Mon Sep 17 00:00:00 2001
From: Roberto Fernandez Bautista <roberto.fer.bau@gmail.com>
Date: Sun, 11 Apr 2021 16:24:11 +0100
Subject: [PATCH] Fix SortByPageNumbers: use Sheet Name when page numbers are
 equal

---
 eeschema/hierarch.cpp       |  5 +----
 eeschema/sch_sheet.cpp      | 22 +++++++++-------------
 eeschema/sch_sheet_path.cpp | 26 ++++++++++++++++++++++----
 eeschema/sch_sheet_path.h   | 13 +++++++++++++
 4 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/eeschema/hierarch.cpp b/eeschema/hierarch.cpp
index 2088031aac..a69e90e2b6 100644
--- a/eeschema/hierarch.cpp
+++ b/eeschema/hierarch.cpp
@@ -166,10 +166,7 @@ int HIERARCHY_TREE::OnCompareItems( const wxTreeItemId& item1, const wxTreeItemI
     SCH_SHEET_PATH* item1Path = &static_cast<TreeItemData*>( GetItemData( item1 ) )->m_SheetPath;
     SCH_SHEET_PATH* item2Path = &static_cast<TreeItemData*>( GetItemData( item2 ) )->m_SheetPath;
 
-    wxString item1PageNo = item1Path->Last()->GetPageNumber( *item1Path );
-    wxString item2PageNo = item2Path->Last()->GetPageNumber( *item2Path );
-
-    return SCH_SHEET::ComparePageNum( item1PageNo, item2PageNo );
+    return item1Path->ComparePageNumAndName( *item2Path );
 }
 
 
diff --git a/eeschema/sch_sheet.cpp b/eeschema/sch_sheet.cpp
index 503a980cb5..0310a19b7f 100644
--- a/eeschema/sch_sheet.cpp
+++ b/eeschema/sch_sheet.cpp
@@ -1150,7 +1150,7 @@ void SCH_SHEET::SetPageNumber( const SCH_SHEET_PATH& aInstance, const wxString&
 int SCH_SHEET::ComparePageNum( const wxString& aPageNumberA, const wxString aPageNumberB )
 {
     if( aPageNumberA == aPageNumberB )
-        return 1;
+        return 0; // A == B
 
     // First sort numerically if the page numbers are integers
     long pageA, pageB;
@@ -1159,29 +1159,25 @@ int SCH_SHEET::ComparePageNum( const wxString& aPageNumberA, const wxString aPag
 
     if( isIntegerPageA && isIntegerPageB )
     {
-        if( pageA > pageB )
-            return 1;
-        else if( pageA == pageB )
-            return 0;
+        if( pageA < pageB )
+            return -1; //A < B
         else
-            return -1;
+            return 1; // A > B
     }
 
     // Numerical page numbers always before strings
     if( isIntegerPageA )
-        return -1;
+        return -1; //A < B
     else if( isIntegerPageB )
-        return 1;
+        return 1; // A > B
 
     // If not numeric, then sort as strings
     int result = aPageNumberA.Cmp( aPageNumberB );
 
-    if( result == 0 )
-        return 0;
-    else if( result > 0 )
-        return 1;
+    if( result > 0 )
+        return 1; // A > B
 
-    return -1;
+    return -1; //A < B
 }
 
 
diff --git a/eeschema/sch_sheet_path.cpp b/eeschema/sch_sheet_path.cpp
index 7706cad48f..cf4c32de68 100644
--- a/eeschema/sch_sheet_path.cpp
+++ b/eeschema/sch_sheet_path.cpp
@@ -156,6 +156,27 @@ int SCH_SHEET_PATH::Cmp( const SCH_SHEET_PATH& aSheetPathToTest ) const
 }
 
 
+int SCH_SHEET_PATH::ComparePageNumAndName( const SCH_SHEET_PATH& aSheetPathToTest ) const
+{
+    wxString pageA = GetPageNumber();
+    wxString pageB = aSheetPathToTest.GetPageNumber();
+
+    int pageNumComp = SCH_SHEET::ComparePageNum( pageA, pageB );
+
+    if( pageNumComp == 0 )
+    {
+        wxString nameA = Last()->GetName();
+        wxString nameB = aSheetPathToTest.Last()->GetName();
+
+        return nameA.Cmp( nameB );
+    }
+    else
+    {
+        return pageNumComp;
+    }
+}
+
+
 SCH_SHEET* SCH_SHEET_PATH::Last() const
 {
     if( !empty() )
@@ -499,10 +520,7 @@ void SCH_SHEET_LIST::SortByPageNumbers( bool aUpdateVirtualPageNums )
     std::sort( begin(), end(),
         []( SCH_SHEET_PATH a, SCH_SHEET_PATH b ) -> bool
         {
-            wxString pageA = a.GetPageNumber();
-            wxString pageB = b.GetPageNumber();
-
-            return SCH_SHEET::ComparePageNum( pageA, pageB ) < 0;
+             return a.ComparePageNumAndName(b) < 0;
         } );
 
     if( aUpdateVirtualPageNums )
diff --git a/eeschema/sch_sheet_path.h b/eeschema/sch_sheet_path.h
index e662cc4103..3839f661ca 100644
--- a/eeschema/sch_sheet_path.h
+++ b/eeschema/sch_sheet_path.h
@@ -213,6 +213,16 @@ public:
      */
     int Cmp( const SCH_SHEET_PATH& aSheetPathToTest ) const;
 
+    /**
+     * Compare sheets by their page number and then by their name. Finally
+     * compare using #Cmp()
+     *
+     * @return -1 if aSheetPathToTest is greater than this (should appear later in the sort order)
+     *          0 if aSheetPathToTest is equal to this
+     *          1 if aSheetPathToTest is less than this (should appear earlier in the sort order)
+     */
+    int ComparePageNumAndName( const SCH_SHEET_PATH& aSheetPathToTest ) const;
+
     /**
      * Return a pointer to the last #SCH_SHEET of the list.
      *
@@ -469,6 +479,9 @@ public:
     /**
      * Sort the list of sheets by page number. This should be called after #BuildSheetList
      *
+     * If page numbers happen to be equal, then it compares the sheet names to ensure deterministic
+     * ordering.
+     *
      * @param aUpdateVirtualPageNums If true, updates the virtual page numbers to match the new
      * ordering
      */