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

@@ -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 );

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 IsSymbolFromSchematic() && GetScreen() && GetScreen()->IsModify();
-                return GetScreen() && GetScreen()->IsModify();
-            else
-                return m_libMgr->HasModifications();
         };
 
-    auto modifiedDocumentCondition =
+    auto libModifiedCondition =
         [this] ( const SELECTION& sel )
         {
-            LIB_ID libId = getTargetLibId();
+            return m_libMgr->HasModifications();
-            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 ) );
         };
 
     mgr->SetConditions( ACTIONS::saveAll,
-                        ENABLE( libMgrModifiedCond ) );
+                        ENABLE( schematicModifiedCond || libModifiedCondition ) );
     mgr->SetConditions( ACTIONS::save,
-                        ENABLE( libMgrModifiedCond ||
+                        ENABLE( schematicModifiedCond || libModifiedCondition ) );
-                                ( haveSymbolCond && modifiedDocumentCondition ) ) );
+    mgr->SetConditions( EE_ACTIONS::saveInSchematic,
-    mgr->SetConditions( EE_ACTIONS::saveInSchematic, ENABLE( libMgrModifiedCond ) );
+                        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();

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.

eeschema/symbol_editor/symbol_editor.cpp

@@ -25,6 +25,7 @@
 
 #include <pgm_base.h>
 #include <confirm.h>
+#include <kiway.h>
 #include <widgets/infobar.h>
 #include <tools/ee_actions.h>
 #include <tools/symbol_editor_drawing_tools.h>
@@ -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();
+    if( getTargetPart() == m_my_part )
-    const wxString& libName = libId.GetLibNickname();
+    {
-    const wxString& partName = libId.GetLibItemName();
+        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 );
+        LIB_ID          libId = getTargetLibId();
-        wxString msg2 = _( "You must save to a different location." );
+        const wxString& libName = libId.GetLibNickname();
+        const wxString& partName = libId.GetLibItemName();
 
-        if( OKOrCancelDialog( this, _( "Warning" ), msg, msg2 ) == wxID_OK )
+        if( m_libMgr->IsLibraryReadOnly( libName ) )
-            saveLibrary( libName, true );
+        {
-    }
+            wxString msg = wxString::Format( _( "Symbol library '%s' is not writeable." ), libName );
-    else if( partName.IsEmpty() )
+            wxString msg2 = _( "You must save to a different location." );
-    {
+
-        saveLibrary( libName, false );
+            if( OKOrCancelDialog( this, _( "Warning" ), msg, msg2 ) == wxID_OK )
-    }
+                saveLibrary( libName, true );
-    else
+        }
-    {
+        else
-        // Save a single library.
+        {
-        if( m_libMgr->FlushPart( partName, libName ) )
+            saveLibrary( libName, false );
-            m_libMgr->ClearPartModified( partName, libName );
+        }
     }
 
     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();
+    LIB_PART* part = getTargetPart();
-    wxString old_name = old_lib_id.GetLibItemName();
-    wxString old_lib = old_lib_id.GetLibNickname();
-    LIB_PART* part = m_libMgr->GetBufferedPart( old_name, old_lib );
 
     if( part )
     {
-        SYMBOL_LIB_TABLE* tbl = Prj().SchSymbolLibTable();
+        LIB_ID   old_lib_id = part->GetLibId();
-        wxArrayString headers;
+        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"
+            DisplayError( this, _( "Derived symbols must be saved in the same library as their "
-                                   "that the parent symbol exists." ) );
+                                   "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\"" ),
+            wxString msg = wxString::Format( _( "Symbol '%s' already exists in library '%s'" ),
-                                             new_name, new_lib );
+                                             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();
 }
 
 

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();
+    }
+}

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;
 };