Make prev/next marker work reliably on all platforms.

Requires us to move from arrow-keys to our own commands (the only
way that the OSX wxWidgets impl doesn't eat the keys when the dataVIew
has focus).

While there we might as well add a command to exlucde markers.

ADDED Prev Marker, Next Marker, Exclude Marker

Fixes https://gitlab.com/kicad/code/kicad/issues/6575

Fixes https://gitlab.com/kicad/code/kicad/issues/5501

common/rc_item.cpp

@@ -514,6 +514,55 @@ void RC_TREE_MODEL::DeleteItems( bool aCurrentOnly, bool aIncludeExclusions, boo
 }
 
 
+void RC_TREE_MODEL::PrevMarker()
+{
+    RC_TREE_NODE* currentNode = ToNode( m_view->GetCurrentItem() );
+    RC_TREE_NODE* prevMarker = nullptr;
+
+    while( currentNode && currentNode->m_Type != RC_TREE_NODE::MARKER )
+        currentNode = currentNode->m_Parent;
+
+    for( RC_TREE_NODE* candidate : m_tree )
+    {
+        if( candidate == currentNode )
+            break;
+        else
+            prevMarker = candidate;
+    }
+
+    if( prevMarker )
+        m_view->Select( ToItem( prevMarker ) );
+}
+
+
+void RC_TREE_MODEL::NextMarker()
+{
+    RC_TREE_NODE* currentNode = ToNode( m_view->GetCurrentItem() );
+
+    while( currentNode && currentNode->m_Type != RC_TREE_NODE::MARKER )
+        currentNode = currentNode->m_Parent;
+
+    RC_TREE_NODE* nextMarker = nullptr;
+    bool          trigger = currentNode == nullptr;
+
+    for( RC_TREE_NODE* candidate : m_tree )
+    {
+        if( candidate == currentNode )
+        {
+            trigger = true;
+        }
+        else if( trigger )
+        {
+            nextMarker = candidate;
+            break;
+        }
+    }
+
+    if( nextMarker )
+        m_view->Select( ToItem( nextMarker ) );
+}
+
+
 void RC_TREE_MODEL::onSizeView( wxSizeEvent& aEvent )
 {
     int width = m_view->GetMainWindow()->GetRect().GetWidth() - WX_DATAVIEW_WINDOW_PADDING;

common/rc_item.h

@@ -237,6 +237,9 @@ public:
 
     void ExpandAll();
 
+    void PrevMarker();
+    void NextMarker();
+
     bool IsContainer( wxDataViewItem const& aItem ) const override;
 
     wxDataViewItem GetParent( wxDataViewItem const& aItem ) const override;

common/tool/actions.cpp

@@ -237,6 +237,26 @@ TOOL_ACTION ACTIONS::replaceAll( "common.Interactive.replaceAll",
 TOOL_ACTION ACTIONS::updateFind( "common.Control.updateFind",
         AS_GLOBAL );
 
+
+// Marker Controls
+TOOL_ACTION ACTIONS::prevMarker( "common.Checker.prevMarker",
+        AS_GLOBAL,
+        0, "",
+        _( "Previous Marker" ), _( "Go to previous marker in Checker window" ),
+        nullptr );
+
+TOOL_ACTION ACTIONS::nextMarker( "common.Checker.nextMarker",
+        AS_GLOBAL,
+        0, "",
+        _( "Next Marker" ), _( "Go to next marker in Checker window" ),
+        nullptr );
+
+TOOL_ACTION ACTIONS::excludeMarker( "common.Checker.excludeMarker",
+        AS_GLOBAL,
+        0, "",
+        _( "Exclude Marker" ), _( "Mark current violation in Checker window as an exclusion" ),
+        nullptr );
+
 // View Controls
 TOOL_ACTION ACTIONS::zoomRedraw( "common.Control.zoomRedraw",
         AS_GLOBAL,

eeschema/dialogs/dialog_erc.cpp

@@ -547,18 +547,34 @@ void DIALOG_ERC::OnERCItemRClick( wxDataViewEvent& aEvent )
                      _( "Open the Schematic Setup... dialog" ) );
     }
 
+    bool modified = false;
+
     switch( GetPopupMenuSelectionFromUser( menu ) )
     {
     case 1:
-        node->m_RcItem->GetParent()->SetExcluded( false );
+    {
+        SCH_MARKER* marker = dynamic_cast<SCH_MARKER*>( node->m_RcItem->GetParent() );
+
+        if( marker )
+        {
+            marker->SetExcluded( false );
+            m_parent->GetCanvas()->GetView()->Update( marker );
 
             // Update view
             static_cast<RC_TREE_MODEL*>( aEvent.GetModel() )->ValueChanged( node );
-        updateDisplayedCounts();
+            modified = true;
+        }
+    }
         break;
 
     case 2:
-        node->m_RcItem->GetParent()->SetExcluded( true );
+    {
+        SCH_MARKER* marker = dynamic_cast<SCH_MARKER*>( node->m_RcItem->GetParent() );
+
+        if( marker )
+        {
+            marker->SetExcluded( true );
+            m_parent->GetCanvas()->GetView()->Update( marker );
 
             // Update view
             if( m_severities & RPT_SEVERITY_EXCLUSION )
@@ -566,23 +582,41 @@ void DIALOG_ERC::OnERCItemRClick( wxDataViewEvent& aEvent )
             else
                 static_cast<RC_TREE_MODEL*>( aEvent.GetModel() )->DeleteCurrentItem( false );
 
-        updateDisplayedCounts();
+            modified = true;
+        }
+    }
         break;
 
     case 4:
         settings.SetSeverity( rcItem->GetErrorCode(), RPT_SEVERITY_ERROR );
 
+        for( SCH_ITEM* item : m_parent->GetScreen()->Items().OfType( SCH_MARKER_T ) )
+        {
+            SCH_MARKER* marker = static_cast<SCH_MARKER*>( item );
+
+            if( marker->GetRCItem()->GetErrorCode() == rcItem->GetErrorCode() )
+                m_parent->GetCanvas()->GetView()->Update( marker );
+        }
+
         // Rebuild model and view
         static_cast<RC_TREE_MODEL*>( aEvent.GetModel() )->SetProvider( m_markerProvider );
-        updateDisplayedCounts();
+        modified = true;
         break;
 
     case 5:
         settings.SetSeverity( rcItem->GetErrorCode(), RPT_SEVERITY_WARNING );
 
+        for( SCH_ITEM* item : m_parent->GetScreen()->Items().OfType( SCH_MARKER_T ) )
+        {
+            SCH_MARKER* marker = static_cast<SCH_MARKER*>( item );
+
+            if( marker->GetRCItem()->GetErrorCode() == rcItem->GetErrorCode() )
+                m_parent->GetCanvas()->GetView()->Update( marker );
+        }
+
         // Rebuild model and view
         static_cast<RC_TREE_MODEL*>( aEvent.GetModel() )->SetProvider( m_markerProvider );
-        updateDisplayedCounts();
+        modified = true;
         break;
 
     case 6:
@@ -597,7 +631,7 @@ void DIALOG_ERC::OnERCItemRClick( wxDataViewEvent& aEvent )
 
         // Rebuild model and view
         static_cast<RC_TREE_MODEL*>( aEvent.GetModel() )->SetProvider( m_markerProvider );
-        updateDisplayedCounts();
+        modified = true;
     }
         break;
 
@@ -609,6 +643,57 @@ void DIALOG_ERC::OnERCItemRClick( wxDataViewEvent& aEvent )
         m_parent->ShowSchematicSetupDialog( _( "Violation Severity" ) );
         break;
     }
+
+    if( modified )
+    {
+        updateDisplayedCounts();
+        redrawDrawPanel();
+        m_parent->OnModify();
+    }
+}
+
+
+void DIALOG_ERC::PrevMarker()
+{
+    if( m_notebook->GetSelection() != 1 )
+        m_notebook->SetSelection( 1 );
+
+    m_markerTreeModel->PrevMarker();
+}
+
+
+void DIALOG_ERC::NextMarker()
+{
+    if( m_notebook->GetSelection() != 1 )
+        m_notebook->SetSelection( 1 );
+
+    m_markerTreeModel->NextMarker();
+}
+
+
+void DIALOG_ERC::ExcludeMarker()
+{
+    if( m_notebook->GetSelection() != 1 )
+        return;
+
+    RC_TREE_NODE* node = RC_TREE_MODEL::ToNode( m_markerDataView->GetCurrentItem() );
+    SCH_MARKER*   marker = dynamic_cast<SCH_MARKER*>( node->m_RcItem->GetParent() );
+
+    if( marker && !marker->IsExcluded() )
+    {
+        marker->SetExcluded( true );
+        m_parent->GetCanvas()->GetView()->Update( marker );
+
+        // Update view
+        if( m_severities & RPT_SEVERITY_EXCLUSION )
+            m_markerTreeModel->ValueChanged( node );
+        else
+            m_markerTreeModel->DeleteCurrentItem( false );
+
+        updateDisplayedCounts();
+        redrawDrawPanel();
+        m_parent->OnModify();
+    }
 }
 
 

eeschema/dialogs/dialog_erc.h

@@ -57,6 +57,10 @@ public:
     void AdvancePhase( const wxString& aMessage ) override;
     void Report( const wxString& aMessage ) override;
 
+    void PrevMarker();
+    void NextMarker();
+    void ExcludeMarker();
+
 private:
     // from DIALOG_ERC_BASE:
     void OnCloseErcDialog( wxCloseEvent& event ) override;

eeschema/menubar.cpp

@@ -223,7 +223,12 @@ void SCH_EDIT_FRAME::ReCreateMenuBar()
     ACTION_MENU* inspectMenu = new ACTION_MENU( false, selTool );
 
     inspectMenu->Add( EE_ACTIONS::runERC );
+    inspectMenu->Add( ACTIONS::prevMarker );
+    inspectMenu->Add( ACTIONS::nextMarker );
+    inspectMenu->Add( ACTIONS::excludeMarker );
+
 #ifdef KICAD_SPICE
+    inspectMenu->AppendSeparator();
     inspectMenu->Add( EE_ACTIONS::runSimulation );
 #endif
 

eeschema/tools/ee_inspection_tool.cpp

@@ -81,6 +81,13 @@ void EE_INSPECTION_TOOL::Reset( RESET_REASON aReason )
 
 
 int EE_INSPECTION_TOOL::RunERC( const TOOL_EVENT& aEvent )
+{
+    ShowERCDialog();
+    return 0;
+}
+
+
+void EE_INSPECTION_TOOL::ShowERCDialog()
 {
     if( m_frame->IsType( FRAME_SCH ) )
     {
@@ -99,8 +106,6 @@ int EE_INSPECTION_TOOL::RunERC( const TOOL_EVENT& aEvent )
             m_ercDialog->Show( true );
         }
     }
-
-    return 0;
 }
 
 
@@ -113,6 +118,49 @@ void EE_INSPECTION_TOOL::DestroyERCDialog()
 }
 
 
+int EE_INSPECTION_TOOL::PrevMarker( const TOOL_EVENT& aEvent )
+{
+    if( m_ercDialog )
+    {
+        m_ercDialog->Show( true );
+        m_ercDialog->Raise();
+        m_ercDialog->PrevMarker();
+    }
+    else
+    {
+        ShowERCDialog();
+    }
+
+    return 0;
+}
+
+
+int EE_INSPECTION_TOOL::NextMarker( const TOOL_EVENT& aEvent )
+{
+    if( m_ercDialog )
+    {
+        m_ercDialog->Show( true );
+        m_ercDialog->Raise();
+        m_ercDialog->NextMarker();
+    }
+    else
+    {
+        ShowERCDialog();
+    }
+
+    return 0;
+}
+
+
+int EE_INSPECTION_TOOL::ExcludeMarker( const TOOL_EVENT& aEvent )
+{
+    if( m_ercDialog )
+        m_ercDialog->ExcludeMarker();
+
+    return 0;
+}
+
+
 // helper function to sort pins by pin num
 bool sort_by_pin_number( const LIB_PIN* ref, const LIB_PIN* tst )
 {
@@ -400,6 +448,10 @@ int EE_INSPECTION_TOOL::UpdateMessagePanel( const TOOL_EVENT& aEvent )
 void EE_INSPECTION_TOOL::setTransitions()
 {
     Go( &EE_INSPECTION_TOOL::RunERC,              EE_ACTIONS::runERC.MakeEvent() );
+    Go( &EE_INSPECTION_TOOL::PrevMarker,          EE_ACTIONS::prevMarker.MakeEvent() );
+    Go( &EE_INSPECTION_TOOL::NextMarker,          EE_ACTIONS::nextMarker.MakeEvent() );
+    Go( &EE_INSPECTION_TOOL::ExcludeMarker,       EE_ACTIONS::excludeMarker.MakeEvent() );
+
     Go( &EE_INSPECTION_TOOL::CheckSymbol,         EE_ACTIONS::checkSymbol.MakeEvent() );
     Go( &EE_INSPECTION_TOOL::RunSimulation,       EE_ACTIONS::runSimulation.MakeEvent() );
 

eeschema/tools/ee_inspection_tool.h

@@ -47,8 +47,13 @@ public:
     void Reset( RESET_REASON aReason ) override;
 
     int RunERC( const TOOL_EVENT& aEvent );
+    void ShowERCDialog();
     void DestroyERCDialog();
 
+    int PrevMarker( const TOOL_EVENT& aEvent );
+    int NextMarker( const TOOL_EVENT& aEvent );
+    int ExcludeMarker( const TOOL_EVENT& aEvent );
+
     int CheckSymbol( const TOOL_EVENT& aEvent );
 
     int RunSimulation( const TOOL_EVENT& aEvent );

include/tool/actions.h

@@ -84,6 +84,11 @@ public:
     static TOOL_ACTION replaceAll;
     static TOOL_ACTION updateFind;
 
+    // RC Lists
+    static TOOL_ACTION prevMarker;
+    static TOOL_ACTION nextMarker;
+    static TOOL_ACTION excludeMarker;
+
     // View controls
     static TOOL_ACTION zoomRedraw;
     static TOOL_ACTION zoomIn;

pcbnew/dialogs/dialog_drc.cpp

@@ -667,6 +667,60 @@ void DIALOG_DRC::refreshBoardEditor()
 }
 
 
+void DIALOG_DRC::PrevMarker()
+{
+    if( m_Notebook->IsShown() )
+    {
+        switch( m_Notebook->GetSelection() )
+        {
+        case 0: m_markerTreeModel->PrevMarker();            break;
+        case 1: m_unconnectedTreeModel->PrevMarker();       break;
+        case 2: m_footprintWarningsTreeModel->PrevMarker(); break;
+        }
+    }
+}
+
+
+void DIALOG_DRC::NextMarker()
+{
+    if( m_Notebook->IsShown() )
+    {
+        switch( m_Notebook->GetSelection() )
+        {
+        case 0: m_markerTreeModel->NextMarker();            break;
+        case 1: m_unconnectedTreeModel->NextMarker();       break;
+        case 2: m_footprintWarningsTreeModel->NextMarker(); break;
+        }
+    }
+}
+
+
+void DIALOG_DRC::ExcludeMarker()
+{
+    if( !m_Notebook->IsShown() || m_Notebook->GetSelection() != 0 )
+        return;
+
+    RC_TREE_NODE* node = RC_TREE_MODEL::ToNode( m_markerDataView->GetCurrentItem() );
+    PCB_MARKER*   marker = dynamic_cast<PCB_MARKER*>( node->m_RcItem->GetParent() );
+
+    if( marker && !marker->IsExcluded() )
+    {
+        marker->SetExcluded( true );
+        m_brdEditor->GetCanvas()->GetView()->Update( marker );
+
+        // Update view
+        if( m_severities & RPT_SEVERITY_EXCLUSION )
+            m_markerTreeModel->ValueChanged( node );
+        else
+            m_markerTreeModel->DeleteCurrentItem( false );
+
+        updateDisplayedCounts();
+        refreshBoardEditor();
+        m_brdEditor->OnModify();
+    }
+}
+
+
 void DIALOG_DRC::deleteAllMarkers( bool aIncludeExclusions )
 {
     // Clear current selection list to avoid selection of deleted items

pcbnew/dialogs/dialog_drc.h

@@ -55,6 +55,10 @@ public:
     void SetUnconnectedProvider( RC_ITEMS_PROVIDER* aProvider );
     void SetFootprintsProvider( RC_ITEMS_PROVIDER* aProvider );
 
+    void PrevMarker();
+    void NextMarker();
+    void ExcludeMarker();
+
 private:
     /**
      * Function writeReport

pcbnew/menubar_pcb_editor.cpp

@@ -362,11 +362,16 @@ void PCB_EDIT_FRAME::ReCreateMenuBar()
     ACTION_MENU* inspectMenu = new ACTION_MENU( false, selTool );
 
     inspectMenu->Add( PCB_ACTIONS::listNets );
-    inspectMenu->Add( ACTIONS::measureTool );
     inspectMenu->Add( PCB_ACTIONS::boardStatistics );
+    inspectMenu->Add( ACTIONS::measureTool );
 
     inspectMenu->AppendSeparator();
     inspectMenu->Add( PCB_ACTIONS::runDRC );
+    inspectMenu->Add( ACTIONS::prevMarker );
+    inspectMenu->Add( ACTIONS::nextMarker );
+    inspectMenu->Add( ACTIONS::excludeMarker );
+
+    inspectMenu->AppendSeparator();
     inspectMenu->Add( PCB_ACTIONS::inspectClearance );
     inspectMenu->Add( PCB_ACTIONS::inspectConstraints );
 

pcbnew/tools/drc_tool.cpp

@@ -239,9 +239,55 @@ void DRC_TOOL::updatePointers()
 }
 
 
-void DRC_TOOL::setTransitions()
+int DRC_TOOL::PrevMarker( const TOOL_EVENT& aEvent )
 {
-    Go( &DRC_TOOL::ShowDRCDialog,              PCB_ACTIONS::runDRC.MakeEvent() );
+    if( m_drcDialog )
+    {
+        m_drcDialog->Show( true );
+        m_drcDialog->Raise();
+        m_drcDialog->PrevMarker();
+    }
+    else
+    {
+        ShowDRCDialog( nullptr );
+    }
+
+    return 0;
+}
+
+
+int DRC_TOOL::NextMarker( const TOOL_EVENT& aEvent )
+{
+    if( m_drcDialog )
+    {
+        m_drcDialog->Show( true );
+        m_drcDialog->Raise();
+        m_drcDialog->NextMarker();
+    }
+    else
+    {
+        ShowDRCDialog( nullptr );
+    }
+
+    return 0;
+}
+
+
+int DRC_TOOL::ExcludeMarker( const TOOL_EVENT& aEvent )
+{
+    if( m_drcDialog )
+        m_drcDialog->ExcludeMarker();
+
+    return 0;
+}
+
+
+void DRC_TOOL::setTransitions()
+{
+    Go( &DRC_TOOL::ShowDRCDialog,              PCB_ACTIONS::runDRC.MakeEvent() );
+    Go( &DRC_TOOL::PrevMarker,                 ACTIONS::prevMarker.MakeEvent() );
+    Go( &DRC_TOOL::NextMarker,                 ACTIONS::nextMarker.MakeEvent() );
+    Go( &DRC_TOOL::ExcludeMarker,              ACTIONS::excludeMarker.MakeEvent() );
 }
 
 

pcbnew/tools/drc_tool.h

@@ -117,6 +117,10 @@ public:
      */
     void RunTests( PROGRESS_REPORTER* aProgressReporter, bool aRefillZones,
                    bool aReportAllTrackErrors, bool aTestFootprints );
+
+    int PrevMarker( const TOOL_EVENT& aEvent );
+    int NextMarker( const TOOL_EVENT& aEvent );
+    int ExcludeMarker( const TOOL_EVENT& aEvent );
 };