From 13f6153f52332a8eaef90375ed85fedf73556899 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 27 Feb 2026 18:09:05 +0000 Subject: [PATCH] Fix some TODOs in toolbars. Fixes https://gitlab.com/kicad/code/kicad/-/issues/23057 --- eeschema/eeschema_id.h | 2 - .../symbol_editor/toolbars_symbol_editor.cpp | 11 +-- eeschema/symbol_viewer_frame.cpp | 83 ++++++++----------- eeschema/symbol_viewer_frame.h | 7 +- eeschema/toolbars_symbol_viewer.cpp | 11 +-- eeschema/tools/sch_actions.cpp | 12 +++ eeschema/tools/sch_actions.h | 2 + eeschema/tools/symbol_editor_control.cpp | 21 +++++ eeschema/tools/symbol_editor_control.h | 9 +- 9 files changed, 83 insertions(+), 75 deletions(-) diff --git a/eeschema/eeschema_id.h b/eeschema/eeschema_id.h index 2373b0fb79..a8e8ec3489 100644 --- a/eeschema/eeschema_id.h +++ b/eeschema/eeschema_id.h @@ -66,8 +66,6 @@ enum id_eeschema_frm ID_LIBEDIT_SELECT_BODY_STYLE, /* Library viewer horizontal toolbar IDs */ - ID_LIBVIEW_NEXT, - ID_LIBVIEW_PREVIOUS, ID_LIBVIEW_SELECT_UNIT_NUMBER, ID_LIBVIEW_SELECT_BODY_STYLE, ID_LIBVIEW_LIB_FILTER, diff --git a/eeschema/symbol_editor/toolbars_symbol_editor.cpp b/eeschema/symbol_editor/toolbars_symbol_editor.cpp index aac6f48ab2..a6fc205e6a 100644 --- a/eeschema/symbol_editor/toolbars_symbol_editor.cpp +++ b/eeschema/symbol_editor/toolbars_symbol_editor.cpp @@ -110,14 +110,9 @@ std::optional SYMBOL_EDIT_TOOLBAR_SETTINGS::DefaultToolba break; case TOOLBAR_LOC::TOP_MAIN: - config.AppendAction( SCH_ACTIONS::newSymbol ); - -/* TODO (ISM): Handle visibility changes - if( !IsSymbolFromSchematic() ) - config.AppendAction( ACTIONS::saveAll ); - else - config.AppendAction( ACTIONS::save ); -*/ + config.AppendAction( SCH_ACTIONS::newSymbol ) + .AppendAction( ACTIONS::saveAll ) + .AppendAction( ACTIONS::save ); config.AppendSeparator() .AppendAction( ACTIONS::undo ) diff --git a/eeschema/symbol_viewer_frame.cpp b/eeschema/symbol_viewer_frame.cpp index b1d4e5be86..b69c54e302 100644 --- a/eeschema/symbol_viewer_frame.cpp +++ b/eeschema/symbol_viewer_frame.cpp @@ -85,8 +85,6 @@ BEGIN_EVENT_TABLE( SYMBOL_VIEWER_FRAME, SCH_BASE_FRAME ) EVT_ACTIVATE( SYMBOL_VIEWER_FRAME::OnActivate ) // Toolbar events - EVT_TOOL( ID_LIBVIEW_NEXT, SYMBOL_VIEWER_FRAME::onSelectNextSymbol ) - EVT_TOOL( ID_LIBVIEW_PREVIOUS, SYMBOL_VIEWER_FRAME::onSelectPreviousSymbol ) EVT_CHOICE( ID_LIBVIEW_SELECT_UNIT_NUMBER, SYMBOL_VIEWER_FRAME::onSelectSymbolUnit ) EVT_CHOICE( ID_LIBVIEW_SELECT_BODY_STYLE, SYMBOL_VIEWER_FRAME::onSelectSymbolBodyStyle ) EVT_CHOICE( ID_ON_ZOOM_SELECT, SYMBOL_VIEWER_FRAME::OnSelectZoom ) @@ -106,9 +104,8 @@ END_EVENT_TABLE() SYMBOL_VIEWER_FRAME::SYMBOL_VIEWER_FRAME( KIWAY* aKiway, wxWindow* aParent ) : - SCH_BASE_FRAME( aKiway, aParent, FRAME_SCH_VIEWER, _( "Symbol Library Browser" ), - wxDefaultPosition, wxDefaultSize, KICAD_DEFAULT_DRAWFRAME_STYLE, - LIB_VIEW_FRAME_NAME ), + SCH_BASE_FRAME( aKiway, aParent, FRAME_SCH_VIEWER, _( "Symbol Library Browser" ), wxDefaultPosition, + wxDefaultSize, KICAD_DEFAULT_DRAWFRAME_STYLE, LIB_VIEW_FRAME_NAME ), m_unitChoice( nullptr ), m_bodyStyleChoice( nullptr ), m_libList( nullptr ), @@ -157,13 +154,13 @@ SYMBOL_VIEWER_FRAME::SYMBOL_VIEWER_FRAME( KIWAY* aKiway, wxWindow* aParent ) : wxPanel* libPanel = new wxPanel( this ); wxSizer* libSizer = new wxBoxSizer( wxVERTICAL ); - m_libFilter = new wxSearchCtrl( libPanel, ID_LIBVIEW_LIB_FILTER, wxEmptyString, - wxDefaultPosition, wxDefaultSize, wxTE_PROCESS_ENTER ); + m_libFilter = new wxSearchCtrl( libPanel, ID_LIBVIEW_LIB_FILTER, wxEmptyString, wxDefaultPosition, + wxDefaultSize, wxTE_PROCESS_ENTER ); m_libFilter->SetDescriptiveText( _( "Filter" ) ); libSizer->Add( m_libFilter, 0, wxEXPAND, 5 ); - m_libList = new WX_LISTBOX( libPanel, ID_LIBVIEW_LIB_LIST, wxDefaultPosition, wxDefaultSize, - 0, nullptr, wxLB_HSCROLL | wxNO_BORDER ); + m_libList = new WX_LISTBOX( libPanel, ID_LIBVIEW_LIB_LIST, wxDefaultPosition, wxDefaultSize, 0, nullptr, + wxLB_HSCROLL | wxNO_BORDER ); libSizer->Add( m_libList, 1, wxEXPAND, 5 ); libPanel->SetSizer( libSizer ); @@ -172,13 +169,12 @@ SYMBOL_VIEWER_FRAME::SYMBOL_VIEWER_FRAME( KIWAY* aKiway, wxWindow* aParent ) : wxPanel* symbolPanel = new wxPanel( this ); wxSizer* symbolSizer = new wxBoxSizer( wxVERTICAL ); - m_symbolFilter = new wxSearchCtrl( symbolPanel, ID_LIBVIEW_SYM_FILTER, wxEmptyString, - wxDefaultPosition, wxDefaultSize, wxTE_PROCESS_ENTER ); + m_symbolFilter = new wxSearchCtrl( symbolPanel, ID_LIBVIEW_SYM_FILTER, wxEmptyString, wxDefaultPosition, + wxDefaultSize, wxTE_PROCESS_ENTER ); m_symbolFilter->SetDescriptiveText( _( "Filter" ) ); - m_symbolFilter->SetToolTip( - _( "Filter on symbol name, keywords, description and pin count.\n" - "Search terms are separated by spaces. All search terms must match.\n" - "A term which is a number will also match against the pin count." ) ); + m_symbolFilter->SetToolTip( _( "Filter on symbol name, keywords, description and pin count.\n" + "Search terms are separated by spaces. All search terms must match.\n" + "A term which is a number will also match against the pin count." ) ); symbolSizer->Add( m_symbolFilter, 0, wxEXPAND, 5 ); #ifdef __WXGTK__ @@ -188,8 +184,8 @@ SYMBOL_VIEWER_FRAME::SYMBOL_VIEWER_FRAME( KIWAY* aKiway, wxWindow* aParent ) : m_symbolFilter->SetMinSize( wxSize( -1, GetTextExtent( wxT( "qb" ) ).y + 10 ) ); #endif - m_symbolList = new WX_LISTBOX( symbolPanel, ID_LIBVIEW_SYM_LIST, wxDefaultPosition, - wxDefaultSize, 0, nullptr, wxLB_HSCROLL | wxNO_BORDER ); + m_symbolList = new WX_LISTBOX( symbolPanel, ID_LIBVIEW_SYM_LIST, wxDefaultPosition, wxDefaultSize, 0, nullptr, + wxLB_HSCROLL | wxNO_BORDER ); symbolSizer->Add( m_symbolList, 1, wxEXPAND, 5 ); symbolPanel->SetSizer( symbolSizer ); @@ -275,8 +271,8 @@ void SYMBOL_VIEWER_FRAME::setupTools() { // Create the manager and dispatcher & route draw panel events to the dispatcher m_toolManager = new TOOL_MANAGER; - m_toolManager->SetEnvironment( GetScreen(), GetCanvas()->GetView(), - GetCanvas()->GetViewControls(), config(), this ); + m_toolManager->SetEnvironment( GetScreen(), GetCanvas()->GetView(), GetCanvas()->GetViewControls(), config(), + this ); m_actions = new SCH_ACTIONS(); m_toolDispatcher = new TOOL_DISPATCHER( m_toolManager ); @@ -286,7 +282,7 @@ void SYMBOL_VIEWER_FRAME::setupTools() m_toolManager->RegisterTool( new ZOOM_TOOL ); m_toolManager->RegisterTool( new SCH_INSPECTION_TOOL ); // manage show datasheet m_toolManager->RegisterTool( new SCH_SELECTION_TOOL ); // manage context menu - m_toolManager->RegisterTool( new SYMBOL_EDITOR_CONTROL ); // manage render settings + m_toolManager->RegisterTool( new SYMBOL_EDITOR_CONTROL ); // manage render settings m_toolManager->InitTools(); @@ -310,7 +306,7 @@ void SYMBOL_VIEWER_FRAME::setupUIConditions() #define ENABLE( x ) ACTION_CONDITIONS().Enable( x ) #define CHECK( x ) ACTION_CONDITIONS().Check( x ) - mgr->SetConditions( ACTIONS::toggleGrid, CHECK( cond.GridVisible() ) ); + mgr->SetConditions( ACTIONS::toggleGrid, CHECK( cond.GridVisible() ) ); auto electricalTypesShownCondition = [this]( const SELECTION& aSel ) @@ -486,12 +482,12 @@ bool SYMBOL_VIEWER_FRAME::ReCreateLibList() m_libList->Clear(); - COMMON_SETTINGS* cfg = Pgm().GetCommonSettings(); - PROJECT_FILE& project = Kiway().Prj().GetProjectFile(); + COMMON_SETTINGS* cfg = Pgm().GetCommonSettings(); + PROJECT_FILE& project = Kiway().Prj().GetProjectFile(); SYMBOL_LIBRARY_ADAPTER* adapter = PROJECT_SCH::SymbolLibAdapter( &Prj() ); - std::vector libNicknames = adapter->GetLibraryNames(); - std::vector pinnedMatches; - std::vector otherMatches; + std::vector libNicknames = adapter->GetLibraryNames(); + std::vector pinnedMatches; + std::vector otherMatches; auto doAddLib = [&]( const wxString& aLib ) @@ -537,9 +533,8 @@ bool SYMBOL_VIEWER_FRAME::ReCreateLibList() { for( const auto& [nickname, description] : adapter->GetSubLibraries( aLib ) ) { - wxString suffix = nickname.IsEmpty() - ? wxString( wxT( "" ) ) - : wxString::Format( wxT( " - %s" ), nickname ); + wxString suffix = nickname.IsEmpty() ? wxString( wxT( "" ) ) + : wxString::Format( wxT( " - %s" ), nickname ); wxString name = wxString::Format( wxT( "%s%s" ), aLib, suffix ); doAddLib( name ); @@ -583,8 +578,7 @@ bool SYMBOL_VIEWER_FRAME::ReCreateLibList() m_libList->Append( UnescapeString( name ) ); // Search for a previous selection: - int index = - m_libList->FindString( UnescapeString( m_currentSymbol.GetUniStringLibNickname() ) ); + int index = m_libList->FindString( UnescapeString( m_currentSymbol.GetUniStringLibNickname() ) ); if( index != wxNOT_FOUND ) { @@ -671,8 +665,7 @@ bool SYMBOL_VIEWER_FRAME::ReCreateSymbolList() return true; } - int index = - m_symbolList->FindString( UnescapeString( m_currentSymbol.GetUniStringLibItemName() ) ); + int index = m_symbolList->FindString( UnescapeString( m_currentSymbol.GetUniStringLibItemName() ) ); bool changed = false; if( index == wxNOT_FOUND ) @@ -720,12 +713,13 @@ void SYMBOL_VIEWER_FRAME::ClickOnLibList( wxCommandEvent& event ) } -void SYMBOL_VIEWER_FRAME::SetSelectedLibrary( const wxString& aLibraryName, - const wxString& aSubLibName ) +void SYMBOL_VIEWER_FRAME::SetSelectedLibrary( const wxString& aLibraryName, const wxString& aSubLibName ) { if( m_currentSymbol.GetUniStringLibNickname() == aLibraryName && wxString( m_currentSymbol.GetSubLibraryName().wx_str() ) == aSubLibName ) + { return; + } m_currentSymbol.SetLibNickname( aLibraryName ); m_currentSymbol.SetSubLibraryName( aSubLibName ); @@ -943,8 +937,7 @@ void SYMBOL_VIEWER_FRAME::OnCharHook( wxKeyEvent& aEvent ) } else { - wxCommandEvent dummy; - onSelectPreviousSymbol( dummy ); + SelectPreviousSymbol(); } } else if( aEvent.GetKeyCode() == WXK_DOWN ) @@ -964,8 +957,7 @@ void SYMBOL_VIEWER_FRAME::OnCharHook( wxKeyEvent& aEvent ) } else { - wxCommandEvent dummy; - onSelectNextSymbol( dummy ); + SelectNextSymbol(); } } else if( aEvent.GetKeyCode() == WXK_TAB && m_libFilter->HasFocus() ) @@ -995,7 +987,7 @@ void SYMBOL_VIEWER_FRAME::OnCharHook( wxKeyEvent& aEvent ) } -void SYMBOL_VIEWER_FRAME::onSelectNextSymbol( wxCommandEvent& aEvent ) +void SYMBOL_VIEWER_FRAME::SelectNextSymbol() { wxCommandEvent evt( wxEVT_COMMAND_LISTBOX_SELECTED, ID_LIBVIEW_SYM_LIST ); int ii = m_symbolList->GetSelection(); @@ -1009,7 +1001,7 @@ void SYMBOL_VIEWER_FRAME::onSelectNextSymbol( wxCommandEvent& aEvent ) } -void SYMBOL_VIEWER_FRAME::onSelectPreviousSymbol( wxCommandEvent& aEvent ) +void SYMBOL_VIEWER_FRAME::SelectPreviousSymbol() { wxCommandEvent evt( wxEVT_COMMAND_LISTBOX_SELECTED, ID_LIBVIEW_SYM_LIST ); int ii = m_symbolList->GetSelection(); @@ -1058,8 +1050,7 @@ void SYMBOL_VIEWER_FRAME::DisplayLibInfos() SYMBOL_LIBRARY_ADAPTER* adapter = PROJECT_SCH::SymbolLibAdapter( &Prj() ); LIBRARY_TABLE_ROW* row = adapter->GetRow( libName ).value_or( nullptr ); - wxString title = row - ? LIBRARY_MANAGER::GetFullURI( row, true ) + wxString title = row ? LIBRARY_MANAGER::GetFullURI( row, true ) : _( "[no library selected]" ); title += wxT( " \u2014 " ) + _( "Symbol Library Browser" ); @@ -1089,8 +1080,7 @@ void SYMBOL_VIEWER_FRAME::KiwayMailIn( KIWAY_MAIL_EVENT& mail ) wxCHECK2( symbol, break ); SYMBOL_LIBRARY_ADAPTER* adapter = PROJECT_SCH::SymbolLibAdapter( &Prj() ); - LIBRARY_TABLE_ROW* row = - adapter->GetRow( symbol->GetLibId().GetLibNickname() ).value_or( nullptr ); + LIBRARY_TABLE_ROW* row = adapter->GetRow( symbol->GetLibId().GetLibNickname() ).value_or( nullptr ); if( !row ) return; @@ -1098,8 +1088,7 @@ void SYMBOL_VIEWER_FRAME::KiwayMailIn( KIWAY_MAIL_EVENT& mail ) wxString libfullname = LIBRARY_MANAGER::GetFullURI( row, true ); wxString lib( mail.GetPayload() ); - wxLogTrace( traceLibWatch, "Received refresh symbol request for %s, current symbols is %s", - lib, libfullname ); + wxLogTrace( traceLibWatch, "Received refresh symbol request for %s, current symbols is %s", lib, libfullname ); if( lib == libfullname ) { diff --git a/eeschema/symbol_viewer_frame.h b/eeschema/symbol_viewer_frame.h index 5a2d7b7d9a..53846109ab 100644 --- a/eeschema/symbol_viewer_frame.h +++ b/eeschema/symbol_viewer_frame.h @@ -89,13 +89,14 @@ public: /** * Set the selected library in the library window. */ - void SetSelectedLibrary( const wxString& aLibName, - const wxString& aSubLibName = wxEmptyString ); + void SetSelectedLibrary( const wxString& aLibName, const wxString& aSubLibName = wxEmptyString ); /** * Set the selected symbol. */ void SetSelectedSymbol( const wxString& aSymbolName ); + void SelectNextSymbol(); + void SelectPreviousSymbol(); // Accessors: /** @@ -146,8 +147,6 @@ private: void OnSymFilter( wxCommandEvent& aEvent ); void OnCharHook( wxKeyEvent& aEvent ) override; - void onSelectNextSymbol( wxCommandEvent& aEvent ); - void onSelectPreviousSymbol( wxCommandEvent& aEvent ); void onSelectSymbolUnit( wxCommandEvent& aEvent ); void onSelectSymbolBodyStyle( wxCommandEvent& aEvent ); diff --git a/eeschema/toolbars_symbol_viewer.cpp b/eeschema/toolbars_symbol_viewer.cpp index 51ee2f1668..ac3f0a6450 100644 --- a/eeschema/toolbars_symbol_viewer.cpp +++ b/eeschema/toolbars_symbol_viewer.cpp @@ -46,15 +46,8 @@ std::optional SYMBOL_VIEWER_TOOLBAR_SETTINGS::DefaultTool return std::nullopt; case TOOLBAR_LOC::TOP_MAIN: - /* TODO (ISM): Move these to actions - m_tbTopMain->AddTool( ID_LIBVIEW_PREVIOUS, wxEmptyString, - KiScaledBitmap( BITMAPS::lib_previous, this ), - _( "Display previous symbol" ) ); - - m_tbTopMain->AddTool( ID_LIBVIEW_NEXT, wxEmptyString, - KiScaledBitmap( BITMAPS::lib_next, this ), - _( "Display next symbol" ) ); - */ + config.AppendAction( SCH_ACTIONS::previousSymbol ) + .AppendAction( SCH_ACTIONS::nextSymbol ); config.AppendSeparator() .AppendAction( ACTIONS::zoomRedraw ) diff --git a/eeschema/tools/sch_actions.cpp b/eeschema/tools/sch_actions.cpp index ef9f29a909..c965dcd1f3 100644 --- a/eeschema/tools/sch_actions.cpp +++ b/eeschema/tools/sch_actions.cpp @@ -358,6 +358,18 @@ TOOL_ACTION SCH_ACTIONS::showHiddenFields( TOOL_ACTION_ARGS() .ToolbarState( TOOLBAR_STATE::TOGGLE ) .Icon( BITMAPS::text_sketch ) ); +TOOL_ACTION SCH_ACTIONS::previousSymbol( TOOL_ACTION_ARGS() + .Name( "eeschema.SymbolLibraryControl.previousSymbol" ) + .Scope( AS_GLOBAL ) + .FriendlyName( _( "Display previous symbol" ) ) + .Icon( BITMAPS::lib_previous ) ); + +TOOL_ACTION SCH_ACTIONS::nextSymbol( TOOL_ACTION_ARGS() + .Name( "eeschema.SymbolLibraryControl.nextSymbol" ) + .Scope( AS_GLOBAL ) + .FriendlyName( _( "Display next symbol" ) ) + .Icon( BITMAPS::lib_next ) ); + // SYMBOL_EDITOR_DRAWING_TOOLS // diff --git a/eeschema/tools/sch_actions.h b/eeschema/tools/sch_actions.h index c694c8ed8c..55b1c9f05e 100644 --- a/eeschema/tools/sch_actions.h +++ b/eeschema/tools/sch_actions.h @@ -275,6 +275,8 @@ public: static TOOL_ACTION showPythonConsole; static TOOL_ACTION previousUnit; static TOOL_ACTION nextUnit; + static TOOL_ACTION previousSymbol; + static TOOL_ACTION nextSymbol; // Line modes static TOOL_ACTION lineModeFree; diff --git a/eeschema/tools/symbol_editor_control.cpp b/eeschema/tools/symbol_editor_control.cpp index f63b7cd6b5..db8ad0c939 100644 --- a/eeschema/tools/symbol_editor_control.cpp +++ b/eeschema/tools/symbol_editor_control.cpp @@ -972,6 +972,24 @@ int SYMBOL_EDITOR_CONTROL::ChangeUnit( const TOOL_EVENT& aEvent ) } +int SYMBOL_EDITOR_CONTROL::PreviousSymbol( const TOOL_EVENT& aEvent ) +{ + if( SYMBOL_VIEWER_FRAME* viewerFrame = static_cast( m_toolMgr->GetToolHolder() ) ) + viewerFrame->SelectPreviousSymbol(); + + return 0; +} + + +int SYMBOL_EDITOR_CONTROL::NextSymbol( const TOOL_EVENT& aEvent ) +{ + if( SYMBOL_VIEWER_FRAME* viewerFrame = static_cast( m_toolMgr->GetToolHolder() ) ) + viewerFrame->SelectNextSymbol(); + + return 0; +} + + int SYMBOL_EDITOR_CONTROL::ShowLibraryTable( const TOOL_EVENT& aEvent ) { DIALOG_LIB_FIELDS_TABLE::SCOPE scope = DIALOG_LIB_FIELDS_TABLE::SCOPE_LIBRARY; @@ -1036,4 +1054,7 @@ void SYMBOL_EDITOR_CONTROL::setTransitions() Go( &SYMBOL_EDITOR_CONTROL::ChangeUnit, SCH_ACTIONS::previousUnit.MakeEvent() ); Go( &SYMBOL_EDITOR_CONTROL::ChangeUnit, SCH_ACTIONS::nextUnit.MakeEvent() ); + + Go( &SYMBOL_EDITOR_CONTROL::PreviousSymbol, SCH_ACTIONS::previousSymbol.MakeEvent() ); + Go( &SYMBOL_EDITOR_CONTROL::NextSymbol, SCH_ACTIONS::nextSymbol.MakeEvent() ); } diff --git a/eeschema/tools/symbol_editor_control.h b/eeschema/tools/symbol_editor_control.h index 6a600f0820..8dc815c075 100644 --- a/eeschema/tools/symbol_editor_control.h +++ b/eeschema/tools/symbol_editor_control.h @@ -23,8 +23,7 @@ */ -#ifndef SYMBOL_EDITOR_CONTROL_H -#define SYMBOL_EDITOR_CONTROL_H +#pragma once #include #include @@ -75,6 +74,9 @@ public: int ChangeUnit( const TOOL_EVENT& aEvent ); + int PreviousSymbol( const TOOL_EVENT& aEvent ); + int NextSymbol( const TOOL_EVENT& aEvent ); + int DdAddLibrary( const TOOL_EVENT& aEvent ); int ShowLibraryTable( const TOOL_EVENT& aEvent ); @@ -83,6 +85,3 @@ private: ///< Set up handlers for various events. void setTransitions() override; }; - - -#endif // SYMBOL_EDITOR_CONTROL_H