From 9380d6f533953cad1c8c59becd55ca173f2f47a1 Mon Sep 17 00:00:00 2001 From: Roberto Fernandez Bautista Date: Sun, 29 Nov 2020 00:10:17 +0000 Subject: [PATCH] eeschema page numbers: match ordering in hierarchy navigator to sheet page number ordering - update hierarchy navigator after any modifications to the SCH_EDIT_FRAME - do not generate ghost selection events after updating hierarchy tree - use Human Readable path in SCH_EDIT_TOOL::EditPageNumber Fixes https://gitlab.com/kicad/code/kicad/-/issues/5760 --- eeschema/hierarch.cpp | 65 ++++++++++++++++++++++++++++---- eeschema/hierarch.h | 19 ++++++++++ eeschema/sch_edit_frame.cpp | 1 + eeschema/sch_sheet.cpp | 38 +++++++++++++++++++ eeschema/sch_sheet.h | 8 ++++ eeschema/tools/sch_edit_tool.cpp | 2 +- 6 files changed, 125 insertions(+), 8 deletions(-) diff --git a/eeschema/hierarch.cpp b/eeschema/hierarch.cpp index 5e6aeccd45..877dff0baa 100644 --- a/eeschema/hierarch.cpp +++ b/eeschema/hierarch.cpp @@ -42,6 +42,7 @@ #include #include "eeschema_settings.h" +#include #include class HIERARCHY_NAVIG_DLG; @@ -61,6 +62,11 @@ public: } }; +// Need to use wxRTTI macros in order for OnCompareItems to work properly +// See: https://docs.wxwidgets.org/3.1/classwx_tree_ctrl.html#ab90a465793c291ca7aa827a576b7d146 +wxIMPLEMENT_ABSTRACT_CLASS( HIERARCHY_TREE, wxTreeCtrl ); + + HIERARCHY_TREE::HIERARCHY_TREE( HIERARCHY_NAVIG_DLG* parent ) : wxTreeCtrl( (wxWindow*) parent, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTR_HAS_BUTTONS, wxDefaultValidator, wxT( "HierachyTreeCtrl" ) ) @@ -78,6 +84,7 @@ HIERARCHY_TREE::HIERARCHY_TREE( HIERARCHY_NAVIG_DLG* parent ) : AssignImageList( imageList ); } + HIERARCHY_NAVIG_DLG::HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ) : DIALOG_SHIM( aParent, wxID_ANY, _( "Navigator" ), wxDefaultPosition, wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER, HIERARCHY_NAVIG_DLG_WNAME ) @@ -90,7 +97,7 @@ HIERARCHY_NAVIG_DLG::HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ) : m_nbsheets = 1; // root is the link to the main sheet. - wxTreeItemId root = m_Tree->AddRoot( _( "Root" ), 0, 1 ); + wxTreeItemId root = m_Tree->AddRoot( getRootString(), 0, 1 ); m_Tree->SetItemBold( root, true ); m_list.push_back( &m_SchFrameEditor->Schematic().Root() ); @@ -156,7 +163,13 @@ void HIERARCHY_TREE::onChar( wxKeyEvent& event ) int HIERARCHY_TREE::OnCompareItems( const wxTreeItemId& item1, const wxTreeItemId& item2 ) { - return GetItemText( item1 ).CmpNoCase( GetItemText( item2 ) ); + SCH_SHEET_PATH* item1Path = &static_cast( GetItemData( item1 ) )->m_SheetPath; + SCH_SHEET_PATH* item2Path = &static_cast( GetItemData( item2 ) )->m_SheetPath; + + wxString item1PageNo = item1Path->Last()->GetPageNumber( *item1Path ); + wxString item2PageNo = item2Path->Last()->GetPageNumber( *item2Path ); + + return SCH_SHEET::ComparePageNum( item1PageNo, item2PageNo ); } @@ -170,11 +183,13 @@ void HIERARCHY_NAVIG_DLG::buildHierarchyTree( SCH_SHEET_PATH* aList, wxTreeItemI for( SCH_ITEM* aItem : sheetChildren ) { SCH_SHEET* sheet = static_cast( aItem ); - wxString sheetName = sheet->GetFields()[ SHEETNAME ].GetShownText(); + aList->push_back( sheet ); + + wxString sheetName = formatPageString( sheet->GetFields()[SHEETNAME].GetShownText(), + sheet->GetPageNumber( *aList ) ); m_nbsheets++; wxTreeItemId menu; menu = m_Tree->AppendItem( *aPreviousmenu, sheetName, 0, 1 ); - aList->push_back( sheet ); m_Tree->SetItemData( menu, new TreeItemData( *aList ) ); if( *aList == m_currSheet ) @@ -193,20 +208,40 @@ void HIERARCHY_NAVIG_DLG::buildHierarchyTree( SCH_SHEET_PATH* aList, wxTreeItemI m_Tree->SortChildren( *aPreviousmenu ); } + void HIERARCHY_NAVIG_DLG::UpdateHierarchyTree() { Freeze(); - m_currSheet = m_SchFrameEditor->GetCurrentSheet(); - wxTreeItemId root = m_Tree->GetRootItem(); - m_Tree->DeleteChildren( root ); + // Disable selection events + Unbind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); + Unbind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); + + m_currSheet = m_SchFrameEditor->GetCurrentSheet(); + m_Tree->DeleteAllItems(); + m_nbsheets = 1; + + wxTreeItemId root = m_Tree->AddRoot( getRootString(), 0, 1 ); + m_Tree->SetItemBold( root, true ); + m_list.clear(); m_list.push_back( &m_SchFrameEditor->Schematic().Root() ); + m_Tree->SetItemData( root, new TreeItemData( m_list ) ); + + if( m_SchFrameEditor->GetCurrentSheet().Last() == &m_SchFrameEditor->Schematic().Root() ) + m_Tree->SelectItem( root ); + buildHierarchyTree( &m_list, &root ); + m_Tree->ExpandAll(); + + // Enable selection events + Bind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); + Bind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); Thaw(); } + void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event ) { m_SchFrameEditor->GetToolManager()->RunAction( ACTIONS::cancelInteractive, true ); @@ -225,6 +260,22 @@ void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event ) } +wxString HIERARCHY_NAVIG_DLG::getRootString() +{ + SCH_SHEET* rootSheet = &m_SchFrameEditor->Schematic().Root(); + SCH_SHEET_PATH rootPath; + rootPath.push_back( rootSheet ); + + return formatPageString ( _( "Root" ), rootSheet->GetPageNumber( rootPath ) ); +} + + +wxString HIERARCHY_NAVIG_DLG::formatPageString( wxString aName, wxString aPage ) +{ + return aName + wxT( " " ) + wxString::Format( _( "(page %s)" ), aPage ); +} + + void HIERARCHY_NAVIG_DLG::OnCloseNav( wxCloseEvent& event ) { Destroy(); diff --git a/eeschema/hierarch.h b/eeschema/hierarch.h index 8c989a5cd7..c0982d16b5 100644 --- a/eeschema/hierarch.h +++ b/eeschema/hierarch.h @@ -29,6 +29,7 @@ #include #include +#include // wxRTTI macros #include // The window name of the hierarchy navigator, used to find it @@ -44,6 +45,10 @@ class HIERARCHY_NAVIG_DLG; */ class HIERARCHY_TREE : public wxTreeCtrl { + // Need to use wxRTTI macros in order for OnCompareItems to work properly + // See: https://docs.wxwidgets.org/3.1/classwx_tree_ctrl.html#ab90a465793c291ca7aa827a576b7d146 + wxDECLARE_ABSTRACT_CLASS( HIERARCHY_TREE ); + private: HIERARCHY_NAVIG_DLG* m_parent; wxImageList* imageList; @@ -93,6 +98,20 @@ private: * selected. */ void onSelectSheetPath( wxTreeEvent& event ); + + /** + * getRootString + * @return String with page number in parenthesis + */ + wxString getRootString(); + + /** + * formatPageString + * @param aName + * @param aPage + * @return String with page number in parenthesis + */ + wxString formatPageString( wxString aName, wxString aPage ); }; #endif // HIERARCH_H diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 21ec90eb41..85fe2e287b 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -793,6 +793,7 @@ void SCH_EDIT_FRAME::OnModify() } ); GetCanvas()->Refresh(); + UpdateHierarchyNavigator(); } diff --git a/eeschema/sch_sheet.cpp b/eeschema/sch_sheet.cpp index d61d36b4c2..8135474b4e 100644 --- a/eeschema/sch_sheet.cpp +++ b/eeschema/sch_sheet.cpp @@ -1104,6 +1104,44 @@ 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; + + // First sort numerically if the page numbers are integers + long pageA, pageB; + bool isIntegerPageA = aPageNumberA.ToLong( &pageA ); + bool isIntegerPageB = aPageNumberB.ToLong( &pageB ); + + if( isIntegerPageA && isIntegerPageB ) + { + if( pageA > pageB ) + return 1; + else if( pageA == pageB ) + return 0; + else + return -1; + } + + // Numerical page numbers always before strings + if( isIntegerPageA ) + return -1; + else if( isIntegerPageB ) + return 1; + + // If not numeric, then sort as strings + int result = aPageNumberA.Cmp( aPageNumberB ); + + if( result == 0 ) + return 0; + else if( result > 0 ) + return 1; + + return -1; +} + + #if defined(DEBUG) void SCH_SHEET::Show( int nestLevel, std::ostream& os ) const diff --git a/eeschema/sch_sheet.h b/eeschema/sch_sheet.h index 635962c1cc..07d00a9bd0 100644 --- a/eeschema/sch_sheet.h +++ b/eeschema/sch_sheet.h @@ -610,6 +610,14 @@ public: */ void SetPageNumber( const SCH_SHEET_PATH& aInstance, const wxString& aPageNumber ); + /** + * @brief Compares page numbers of schematic sheets. Currently a basic + * @param aPageNumberA + * @param aPageNumberB + * @return 0 if the page numbers are equal, -1 if aPageNumberA < aPageNumberB, 1 otherwise + */ + static int ComparePageNum( const wxString& aPageNumberA, const wxString aPageNumberB ); + #if defined(DEBUG) void Show( int nestLevel, std::ostream& os ) const override; #endif diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index fa0e209921..8d6ae387cc 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1717,7 +1717,7 @@ int SCH_EDIT_TOOL::EditPageNumber( const TOOL_EVENT& aEvent ) } wxString msg; - wxString sheetPath = instance.PathAsString(); + wxString sheetPath = instance.PathHumanReadable( false ); wxString pageNumber = instance.GetPageNumber(); msg.Printf( _( "Enter page number for sheet path%s" ),