From 1648dd6f0ef311d2c1d8ff3eab58fe3eb478c292 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 25 Dec 2020 11:55:45 +0000 Subject: [PATCH] Fix various issues with save & editing symbols from schematic. 1) Zero out selection in tree when canvas captures focus so that save/save as actions will go to canvas, not tree item. 2) Make save do a save to schematic when the canvas symbol is from the schematic. 3) Copy fields when doing a save to schematic (setting the lib part doesn't update them on its own). 4) Remove no-longer-necessary calls to update schematic after saving symbols (the schematic symbols are no longer hot-linked). Fixes https://gitlab.com/kicad/code/kicad/issues/6763 --- eeschema/sch_edit_frame.cpp | 2 + eeschema/symbol_editor/symbol_edit_frame.cpp | 37 +++------ eeschema/symbol_editor/symbol_edit_frame.h | 2 - eeschema/symbol_editor/symbol_editor.cpp | 82 +++++++++++--------- eeschema/widgets/symbol_tree_pane.cpp | 16 +++- eeschema/widgets/symbol_tree_pane.h | 3 +- 6 files changed, 74 insertions(+), 68 deletions(-) diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 322139f8d7..e03b5c2f4f 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -1627,6 +1627,8 @@ void SCH_EDIT_FRAME::UpdateSymbolFromEditor( const LIB_PART& aSymbol ) } symbol->SetLibSymbol( aSymbol.Flatten().release() ); + symbol->UpdateFields( true, true ); + currentScreen->Append( symbol ); selectionTool->SelectHighlightItem( symbol ); GetCanvas()->GetView()->Update( symbol ); diff --git a/eeschema/symbol_editor/symbol_edit_frame.cpp b/eeschema/symbol_editor/symbol_edit_frame.cpp index 31223e2904..991630409b 100644 --- a/eeschema/symbol_editor/symbol_edit_frame.cpp +++ b/eeschema/symbol_editor/symbol_edit_frame.cpp @@ -340,40 +340,30 @@ void SYMBOL_EDIT_FRAME::setupUIConditions() return m_my_part && m_my_part->IsRoot() && !IsSymbolFromLegacyLibrary(); }; - auto libMgrModifiedCond = + auto schematicModifiedCond = [this] ( const SELECTION& ) { - if( IsSymbolFromSchematic() ) - return GetScreen() && GetScreen()->IsModify(); - else - return m_libMgr->HasModifications(); + return IsSymbolFromSchematic() && GetScreen() && GetScreen()->IsModify(); }; - auto modifiedDocumentCondition = + auto libModifiedCondition = [this] ( const SELECTION& sel ) { - LIB_ID libId = getTargetLibId(); - const wxString& libName = libId.GetLibNickname(); - const wxString& partName = libId.GetLibItemName(); - - if( partName.IsEmpty() ) - return ( m_libMgr->IsLibraryModified( libName ) ); - else - return ( m_libMgr->IsPartModified( partName, libName ) ); + return m_libMgr->HasModifications(); }; mgr->SetConditions( ACTIONS::saveAll, - ENABLE( libMgrModifiedCond ) ); + ENABLE( schematicModifiedCond || libModifiedCondition ) ); mgr->SetConditions( ACTIONS::save, - ENABLE( libMgrModifiedCond || - ( haveSymbolCond && modifiedDocumentCondition ) ) ); - mgr->SetConditions( EE_ACTIONS::saveInSchematic, ENABLE( libMgrModifiedCond ) ); + ENABLE( schematicModifiedCond || libModifiedCondition ) ); + mgr->SetConditions( EE_ACTIONS::saveInSchematic, + ENABLE( schematicModifiedCond ) ); mgr->SetConditions( ACTIONS::undo, ENABLE( haveSymbolCond && cond.UndoAvailable() ) ); mgr->SetConditions( ACTIONS::redo, ENABLE( haveSymbolCond && cond.RedoAvailable() ) ); mgr->SetConditions( ACTIONS::revert, - ENABLE( haveSymbolCond && modifiedDocumentCondition ) ); + ENABLE( haveSymbolCond && libModifiedCondition ) ); mgr->SetConditions( ACTIONS::toggleGrid, CHECK( cond.GridVisible() ) ); @@ -810,15 +800,6 @@ bool SYMBOL_EDIT_FRAME::SynchronizePins() } -void SYMBOL_EDIT_FRAME::refreshSchematic() -{ - // There may be no parent window so use KIWAY message to refresh the schematic editor - // in case any symbols have changed. - std::string dummyPayload; - Kiway().ExpressMail( FRAME_SCH, MAIL_SCH_REFRESH, dummyPayload, this ); -} - - bool SYMBOL_EDIT_FRAME::AddLibraryFile( bool aCreateNew ) { wxFileName fn = m_libMgr->GetUniqueLibraryName(); diff --git a/eeschema/symbol_editor/symbol_edit_frame.h b/eeschema/symbol_editor/symbol_edit_frame.h index 73a33b30a0..667a45b5aa 100644 --- a/eeschema/symbol_editor/symbol_edit_frame.h +++ b/eeschema/symbol_editor/symbol_edit_frame.h @@ -395,8 +395,6 @@ private: */ void SaveOneSymbol(); - void refreshSchematic(); - public: /** * Selects the currently active library and loads the symbol from \a aLibId. diff --git a/eeschema/symbol_editor/symbol_editor.cpp b/eeschema/symbol_editor/symbol_editor.cpp index 3b0e17cf9b..fb893ae391 100644 --- a/eeschema/symbol_editor/symbol_editor.cpp +++ b/eeschema/symbol_editor/symbol_editor.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -435,7 +436,6 @@ void SYMBOL_EDIT_FRAME::SaveAll() { saveAllLibraries( false ); m_treePane->GetLibTree()->RefreshLibTree(); - refreshSchematic(); } @@ -574,31 +574,44 @@ void SYMBOL_EDIT_FRAME::CreateNewPart() void SYMBOL_EDIT_FRAME::Save() { - LIB_ID libId = getTargetLibId(); - const wxString& libName = libId.GetLibNickname(); - const wxString& partName = libId.GetLibItemName(); + if( getTargetPart() == m_my_part ) + { + if( IsSymbolFromSchematic() ) + { + SCH_EDIT_FRAME* schframe = (SCH_EDIT_FRAME*) Kiway().Player( FRAME_SCH, false ); - if( !libName.IsEmpty() && m_libMgr->IsLibraryReadOnly( libName ) ) + if( schframe ) + { + schframe->UpdateSymbolFromEditor( *m_my_part ); + GetScreen()->ClrModify(); + } + } + else + { + saveCurrentPart(); + } + } + else if( !getTargetLibId().GetLibNickname().empty() ) { - wxString msg = wxString::Format( _( "Symbol library '%s' is not writeable." ), libName ); - wxString msg2 = _( "You must save to a different location." ); + LIB_ID libId = getTargetLibId(); + const wxString& libName = libId.GetLibNickname(); + const wxString& partName = libId.GetLibItemName(); - if( OKOrCancelDialog( this, _( "Warning" ), msg, msg2 ) == wxID_OK ) - saveLibrary( libName, true ); - } - else if( partName.IsEmpty() ) - { - saveLibrary( libName, false ); - } - else - { - // Save a single library. - if( m_libMgr->FlushPart( partName, libName ) ) - m_libMgr->ClearPartModified( partName, libName ); + if( m_libMgr->IsLibraryReadOnly( libName ) ) + { + wxString msg = wxString::Format( _( "Symbol library '%s' is not writeable." ), libName ); + wxString msg2 = _( "You must save to a different location." ); + + if( OKOrCancelDialog( this, _( "Warning" ), msg, msg2 ) == wxID_OK ) + saveLibrary( libName, true ); + } + else + { + saveLibrary( libName, false ); + } } m_treePane->GetLibTree()->RefreshLibTree(); - refreshSchematic(); } @@ -614,28 +627,28 @@ void SYMBOL_EDIT_FRAME::SaveAs() savePartAs(); m_treePane->GetLibTree()->RefreshLibTree(); - refreshSchematic(); } void SYMBOL_EDIT_FRAME::savePartAs() { - LIB_ID old_lib_id = getTargetLibId(); - wxString old_name = old_lib_id.GetLibItemName(); - wxString old_lib = old_lib_id.GetLibNickname(); - LIB_PART* part = m_libMgr->GetBufferedPart( old_name, old_lib ); + LIB_PART* part = getTargetPart(); if( part ) { - SYMBOL_LIB_TABLE* tbl = Prj().SchSymbolLibTable(); - wxArrayString headers; + LIB_ID old_lib_id = part->GetLibId(); + wxString old_name = old_lib_id.GetLibItemName(); + wxString old_lib = old_lib_id.GetLibNickname(); + + SYMBOL_LIB_TABLE* tbl = Prj().SchSymbolLibTable(); + wxArrayString headers; std::vector< wxArrayString > itemsToDisplay; - std::vector< wxString > libNicknames = tbl->GetLogicalLibs(); + std::vector< wxString > libNicknames = tbl->GetLogicalLibs(); headers.Add( _( "Nickname" ) ); headers.Add( _( "Description" ) ); - for( const auto& name : libNicknames ) + for( const wxString& name : libNicknames ) { wxArrayString item; item.Add( name ); @@ -685,8 +698,8 @@ void SYMBOL_EDIT_FRAME::savePartAs() // solution to ensure derived parts do not get orphaned. if( part->IsAlias() && new_lib != old_lib ) { - DisplayError( this, _( "Derived symbols must be save in the same library\n" - "that the parent symbol exists." ) ); + DisplayError( this, _( "Derived symbols must be saved in the same library as their " + "parent symbol." ) ); return; } @@ -704,8 +717,9 @@ void SYMBOL_EDIT_FRAME::savePartAs() // Test if there is a component with this name already. if( m_libMgr->PartExists( new_name, new_lib ) ) { - wxString msg = wxString::Format( _( "Symbol \"%s\" already exists in library \"%s\"" ), - new_name, new_lib ); + wxString msg = wxString::Format( _( "Symbol '%s' already exists in library '%s'" ), + new_name, + new_lib ); DisplayError( this, msg ); return; } @@ -797,7 +811,6 @@ void SYMBOL_EDIT_FRAME::DeletePartFromLibrary() m_libMgr->RemovePart( libId.GetLibItemName(), libId.GetLibNickname() ); m_treePane->GetLibTree()->RefreshLibTree(); - refreshSchematic(); } @@ -968,7 +981,6 @@ void SYMBOL_EDIT_FRAME::Revert( bool aConfirm ) LoadPart( curr_partName, libName, unit ); m_treePane->Refresh(); - refreshSchematic(); } diff --git a/eeschema/widgets/symbol_tree_pane.cpp b/eeschema/widgets/symbol_tree_pane.cpp index 31487dba2e..79a3e44792 100644 --- a/eeschema/widgets/symbol_tree_pane.cpp +++ b/eeschema/widgets/symbol_tree_pane.cpp @@ -32,7 +32,7 @@ SYMBOL_TREE_PANE::SYMBOL_TREE_PANE( SYMBOL_EDIT_FRAME* aParent, SYMBOL_LIBRARY_MANAGER* aLibMgr ) : wxPanel( aParent ), - m_libEditFrame( aParent ), + m_symbolEditFrame( aParent ), m_tree( nullptr ), m_libMgr( aLibMgr ) { @@ -48,6 +48,7 @@ SYMBOL_TREE_PANE::SYMBOL_TREE_PANE( SYMBOL_EDIT_FRAME* aParent, SYMBOL_LIBRARY_M // Event handlers Bind( COMPONENT_SELECTED, &SYMBOL_TREE_PANE::onComponentSelected, this ); + m_tree->Bind( wxEVT_UPDATE_UI, &SYMBOL_TREE_PANE::onUpdateUI, this ); } @@ -59,7 +60,7 @@ SYMBOL_TREE_PANE::~SYMBOL_TREE_PANE() void SYMBOL_TREE_PANE::onComponentSelected( wxCommandEvent& aEvent ) { - m_libEditFrame->GetToolManager()->RunAction( EE_ACTIONS::editSymbol, true ); + m_symbolEditFrame->GetToolManager()->RunAction( EE_ACTIONS::editSymbol, true ); // Make sure current-part highlighting doesn't get lost in selection highlighting m_tree->Unselect(); @@ -67,3 +68,14 @@ void SYMBOL_TREE_PANE::onComponentSelected( wxCommandEvent& aEvent ) // Turn off any previous current-part highlighting m_tree->RefreshLibTree(); } + + +void SYMBOL_TREE_PANE::onUpdateUI( wxUpdateUIEvent& aEvent ) +{ + if( m_symbolEditFrame->GetCanvas()->HasFocus() ) + { + // Don't allow a selected item in the tree when the canvas has focus: it's too easy + // to confuse the selected-highlighting with the being-edited-on-canvas-highlighting. + m_tree->Unselect(); + } +} diff --git a/eeschema/widgets/symbol_tree_pane.h b/eeschema/widgets/symbol_tree_pane.h index d66106e027..52495a55bd 100644 --- a/eeschema/widgets/symbol_tree_pane.h +++ b/eeschema/widgets/symbol_tree_pane.h @@ -50,8 +50,9 @@ public: protected: void onComponentSelected( wxCommandEvent& aEvent ); + void onUpdateUI( wxUpdateUIEvent& aEvent ); - SYMBOL_EDIT_FRAME* m_libEditFrame; + SYMBOL_EDIT_FRAME* m_symbolEditFrame; LIB_TREE* m_tree; ///< component search tree widget SYMBOL_LIBRARY_MANAGER* m_libMgr; };