haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

LibEdit: fix library tree refresh issue when library is removed. 

A bug in LIB_MANAGER::LibraryExists() prevented a library removed from
the symbol library table from being removed from the tree view.  Add the
proper check to SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Sync().

This exposed another issue with synchronization between the library
table editor and the tree view which could lead to orphaned library
modifications and/or a segfault when the currently selected library was
removed from the symbol library table.  Give the user a chance to save
or revert any changes before allowing changes to the symbol library
table.

Fixes lp:1821691

https://bugs.launchpad.net/kicad/+bug/1821691

(cherry picked from commit 028973d182)
This commit is contained in:
Wayne Stambaugh 2019-06-13 07:44:12 -04:00
parent 9233ae97d0
commit 1ed411e431
8 changed files with 245 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
eeschema/dialogs/panel_sym_lib_table.cpp
View File

@ -703,12 +703,58 @@ size_t PANEL_SYM_LIB_TABLE::m_pageNdx = 0;
void InvokeSchEditSymbolLibTable( KIWAY* aKiway, wxWindow *aParent )
{
SCH_EDIT_FRAME* schEditor = (SCH_EDIT_FRAME*) aKiway->Player( FRAME_SCH, false );
LIB_EDIT_FRAME* libEditor = (LIB_EDIT_FRAME*) aKiway->Player( FRAME_SCH_LIB_EDITOR, false );
LIB_VIEW_FRAME* libViewer = (LIB_VIEW_FRAME*) aKiway->Player( FRAME_SCH_VIEWER, false );
SYMBOL_LIB_TABLE* globalTable = &SYMBOL_LIB_TABLE::GetGlobalLibTable();
wxString globalTablePath = SYMBOL_LIB_TABLE::GetGlobalTableFileName();
SYMBOL_LIB_TABLE* projectTable = aKiway->Prj().SchSymbolLibTable();
wxString projectPath = aKiway->Prj().GetProjectPath();
wxFileName projectTableFn( projectPath, SYMBOL_LIB_TABLE::GetSymbolLibTableFileName() );
wxString msg;
wxString currentLib;
if( libEditor )
{
currentLib = libEditor->GetCurLib();
// This prevents an ugly crash on OSX (https://bugs.launchpad.net/kicad/+bug/1765286)
libEditor->FreezeSearchTree();
// Check the symbol library editor for modifications to give the user a chance to save
// or revert changes before allowing changes to the library table.
if( libEditor->HasLibModifications() )
{
wxMessageDialog saveDlg( aParent,
_( "Modifications have been made to one or more symbol "
"libraries. Changes must be saved or discared before "
"the symbol library table can be modified." ),
_( "Warning" ),
wxYES_NO | wxCANCEL | wxYES_DEFAULT | wxCENTER );
saveDlg.SetYesNoCancelLabels( wxMessageDialog::ButtonLabel( _( "Save Changes" ) ),
wxMessageDialog::ButtonLabel(_( "Discard Changes" ) ),
wxMessageDialog::ButtonLabel( _( "Cancel" ) ) );
int resp = saveDlg.ShowModal();
if( resp == wxID_CANCEL )
{
libEditor->ThawSearchTree();
return;
}
if( resp == wxID_YES )
{
wxCommandEvent dummy;
libEditor->OnSaveAll( dummy );
}
else
{
libEditor->RevertAll();
}
}
}
DIALOG_EDIT_LIBRARY_TABLES dlg( aParent, _( "Symbol Libraries" ) );
@ -717,7 +763,12 @@ void InvokeSchEditSymbolLibTable( KIWAY* aKiway, wxWindow *aParent )
aKiway->Prj().GetProjectPath() ) );
if( dlg.ShowModal() == wxID_CANCEL )
{
if( libEditor )
libEditor->ThawSearchTree();
return;
}
if( dlg.m_GlobalTableChanged )
{
@ -750,20 +801,25 @@ void InvokeSchEditSymbolLibTable( KIWAY* aKiway, wxWindow *aParent )
schematic.UpdateSymbolLinks( true ); // Update all symbol library links for all sheets.
SCH_EDIT_FRAME* schEditor = (SCH_EDIT_FRAME*) aKiway->Player( FRAME_SCH, false );
if( schEditor )
schEditor->SyncView();
auto editor = (LIB_EDIT_FRAME*) aKiway->Player( FRAME_SCH_LIB_EDITOR, false );
if( libEditor )
{
// Check if the currently selected symbol library been removed or disabled.
if( !currentLib.empty()
&& !projectTable->HasLibrary( currentLib, true ) )
{
libEditor->SetCurLib( wxEmptyString );
libEditor->emptyScreen();
}
if( editor )
editor->SyncLibraries( true );
libEditor->SyncLibraries( true );
libEditor->ThawSearchTree();
}
LIB_VIEW_FRAME* viewer = (LIB_VIEW_FRAME*) aKiway->Player( FRAME_SCH_VIEWER, false );
if( viewer )
viewer->ReCreateListLib();
if( libViewer )
libViewer->ReCreateListLib();
}

21
eeschema/libedit/lib_edit_frame.cpp
View File

@ -410,6 +410,18 @@ void LIB_EDIT_FRAME::OnToggleSearchTree( wxCommandEvent& event )
}
void LIB_EDIT_FRAME::FreezeSearchTree()
{
m_libMgr->GetAdapter()->Freeze();
}
void LIB_EDIT_FRAME::ThawSearchTree()
{
m_libMgr->GetAdapter()->Thaw();
}
void LIB_EDIT_FRAME::OnEditSymbolLibTable( wxCommandEvent& aEvent )
{
m_libMgr->GetAdapter()->Freeze();
@ -1727,6 +1739,7 @@ bool LIB_EDIT_FRAME::isCurrentPart( const LIB_ID& aLibId ) const
void LIB_EDIT_FRAME::emptyScreen()
{
m_treePane->GetLibTree()->Unselect();
SetCurLib( wxEmptyString );
SetCurPart( nullptr );
m_lastDrawItem = nullptr;
@ -1868,3 +1881,11 @@ void LIB_EDIT_FRAME::OnSwitchCanvas( wxCommandEvent& aEvent )
// Set options specific to symbol editor (axies are always enabled):
GetGalCanvas()->GetGAL()->SetAxesEnabled( true );
}
bool LIB_EDIT_FRAME::HasLibModifications() const
{
wxCHECK( m_libMgr, false );
return m_libMgr->HasModifications();
}

26
eeschema/libedit/lib_edit_frame.h
View File

@ -159,6 +159,16 @@ public:
*/
void OnSwitchCanvas( wxCommandEvent& aEvent ) override;
/**
* Check if any pending libraries have been modified.
*
* This only checks for modified libraries. If a new symbol was created and
* modified and no libraries have been modified, the return value will be false.
*
* @return True if there are any pending library modifications.
*/
bool HasLibModifications() const;
/** The nickname of the current library being edited and empty string if none. */
wxString GetCurLib() const;
@ -293,12 +303,18 @@ public:
/**
* Reverts unsaved changes in a part, restoring to the last saved state.
*/
void OnRevert( wxCommandEvent& aEvent );
void OnRevert( wxCommandEvent& aEvent ) { Revert(); }
/**
* Removes a part from the working copy of a library.
*/
void OnRemovePart( wxCommandEvent& aEvent );
void Revert( bool aConfirm = true );
void RevertAll();
void DeletePartFromLibrary();
void CopyPartToClipboard();
void OnDuplicatePart( wxCommandEvent& aEvent );
void OnCopyCutPart( wxCommandEvent& aEvent );
@ -312,6 +328,8 @@ public:
void OnShowElectricalType( wxCommandEvent& event );
void OnToggleSearchTree( wxCommandEvent& event );
void FreezeSearchTree();
void ThawSearchTree();
void OnEditSymbolLibTable( wxCommandEvent& aEvent ) override;
@ -757,6 +775,9 @@ public:
void KiwayMailIn( KIWAY_EXPRESS& mail ) override;
///> Restores the empty editor screen, without any part or library selected.
void emptyScreen();
private:
///> Helper screen used when no part is loaded
SCH_SCREEN* m_dummyScreen;
@ -800,9 +821,6 @@ private:
///> Returns true if \a aLibId is an alias for the editor screen part.
bool isCurrentPart( const LIB_ID& aLibId ) const;
///> Restores the empty editor screen, without any part or library selected.
void emptyScreen();
///> Renames LIB_PART aliases to avoid conflicts before adding a component to a library
void fixDuplicateAliases( LIB_PART* aPart, const wxString& aLibrary );

71
eeschema/libedit/lib_manager.cpp
View File

@ -2,6 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2017 CERN
* Copyright (C) 2019 KiCad Developers, see AUTHORS.txt for contributors.
* @author Maciej Suminski <maciej.suminski@cern.ch>
*
* This program is free software; you can redistribute it and/or
@ -36,15 +37,17 @@
#include <list>
LIB_MANAGER::LIB_MANAGER( LIB_EDIT_FRAME& aFrame )
: m_frame( aFrame ), m_syncHash( 0 )
LIB_MANAGER::LIB_MANAGER( LIB_EDIT_FRAME& aFrame ) :
m_frame( aFrame ),
m_syncHash( 0 )
{
m_adapter = SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Create( this );
m_adapter = SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Create( &aFrame, this );
m_adapter->ShowUnits( false );
}
void LIB_MANAGER::Sync( bool aForce, std::function<void(int, int, const wxString&)> aProgressCallback )
void LIB_MANAGER::Sync( bool aForce,
std::function<void( int, int, const wxString& )> aProgressCallback )
{
int libTableHash = symTable()->GetModifyHash();
@ -56,6 +59,18 @@ void LIB_MANAGER::Sync( bool aForce, std::function<void(int, int, const wxString
}
bool LIB_MANAGER::HasModifications() const
{
for( auto lib : m_libs )
{
if( lib.second.IsModified() )
return true;
}
return false;
}
int LIB_MANAGER::GetHash() const
{
int hash = symTable()->GetModifyHash();
@ -506,6 +521,32 @@ bool LIB_MANAGER::RevertLibrary( const wxString& aLibrary )
}
bool LIB_MANAGER::RevertAll()
{
bool retv = true;
// Nothing to revert.
if( GetHash() == 0 )
return true;
for( auto lib : m_libs )
{
if( !lib.second.IsModified() )
continue;
for( auto buffer : lib.second.GetBuffers() )
{
if( !buffer->IsModified() )
continue;
RevertPart( lib.first, buffer->GetOriginal()->GetName() );
}
}
return retv;
}
bool LIB_MANAGER::RemovePart( const wxString& aAlias, const wxString& aLibrary )
{
LIB_BUFFER& libBuf = getLibraryBuffer( aLibrary );
@ -560,7 +601,8 @@ bool LIB_MANAGER::PartExists( const wxString& aAlias, const wxString& aLibrary )
try
{
alias = symTable()->LoadSymbol( aLibrary, aAlias );
} catch( IO_ERROR& )
}
catch( IO_ERROR& )
{
// checking if certain symbol exists, so its absence is perfectly fine
}
@ -580,6 +622,7 @@ bool LIB_MANAGER::LibraryExists( const wxString& aLibrary, bool aCheckEnabled )
return symTable()->HasLibrary( aLibrary, aCheckEnabled );
}
wxString LIB_MANAGER::GetUniqueLibraryName() const
{
wxString name = "New_Library";
@ -686,8 +729,8 @@ std::set<LIB_PART*> LIB_MANAGER::getOriginalParts( const wxString& aLibrary )
}
catch( const IO_ERROR& e )
{
DisplayErrorMessage( &m_frame, wxString::Format( _( "Cannot enumerate library \"%s\"" ),
aLibrary ),
DisplayErrorMessage( &m_frame,
wxString::Format( _( "Cannot enumerate library \"%s\"" ), aLibrary ),
e.What() );
}
@ -713,8 +756,9 @@ LIB_MANAGER::LIB_BUFFER& LIB_MANAGER::getLibraryBuffer( const wxString& aLibrary
}
LIB_MANAGER::PART_BUFFER::PART_BUFFER( LIB_PART* aPart, std::unique_ptr<SCH_SCREEN> aScreen )
: m_screen( std::move( aScreen ) ), m_part( aPart )
LIB_MANAGER::PART_BUFFER::PART_BUFFER( LIB_PART* aPart, std::unique_ptr<SCH_SCREEN> aScreen ) :
m_screen( std::move( aScreen ) ),
m_part( aPart )
{
m_original = new LIB_PART( *aPart );
}
@ -795,7 +839,8 @@ bool LIB_MANAGER::LIB_BUFFER::CreateBuffer( LIB_PART* aCopy, SCH_SCREEN* aScreen
}
bool LIB_MANAGER::LIB_BUFFER::UpdateBuffer( LIB_MANAGER::PART_BUFFER::PTR aPartBuf, LIB_PART* aCopy )
bool LIB_MANAGER::LIB_BUFFER::UpdateBuffer( LIB_MANAGER::PART_BUFFER::PTR aPartBuf,
LIB_PART* aCopy )
{
bool ret = true;
@ -826,7 +871,8 @@ bool LIB_MANAGER::LIB_BUFFER::SaveBuffer( LIB_MANAGER::PART_BUFFER::PTR aPartBuf
wxCHECK( aPartBuf, false );
LIB_PART* part = aPartBuf->GetPart();
wxCHECK( part, false );
wxCHECK( aLibTable->SaveSymbol( m_libName, new LIB_PART( *part ) ) == SYMBOL_LIB_TABLE::SAVE_OK, false );
wxCHECK( aLibTable->SaveSymbol( m_libName,
new LIB_PART( *part ) ) == SYMBOL_LIB_TABLE::SAVE_OK, false );
aPartBuf->SetOriginal( new LIB_PART( *part ) );
++m_hash;
@ -862,7 +908,8 @@ bool LIB_MANAGER::LIB_BUFFER::addAliases( PART_BUFFER::PTR aPartBuf )
for( unsigned int i = 0; i < part->GetAliasCount(); ++i )
{
bool newAlias;
std::tie( std::ignore, newAlias ) = m_aliases.emplace( part->GetAlias( i )->GetName(), aPartBuf );
std::tie( std::ignore, newAlias ) = m_aliases.emplace( part->GetAlias( i )->GetName(),
aPartBuf );
if( !newAlias ) // Overwrite check
{

19
eeschema/libedit/lib_manager.h
View File

@ -2,6 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2017 CERN
* Copyright (C) 2019 KiCad Developers, see AUTHORS.txt for contributors.
* @author Maciej Suminski <maciej.suminski@cern.ch>
*
* This program is free software; you can redistribute it and/or
@ -59,8 +60,10 @@ public:
int GetHash() const;
bool HasModifications() const;
/**
* Retruns a library hash value to determine if it has changed.
* Returns a library hash value to determine if it has changed.
*
* For buffered libraries, it returns a number corresponding to the number
* of modifications. For original libraries, hash is computed basing on the
@ -217,6 +220,13 @@ public:
*/
bool RevertLibrary( const wxString& aLibrary );
/**
* Revert all pending changes.
*
* @return True if all changes successfully reverted.
*/
bool RevertAll();
/**
* Returns a library name that is not currently in use.
* Used for generating names for new libraries.
@ -385,7 +395,7 @@ private:
///> to disk as well, depending on the row properties.
bool SaveBuffer( PART_BUFFER::PTR aPartBuf, SYMBOL_LIB_TABLE* aLibTable );
///> Saves stored modificatiosn using a plugin. aBuffer decides whether the changes
///> Saves stored modifications using a plugin. aBuffer decides whether the changes
///> should be cached or stored directly to the disk (for SCH_LEGACY_PLUGIN).
bool SaveBuffer( PART_BUFFER::PTR aPartBuf, SCH_PLUGIN* aPlugin, bool aBuffer );
@ -449,7 +459,10 @@ private:
wxString m_currentPart;
SYMBOL_TREE_SYNCHRONIZING_ADAPTER::PTR m_adapter;
SYMBOL_TREE_SYNCHRONIZING_ADAPTER* getAdapter() { return static_cast<SYMBOL_TREE_SYNCHRONIZING_ADAPTER*>( m_adapter.get() ); }
SYMBOL_TREE_SYNCHRONIZING_ADAPTER* getAdapter()
{
return static_cast<SYMBOL_TREE_SYNCHRONIZING_ADAPTER*>( m_adapter.get() );
}
};
#endif /* LIB_MANAGER_H */

15
eeschema/libedit/libedit.cpp
View File

@ -152,7 +152,7 @@ bool LIB_EDIT_FRAME::LoadComponentFromCurrentLib( const wxString& aAliasName, in
}
/**
* Synchronise screen settings from a current screen into another screen.
* Synchronize screen settings from a current screen into another screen.
*
* This can be used, for example, when loading a new screen into a frame,
* but you want the new screen to inherit some settings (e.g. grids) from the
@ -589,7 +589,7 @@ void LIB_EDIT_FRAME::fixDuplicateAliases( LIB_PART* aPart, const wxString& aLibr
}
void LIB_EDIT_FRAME::OnRevert( wxCommandEvent& aEvent )
void LIB_EDIT_FRAME::Revert( bool aConfirm )
{
LIB_ID libId = getTargetLibId();
const wxString& libName = libId.GetLibNickname();
@ -598,7 +598,7 @@ void LIB_EDIT_FRAME::OnRevert( wxCommandEvent& aEvent )
wxString msg = wxString::Format( _( "Revert \"%s\" to last version saved?" ),
partName.IsEmpty() ? libName : partName );
if( !ConfirmRevertDialog( this, msg ) )
if( aConfirm && !ConfirmRevertDialog( this, msg ) )
return;
bool reload_currentPart = false;
@ -645,6 +645,15 @@ void LIB_EDIT_FRAME::OnRevert( wxCommandEvent& aEvent )
}
void LIB_EDIT_FRAME::RevertAll()
{
wxCHECK_RET( m_libMgr, "Library manager object not created." );
Revert( false );
m_libMgr->RevertAll();
}
void LIB_EDIT_FRAME::loadPart( const wxString& aAlias, const wxString& aLibrary, int aUnit )
{
wxCHECK( m_libMgr->PartExists( aAlias, aLibrary ), /* void */ );

30
eeschema/symbol_tree_synchronizing_adapter.cpp
View File

@ -2,6 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2017 CERN
* Copyright (C) 2019 KiCad Developers, see AUTHORS.txt for contributors.
* @author Maciej Suminski <maciej.suminski@cern.ch>
*
* This program is free software; you can redistribute it and/or
@ -23,19 +24,23 @@
*/
#include <symbol_tree_synchronizing_adapter.h>
#include <lib_edit_frame.h>
#include <lib_manager.h>
#include <symbol_lib_table.h>
#include <class_libentry.h>
LIB_TREE_MODEL_ADAPTER::PTR SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Create( LIB_MANAGER* aLibMgr )
LIB_TREE_MODEL_ADAPTER::PTR SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Create( LIB_EDIT_FRAME* aParent,
LIB_MANAGER* aLibMgr )
{
return PTR( new SYMBOL_TREE_SYNCHRONIZING_ADAPTER( aLibMgr ) );
return PTR( new SYMBOL_TREE_SYNCHRONIZING_ADAPTER( aParent, aLibMgr ) );
}
SYMBOL_TREE_SYNCHRONIZING_ADAPTER::SYMBOL_TREE_SYNCHRONIZING_ADAPTER( LIB_MANAGER* aLibMgr )
: m_libMgr( aLibMgr ),
SYMBOL_TREE_SYNCHRONIZING_ADAPTER::SYMBOL_TREE_SYNCHRONIZING_ADAPTER( LIB_EDIT_FRAME* aParent,
LIB_MANAGER* aLibMgr ) :
m_frame( aParent ),
m_libMgr( aLibMgr ),
m_lastSyncHash( -1 )
{
}
@ -50,7 +55,8 @@ bool SYMBOL_TREE_SYNCHRONIZING_ADAPTER::IsContainer( const wxDataViewItem& aItem
#define PROGRESS_INTERVAL_MILLIS 120
void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Sync( bool aForce, std::function<void(int, int, const wxString&)> aProgressCallback )
void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Sync( bool aForce,
std::function<void( int, int, const wxString& )> aProgressCallback )
{
wxLongLong nextUpdate = wxGetUTCTimeMillis() + (PROGRESS_INTERVAL_MILLIS / 2);
@ -73,7 +79,11 @@ void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Sync( bool aForce, std::function<void(in
nextUpdate = wxGetUTCTimeMillis() + PROGRESS_INTERVAL_MILLIS;
}
if( !m_libMgr->LibraryExists( name, true ) )
// There is a bug in LIB_MANAGER::LibraryExists() that uses the buffered modified
// libraries before the symbol library table which prevents the library from being
// removed from the tree control.
if( !m_libMgr->LibraryExists( name, true )
|| !m_frame->Prj().SchSymbolLibTable()->HasLibrary( name, true ) )
{
it = deleteLibrary( it );
continue;
@ -135,7 +145,7 @@ void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::updateLibrary( LIB_TREE_NODE_LIB& aLibNo
}
else if( hashIt->second != m_libMgr->GetLibraryHash( aLibNode.Name ) )
{
// update an existing libary
// update an existing library
std::list<LIB_ALIAS*> aliases = m_libMgr->GetAliases( aLibNode.Name );
// remove the common part from the aliases list
@ -210,11 +220,11 @@ void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::GetValue( wxVariant& aVariant, wxDataVie
case 0:
aVariant = node->Name;
// mark modified libs with an asterix
// mark modified libs with an asterisk
if( node->Type == LIB_TREE_NODE::LIB && m_libMgr->IsLibraryModified( node->Name ) )
aVariant = node->Name + " *";
// mark modified parts with an asterix
// mark modified parts with an aster-ix
if( node->Type == LIB_TREE_NODE::LIBID
&& m_libMgr->IsPartModified( node->Name, node->Parent->Name ) )
aVariant = node->Name + " *";
@ -298,5 +308,3 @@ bool SYMBOL_TREE_SYNCHRONIZING_ADAPTER::GetAttr( wxDataViewItem const& aItem, un
return true;
}

6
eeschema/symbol_tree_synchronizing_adapter.h
View File

@ -28,12 +28,13 @@
#include <lib_tree_model_adapter.h>
#include <map>
class LIB_EDIT_FRAME;
class LIB_MANAGER;
class SYMBOL_TREE_SYNCHRONIZING_ADAPTER : public LIB_TREE_MODEL_ADAPTER
{
public:
static PTR Create( LIB_MANAGER* aLibs );
static PTR Create( LIB_EDIT_FRAME* aParent, LIB_MANAGER* aLibs );
bool IsContainer( const wxDataViewItem& aItem ) const override;
@ -54,8 +55,9 @@ protected:
bool GetAttr( wxDataViewItem const& aItem, unsigned int aCol,
wxDataViewItemAttr& aAttr ) const override;
SYMBOL_TREE_SYNCHRONIZING_ADAPTER( LIB_MANAGER* aLibMgr );
SYMBOL_TREE_SYNCHRONIZING_ADAPTER( LIB_EDIT_FRAME* aParent, LIB_MANAGER* aLibMgr );
LIB_EDIT_FRAME* m_frame;
LIB_MANAGER* m_libMgr;
///> Hashes to decide whether a library needs an update