From 9ff8d02e60d3f5e475722f8668ba15e64a1e0d5d Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Mon, 9 Oct 2023 14:58:49 -0400 Subject: [PATCH] Symbol library manager object changes. * Fix issue handling symbols with multiple inheritance. * Remove unused code from the symbol library manager object. * Splits out the library buffer and symbol buffer object so the can be unit tested without having to mock the symbol library manager object. * Add unit tests for library buffer and symbol buffer objects. --- eeschema/symbol_library_manager.cpp | 272 +++++------------- eeschema/symbol_library_manager.h | 261 ++++++++--------- qa/tests/eeschema/CMakeLists.txt | 6 +- .../eeschema/test_symbol_library_manager.cpp | 141 +++++++++ 4 files changed, 329 insertions(+), 351 deletions(-) create mode 100644 qa/tests/eeschema/test_symbol_library_manager.cpp diff --git a/eeschema/symbol_library_manager.cpp b/eeschema/symbol_library_manager.cpp index 350ec7af55..b275eb69a0 100644 --- a/eeschema/symbol_library_manager.cpp +++ b/eeschema/symbol_library_manager.cpp @@ -188,7 +188,7 @@ bool SYMBOL_LIBRARY_MANAGER::SaveLibrary( const wxString& aLibrary, const wxStri const auto& symbolBuffers = libBuf.GetBuffers(); - for( const auto& symbolBuf : symbolBuffers ) + for( const std::shared_ptr& symbolBuf : symbolBuffers ) { if( !libBuf.SaveBuffer( symbolBuf, aFileName, &*pi, true ) ) { @@ -287,13 +287,12 @@ bool SYMBOL_LIBRARY_MANAGER::IsSymbolModified( const wxString& aAlias, return false; const LIB_BUFFER& buf = libIt->second; - const std::shared_ptr symbolBuf = buf.GetBuffer( aAlias ); + const std::shared_ptr symbolBuf = buf.GetBuffer( aAlias ); return symbolBuf ? symbolBuf->IsModified() : false; } -void SYMBOL_LIBRARY_MANAGER::SetSymbolModified( const wxString& aAlias, - const wxString& aLibrary ) +void SYMBOL_LIBRARY_MANAGER::SetSymbolModified( const wxString& aAlias, const wxString& aLibrary ) { auto libIt = m_libs.find( aLibrary ); @@ -301,7 +300,7 @@ void SYMBOL_LIBRARY_MANAGER::SetSymbolModified( const wxString& aAlias, return; const LIB_BUFFER& buf = libIt->second; - std::shared_ptr symbolBuf = buf.GetBuffer( aAlias ); + std::shared_ptr symbolBuf = buf.GetBuffer( aAlias ); wxCHECK( symbolBuf, /* void */ ); @@ -513,20 +512,6 @@ bool SYMBOL_LIBRARY_MANAGER::UpdateSymbolAfterRename( LIB_SYMBOL* aSymbol, const } -bool SYMBOL_LIBRARY_MANAGER::FlushSymbol( const wxString& aAlias, const wxString& aLibrary ) -{ - auto it = m_libs.find( aLibrary ); - - if( it == m_libs.end() ) // no items to flush - return true; - - auto symbolBuf = it->second.GetBuffer( aAlias ); - wxCHECK( symbolBuf, false ); - - return it->second.SaveBuffer( symbolBuf, symTable() ); -} - - LIB_ID SYMBOL_LIBRARY_MANAGER::RevertSymbol( const wxString& aAlias, const wxString& aLibrary ) { auto it = m_libs.find( aLibrary ); @@ -800,8 +785,7 @@ std::set SYMBOL_LIBRARY_MANAGER::getOriginalSymbols( const wxString } -SYMBOL_LIBRARY_MANAGER::LIB_BUFFER& SYMBOL_LIBRARY_MANAGER::getLibraryBuffer( - const wxString& aLibrary ) +LIB_BUFFER& SYMBOL_LIBRARY_MANAGER::getLibraryBuffer( const wxString& aLibrary ) { auto it = m_libs.find( aLibrary ); @@ -875,23 +859,22 @@ bool SYMBOL_LIBRARY_MANAGER::UpdateLibraryBuffer( const wxString& aLibrary ) } -SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SYMBOL_BUFFER( LIB_SYMBOL* aSymbol, - std::unique_ptr aScreen ) : - m_screen( std::move( aScreen ) ), - m_symbol( aSymbol ) +SYMBOL_BUFFER::SYMBOL_BUFFER( LIB_SYMBOL* aSymbol, std::unique_ptr aScreen ) : + m_screen( std::move( aScreen ) ), + m_symbol( aSymbol ) { m_original = new LIB_SYMBOL( *aSymbol ); } -SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::~SYMBOL_BUFFER() +SYMBOL_BUFFER::~SYMBOL_BUFFER() { delete m_symbol; delete m_original; } -void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetSymbol( LIB_SYMBOL* aSymbol ) +void SYMBOL_BUFFER::SetSymbol( LIB_SYMBOL* aSymbol ) { wxCHECK( m_symbol != aSymbol, /* void */ ); wxASSERT( aSymbol ); @@ -907,7 +890,7 @@ void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetSymbol( LIB_SYMBOL* aSymbol ) } -void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetOriginal( LIB_SYMBOL* aSymbol ) +void SYMBOL_BUFFER::SetOriginal( LIB_SYMBOL* aSymbol ) { wxCHECK( m_original != aSymbol, /* void */ ); wxASSERT( aSymbol ); @@ -923,13 +906,13 @@ void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetOriginal( LIB_SYMBOL* aSymbol ) } -bool SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::IsModified() const +bool SYMBOL_BUFFER::IsModified() const { return m_screen && m_screen->IsContentModified(); } -LIB_SYMBOL* SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbol( const wxString& aAlias ) const +LIB_SYMBOL* LIB_BUFFER::GetSymbol( const wxString& aAlias ) const { auto buf = GetBuffer( aAlias ); @@ -940,11 +923,11 @@ LIB_SYMBOL* SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbol( const wxString& aAlia wxCHECK( symbol, nullptr ); - return symbol; + return symbol; } -bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen ) +bool LIB_BUFFER::CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen ) { wxASSERT( aCopy ); wxASSERT( aCopy->GetLib() == nullptr ); @@ -963,8 +946,7 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::CreateBuffer( LIB_SYMBOL* aCopy, SCH_SC } -bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::UpdateBuffer( std::shared_ptr aSymbolBuf, - LIB_SYMBOL* aCopy ) +bool LIB_BUFFER::UpdateBuffer( std::shared_ptr aSymbolBuf, LIB_SYMBOL* aCopy ) { wxCHECK( aCopy && aSymbolBuf, false ); @@ -979,7 +961,7 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::UpdateBuffer( std::shared_ptr aSymbolBuf ) +bool LIB_BUFFER::DeleteBuffer( std::shared_ptr aSymbolBuf ) { auto symbolBufIt = std::find( m_symbols.begin(), m_symbols.end(), aSymbolBuf ); wxCHECK( symbolBufIt != m_symbols.end(), false ); @@ -1001,124 +983,24 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::DeleteBuffer( std::shared_ptr aSymbolBuf, - SYMBOL_LIB_TABLE* aLibTable ) +bool LIB_BUFFER::SaveBuffer( std::shared_ptr aSymbolBuf, + const wxString& aFileName, SCH_PLUGIN* aPlugin, bool aBuffer ) { wxCHECK( aSymbolBuf, false ); - LIB_SYMBOL* libSymbol = aSymbolBuf->GetSymbol(); - LIB_SYMBOL* originalSymbol = aSymbolBuf->GetOriginal(); - wxCHECK( libSymbol && originalSymbol, false ); - SYMBOL_LIB_TABLE::SAVE_T result; - STRING_UTF8_MAP properties; - properties.emplace( SCH_LEGACY_PLUGIN::PropBuffering, "" ); - - wxString errorMsg = _( "Error saving symbol %s to library '%s'." ) + wxS( "\n%s" ); - - // Delete the original symbol if the symbol name has been changed. - if( libSymbol->GetName() != originalSymbol->GetName() ) - { - // DeleteSymbol may throw - try - { - if( aLibTable->LoadSymbol( m_libName, originalSymbol->GetName() ) ) - aLibTable->DeleteSymbol( m_libName, originalSymbol->GetName() ); - } - catch( const IO_ERROR& ioe ) - { - wxLogError( errorMsg, UnescapeString( originalSymbol->GetName() ), m_libName, - ioe.What() ); - return false; - } - } - - if( libSymbol->IsAlias() ) - { - LIB_SYMBOL* newCachedSymbol = new LIB_SYMBOL( *libSymbol ); - std::shared_ptr< LIB_SYMBOL > bufferedParent = libSymbol->GetParent().lock(); - - wxCHECK( bufferedParent, false ); - - LIB_SYMBOL* cachedParent = aLibTable->LoadSymbol( m_libName, bufferedParent->GetName() ); - - if( !cachedParent ) - { - cachedParent = new LIB_SYMBOL( *bufferedParent.get() ); - newCachedSymbol->SetParent( cachedParent ); - result = aLibTable->SaveSymbol( m_libName, cachedParent ); - wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false ); - result = aLibTable->SaveSymbol( m_libName, newCachedSymbol ); - wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false ); - - LIB_SYMBOL* originalParent = new LIB_SYMBOL( *bufferedParent.get() ); - aSymbolBuf->SetOriginal( originalParent ); - originalSymbol = new LIB_SYMBOL( *libSymbol ); - originalSymbol->SetParent( originalParent ); - aSymbolBuf->SetOriginal( originalSymbol ); - } - else - { - newCachedSymbol->SetParent( cachedParent ); - result = aLibTable->SaveSymbol( m_libName, newCachedSymbol ); - wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false ); - - auto originalBufferedParent = GetBuffer( bufferedParent->GetName() ); - wxCHECK( originalBufferedParent, false ); - originalSymbol = new LIB_SYMBOL( *libSymbol ); - originalSymbol->SetParent( originalBufferedParent->GetSymbol() ); - aSymbolBuf->SetOriginal( originalSymbol ); - } - } - else - { - wxArrayString derivedSymbols; - - if( GetDerivedSymbolNames( libSymbol->GetName(), derivedSymbols ) == 0 ) - { - result = aLibTable->SaveSymbol( m_libName, new LIB_SYMBOL( *libSymbol ) ); - wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false ); - aSymbolBuf->SetOriginal( new LIB_SYMBOL( *libSymbol ) ); - } - else - { - LIB_SYMBOL* parentSymbol = new LIB_SYMBOL( *libSymbol ); - - aLibTable->SaveSymbol( m_libName, parentSymbol ); - - for( auto& entry : derivedSymbols ) - { - std::shared_ptr symbol = GetBuffer( entry ); - - wxCHECK2( symbol, continue ); - - LIB_SYMBOL* derivedSymbol = new LIB_SYMBOL( *symbol->GetSymbol() ); - derivedSymbol->SetParent( parentSymbol ); - result = aLibTable->SaveSymbol( m_libName, derivedSymbol ); - wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false ); - } - } - } - - ++m_hash; - return true; -} - - -bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr aSymbolBuf, - const wxString& aFileName, - SCH_PLUGIN* aPlugin, bool aBuffer ) -{ - wxCHECK( aSymbolBuf, false ); - LIB_SYMBOL* libSymbol = aSymbolBuf->GetSymbol(); - LIB_SYMBOL* originalSymbol = aSymbolBuf->GetOriginal(); - wxCHECK( libSymbol && originalSymbol, false ); wxCHECK( !aFileName.IsEmpty(), false ); wxString errorMsg = _( "Error saving symbol %s to library '%s'." ) + wxS( "\n%s" ); - // set properties to prevent save file on every symbol save + // Set properties to prevent saving the file on every symbol save. STRING_UTF8_MAP properties; properties.emplace( SCH_LEGACY_PLUGIN::PropBuffering, "" ); + std::shared_ptr& symbolBuf = aSymbolBuf; + LIB_SYMBOL* libSymbol = symbolBuf->GetSymbol(); + LIB_SYMBOL* originalSymbol = symbolBuf->GetOriginal(); + + wxCHECK( libSymbol && originalSymbol, false ); + // Delete the original symbol if the symbol name has been changed. if( libSymbol->GetName() != originalSymbol->GetName() ) { @@ -1135,10 +1017,13 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptrIsAlias() ) { LIB_SYMBOL* newCachedSymbol = new LIB_SYMBOL( *libSymbol ); std::shared_ptr< LIB_SYMBOL > bufferedParent = libSymbol->GetParent().lock(); + parentSymbol = newCachedSymbol; wxCHECK( bufferedParent, false ); @@ -1210,64 +1095,48 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptrGetName(), derivedSymbols ) == 0 ) + try { + aPlugin->SaveSymbol( aFileName, parentSymbol, aBuffer ? &properties : nullptr ); + } + catch( const IO_ERROR& ioe ) + { + wxLogError( errorMsg, UnescapeString( libSymbol->GetName() ), aFileName, + ioe.What() ); + return false; + } + + aSymbolBuf->SetOriginal( new LIB_SYMBOL( *libSymbol ) ); + } + + wxArrayString derivedSymbols; + + // Reparent all symbols derived from the saved symbol. + if( GetDerivedSymbolNames( libSymbol->GetName(), derivedSymbols ) != 0 ) + { + // Save the derived symbols. + for( const wxString& entry : derivedSymbols ) + { + std::shared_ptr symbol = GetBuffer( entry ); + + wxCHECK2( symbol, continue ); + + LIB_SYMBOL* derivedSymbol = new LIB_SYMBOL( *symbol->GetSymbol() ); + derivedSymbol->SetParent( parentSymbol ); + try { - aPlugin->SaveSymbol( aFileName, new LIB_SYMBOL( *libSymbol ), + aPlugin->SaveSymbol( aFileName, new LIB_SYMBOL( *derivedSymbol ), aBuffer ? &properties : nullptr ); } catch( const IO_ERROR& ioe ) { - wxLogError( errorMsg, UnescapeString( libSymbol->GetName() ), aFileName, + wxLogError( errorMsg, UnescapeString( derivedSymbol->GetName() ), aFileName, ioe.What() ); return false; } - - aSymbolBuf->SetOriginal( new LIB_SYMBOL( *libSymbol ) ); - } - else - { - LIB_SYMBOL* parentSymbol = new LIB_SYMBOL( *libSymbol ); - - // Save the modified root symbol. - try - { - aPlugin->SaveSymbol( aFileName, parentSymbol, aBuffer ? &properties : nullptr ); - } - catch( const IO_ERROR& ioe ) - { - wxLogError( errorMsg, UnescapeString( libSymbol->GetName() ), aFileName, - ioe.What() ); - return false; - } - - aSymbolBuf->SetOriginal( new LIB_SYMBOL( *libSymbol ) ); - - // Save the derived symbols. - for( const wxString& entry : derivedSymbols ) - { - std::shared_ptr symbol = GetBuffer( entry ); - - wxCHECK2( symbol, continue ); - - LIB_SYMBOL* derivedSymbol = new LIB_SYMBOL( *symbol->GetSymbol() ); - derivedSymbol->SetParent( parentSymbol ); - - try - { - aPlugin->SaveSymbol( aFileName, new LIB_SYMBOL( *derivedSymbol ), - aBuffer ? &properties : nullptr ); - } - catch( const IO_ERROR& ioe ) - { - wxLogError( errorMsg, UnescapeString( derivedSymbol->GetName() ), aFileName, - ioe.What() ); - return false; - } - } } } @@ -1276,10 +1145,9 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr -SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetBuffer( const wxString& aAlias ) const +std::shared_ptr LIB_BUFFER::GetBuffer( const wxString& aAlias ) const { - for( std::shared_ptr entry : m_symbols ) + for( std::shared_ptr entry : m_symbols ) { if( entry->GetSymbol()->GetName() == aAlias ) return entry; @@ -1289,9 +1157,9 @@ SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetBuffer( const wxString& aAlias ) const } -bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::HasDerivedSymbols( const wxString& aParentName ) const +bool LIB_BUFFER::HasDerivedSymbols( const wxString& aParentName ) const { - for( auto& entry : m_symbols ) + for( const std::shared_ptr& entry : m_symbols ) { if( entry->GetSymbol()->IsAlias() ) { @@ -1309,10 +1177,9 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::HasDerivedSymbols( const wxString& aPar } -void SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbolNames( wxArrayString& aSymbolNames, - SYMBOL_NAME_FILTER aFilter ) +void LIB_BUFFER::GetSymbolNames( wxArrayString& aSymbolNames, SYMBOL_NAME_FILTER aFilter ) { - for( auto& entry : m_symbols ) + for( std::shared_ptr& entry : m_symbols ) { if( ( entry->GetSymbol()->IsAlias() && ( aFilter == SYMBOL_NAME_FILTER::ROOT_ONLY ) ) || ( entry->GetSymbol()->IsRoot() && ( aFilter == SYMBOL_NAME_FILTER::DERIVED_ONLY ) ) ) @@ -1323,12 +1190,11 @@ void SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbolNames( wxArrayString& aSymbolN } -size_t SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetDerivedSymbolNames( const wxString& aSymbolName, - wxArrayString& aList ) +size_t LIB_BUFFER::GetDerivedSymbolNames( const wxString& aSymbolName, wxArrayString& aList ) { wxCHECK( !aSymbolName.IsEmpty(), 0 ); - for( auto& entry : m_symbols ) + for( std::shared_ptr& entry : m_symbols ) { if( entry->GetSymbol()->IsAlias() ) { @@ -1350,7 +1216,7 @@ size_t SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetDerivedSymbolNames( const wxString } -int SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::removeChildSymbols( std::shared_ptr& aSymbolBuf ) +int LIB_BUFFER::removeChildSymbols( std::shared_ptr& aSymbolBuf ) { wxCHECK( aSymbolBuf, 0 ); diff --git a/eeschema/symbol_library_manager.h b/eeschema/symbol_library_manager.h index b29a352a68..a7ad453c0e 100644 --- a/eeschema/symbol_library_manager.h +++ b/eeschema/symbol_library_manager.h @@ -54,6 +54,121 @@ enum class SYMBOL_NAME_FILTER }; +class SYMBOL_BUFFER +{ +public: + SYMBOL_BUFFER( LIB_SYMBOL* aSymbol = nullptr, std::unique_ptr aScreen = nullptr ); + ~SYMBOL_BUFFER(); + + LIB_SYMBOL* GetSymbol() const { return m_symbol; } + void SetSymbol( LIB_SYMBOL* aSymbol ); + + LIB_SYMBOL* GetOriginal() const { return m_original; } + void SetOriginal( LIB_SYMBOL* aSymbol ); + + bool IsModified() const; + SCH_SCREEN* GetScreen() const { return m_screen.get(); } + +private: + std::unique_ptr m_screen; + + LIB_SYMBOL* m_symbol; // Working copy + LIB_SYMBOL* m_original; // Initial state of the symbol +}; + + +///< Store a working copy of a library. +class LIB_BUFFER +{ +public: + LIB_BUFFER( const wxString& aLibrary ) : m_libName( aLibrary ), m_hash( 1 ) {} + + bool IsModified() const + { + if( !m_deleted.empty() ) + return true; + + for( const std::shared_ptr& symbolBuf : m_symbols ) + { + if( symbolBuf->IsModified() ) + return true; + } + + return false; + } + + int GetHash() const { return m_hash; } + + ///< Return the working copy of a LIB_SYMBOL root object with specified alias. + LIB_SYMBOL* GetSymbol( const wxString& aAlias ) const; + + ///< Create a new buffer to store a symbol. LIB_BUFFER takes ownership of aCopy. + bool CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen ); + + ///< Update the buffered symbol with the contents of \a aCopy. + bool UpdateBuffer( std::shared_ptr aSymbolBuf, LIB_SYMBOL* aCopy ); + + bool DeleteBuffer( std::shared_ptr aSymbolBuf ); + + void ClearDeletedBuffer() { m_deleted.clear(); } + + ///< Save 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( std::shared_ptr aSymbolBuf, const wxString& aFileName, + SCH_PLUGIN* aPlugin, bool aBuffer ); + + ///< Return a symbol buffer with LIB_SYMBOL holding a symbolicular alias + std::shared_ptr GetBuffer( const wxString& aAlias ) const; + + ///< Return all buffered symbols + const std::deque>& GetBuffers() const { return m_symbols; } + + /** + * Check to see any symbols in the buffer are derived from a parent named \a aParentName. + * + * @param aParentName is the name of the parent to test. + * @return true if any symbols are found derived from a symbol named \a aParent, otherwise + * false. + */ + bool HasDerivedSymbols( const wxString& aParentName ) const; + + /** + * Fetch a list of root symbols names from the library buffer. + * + * @param aRootSymbolNames is a reference to a list to populate with root symbol names. + * @param aFilter is the symbol derivation type. + */ + void GetSymbolNames( wxArrayString& aSymbolNames, + SYMBOL_NAME_FILTER aFilter = SYMBOL_NAME_FILTER::ALL ); + + /** + * Fetch all of the symbols derived from a \a aSymbolName into \a aList. + * + * @param aSymbolName is the name of the symbol to search for derived symbols in this + * buffer. + * @param aList is the list of symbols names derived from \a aSymbolName. + * @return a size_t count of the number of symbols derived from \a aSymbolName. + */ + size_t GetDerivedSymbolNames( const wxString& aSymbolName, wxArrayString& aList ); + +private: + /** + * Remove all symbols derived from \a aParent from the library buffer. + * + * @param aParent is the #SYMBOL_BUFFER to check against. + * @return the count of #SYMBOL_BUFFER objects removed from the library. + */ + int removeChildSymbols( std::shared_ptr& aSymbolBuf ); + + std::deque> m_symbols; + + ///< Buffer for deleted symbols until library is saved. + std::deque> m_deleted; + const wxString m_libName; // Buffered library name + int m_hash; +}; + + /** * Class to handle modifications to the symbol libraries. */ @@ -200,14 +315,6 @@ public: */ bool IsLibraryReadOnly( const wxString& aLibrary ) const; - /** - * Save symbol changes to the library copy used by the schematic editor. Not it is not - * necessarily saved to the file. - * - * @return True on success, false otherwise. - */ - bool FlushSymbol( const wxString& aAlias, const wxString& aLibrary ); - /** * Save library to a file, including unsaved changes. * @@ -273,144 +380,6 @@ protected: SYMBOL_LIB_TABLE* symTable() const; ///< Class to store a working copy of a LIB_SYMBOL object and editor context. - class SYMBOL_BUFFER - { - public: - SYMBOL_BUFFER( LIB_SYMBOL* aSymbol = nullptr, - std::unique_ptr aScreen = nullptr ); - ~SYMBOL_BUFFER(); - - LIB_SYMBOL* GetSymbol() const { return m_symbol; } - void SetSymbol( LIB_SYMBOL* aSymbol ); - - LIB_SYMBOL* GetOriginal() const { return m_original; } - void SetOriginal( LIB_SYMBOL* aSymbol ); - - bool IsModified() const; - SCH_SCREEN* GetScreen() const { return m_screen.get(); } - - ///< Transfer the screen ownership - std::unique_ptr RemoveScreen() - { - return std::move( m_screen ); - } - - bool SetScreen( std::unique_ptr aScreen ) - { - bool ret = !!m_screen; - m_screen = std::move( aScreen ); - return ret; - } - - private: - std::unique_ptr m_screen; - - LIB_SYMBOL* m_symbol; // Working copy - LIB_SYMBOL* m_original; // Initial state of the symbol - }; - - - ///< Store a working copy of a library. - class LIB_BUFFER - { - public: - LIB_BUFFER( const wxString& aLibrary ) : - m_libName( aLibrary ), - m_hash( 1 ) - { } - - bool IsModified() const - { - if( !m_deleted.empty() ) - return true; - - for( const std::shared_ptr& symbolBuf : m_symbols ) - { - if( symbolBuf->IsModified() ) - return true; - } - - return false; - } - - int GetHash() const { return m_hash; } - - ///< Return the working copy of a LIB_SYMBOL root object with specified alias. - LIB_SYMBOL* GetSymbol( const wxString& aAlias ) const; - - ///< Create a new buffer to store a symbol. LIB_BUFFER takes ownership of aCopy. - bool CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen ); - - ///< Update the buffered symbol with the contents of \a aCopy. - bool UpdateBuffer( std::shared_ptr aSymbolBuf, LIB_SYMBOL* aCopy ); - - bool DeleteBuffer( std::shared_ptr aSymbolBuf ); - - void ClearDeletedBuffer() - { - m_deleted.clear(); - } - - ///< Save stored modifications to Symbol Lib Table. It may result in saving the symbol - ///< to disk as well, depending on the row properties. - bool SaveBuffer( std::shared_ptr aSymbolBuf, SYMBOL_LIB_TABLE* aLibTable ); - - ///< Save 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( std::shared_ptr aSymbolBuf, const wxString& aFileName, - SCH_PLUGIN* aPlugin, bool aBuffer ); - - ///< Return a symbol buffer with LIB_SYMBOL holding a symbolicular alias - std::shared_ptr GetBuffer( const wxString& aAlias ) const; - - ///< Return all buffered symbols - const std::deque< std::shared_ptr >& GetBuffers() const { return m_symbols; } - - /** - * Check to see any symbols in the buffer are derived from a parent named \a aParentName. - * - * @param aParentName is the name of the parent to test. - * @return true if any symbols are found derived from a symbol named \a aParent, otherwise - * false. - */ - bool HasDerivedSymbols( const wxString& aParentName ) const; - - /** - * Fetch a list of root symbols names from the library buffer. - * - * @param aRootSymbolNames is a reference to a list to populate with root symbol names. - * @param aFilter is the symbol derivation type. - */ - void GetSymbolNames( wxArrayString& aSymbolNames, - SYMBOL_NAME_FILTER aFilter = SYMBOL_NAME_FILTER::ALL ); - - /** - * Fetch all of the symbols derived from a \a aSymbolName into \a aList. - * - * @param aSymbolName is the name of the symbol to search for derived symbols in this - * buffer. - * @param aList is the list of symbols names derived from \a aSymbolName. - * @return a size_t count of the number of symbols derived from \a aSymbolName. - */ - size_t GetDerivedSymbolNames( const wxString& aSymbolName, wxArrayString& aList ); - - private: - /** - * Remove all symbols derived from \a aParent from the library buffer. - * - * @param aParent is the #SYMBOL_BUFFER to check against. - * @return the count of #SYMBOL_BUFFER objects removed from the library. - */ - int removeChildSymbols( std::shared_ptr& aSymbolBuf ); - - std::deque< std::shared_ptr > m_symbols; - - ///< Buffer for deleted symbols until library is saved. - std::deque< std::shared_ptr > m_deleted; - const wxString m_libName; // Buffered library name - int m_hash; - }; - /** * Return a set of #LIB_SYMBOL objects belonging to the original library. */ diff --git a/qa/tests/eeschema/CMakeLists.txt b/qa/tests/eeschema/CMakeLists.txt index 9ebbe3ba68..42449c8e93 100644 --- a/qa/tests/eeschema/CMakeLists.txt +++ b/qa/tests/eeschema/CMakeLists.txt @@ -3,7 +3,7 @@ # # Copyright (C) 2017 CERN # @author Alejandro GarcĂ­a Montoro -# Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors. +# Copyright (C) 2022-2023 KiCad Developers, see AUTHORS.txt for contributors. # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License @@ -71,11 +71,13 @@ set( QA_EESCHEMA_SRCS test_sch_sheet_path.cpp test_sch_sheet_list.cpp test_sch_symbol.cpp + test_symbol_library_manager.cpp ) if( WIN32 ) # We want to declare a resource manifest on Windows to enable UTF8 mode - # Without UTF8 mode, some random IO tests may fail, we set the active code page on normal kicad to UTF8 as well + # Without UTF8 mode, some random IO tests may fail, we set the active code page on normal + # kicad to UTF8 as well. if( MINGW ) # QA_EESCHEMA_RESOURCES variable is set by the macro. mingw_resource_compiler( qa_eeschema ) diff --git a/qa/tests/eeschema/test_symbol_library_manager.cpp b/qa/tests/eeschema/test_symbol_library_manager.cpp new file mode 100644 index 0000000000..8e9c634c2d --- /dev/null +++ b/qa/tests/eeschema/test_symbol_library_manager.cpp @@ -0,0 +1,141 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright (C) 2023 Wayne Stambaugh + * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation, either version 3 of the License, or (at your + * option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + */ + +/** + * @file + * Test suite for SYMBOL_LIBRARY_MANAGER object. + */ + +#include + +// Code under test +#include + +class SYMBOL_LIBRARY_MANAGER_TEST_FIXTURE +{ +public: + SYMBOL_LIBRARY_MANAGER_TEST_FIXTURE() + { + } +}; + + +BOOST_FIXTURE_TEST_SUITE( SymbolLibraryManager, SYMBOL_LIBRARY_MANAGER_TEST_FIXTURE ) + + +/** + * Test the SYMBOL_BUFFER object. + */ +BOOST_AUTO_TEST_CASE( SymbolBuffer ) +{ + LIB_SYMBOL* symbol = new LIB_SYMBOL( wxS( "Symbol" ) ); + std::unique_ptr screen( new SCH_SCREEN() ); + + SYMBOL_BUFFER buffer( symbol, std::move( screen ) ); + + BOOST_CHECK( !buffer.IsModified() ); + BOOST_CHECK( buffer.GetSymbol() == symbol ); + BOOST_CHECK( *buffer.GetOriginal() == *symbol ); + + buffer.GetScreen()->SetContentModified(); + BOOST_CHECK( buffer.IsModified() ); + + LIB_SYMBOL* originalSymbol = new LIB_SYMBOL( wxS( "OriginalSymbol" ) ); + buffer.SetOriginal( originalSymbol ); + + BOOST_CHECK( buffer.GetOriginal() == originalSymbol ); + BOOST_CHECK( *buffer.GetOriginal() == *originalSymbol ); + BOOST_CHECK( *buffer.GetSymbol() != *buffer.GetOriginal() ); + + // Allocated memory is cleaned up by the SYMBOL_BUFFER object dtor. +} + + +/** + * Test the LIB_BUFFER object. + */ +BOOST_AUTO_TEST_CASE( LibBuffer ) +{ + wxArrayString symbolNames; + LIB_BUFFER libBuffer( wxS( "TestLibrary" ) ); + + BOOST_CHECK( !libBuffer.IsModified() ); + BOOST_CHECK_EQUAL( libBuffer.GetHash(), 1 ); + + LIB_SYMBOL* parentSymbol1 = new LIB_SYMBOL( wxS( "Parent1" ) ); + + BOOST_CHECK_EQUAL( libBuffer.GetSymbol( parentSymbol1->GetName() ), nullptr ); + + parentSymbol1->GetValueField().SetText( parentSymbol1->GetName() ); + libBuffer.CreateBuffer( parentSymbol1, new SCH_SCREEN() ); + BOOST_CHECK( libBuffer.GetSymbol( parentSymbol1->GetName() ) == parentSymbol1 ); + BOOST_CHECK( !libBuffer.HasDerivedSymbols( parentSymbol1->GetName() ) ); + BOOST_CHECK_EQUAL( libBuffer.GetHash(), 2 ); + + libBuffer.GetSymbolNames( symbolNames ); + BOOST_CHECK_EQUAL( symbolNames.GetCount(), 1 ); + BOOST_CHECK_EQUAL( symbolNames[0], parentSymbol1->GetName() ); + + symbolNames.Clear(); + libBuffer.GetSymbolNames( symbolNames, SYMBOL_NAME_FILTER::ROOT_ONLY ); + BOOST_CHECK_EQUAL( symbolNames.GetCount(), 1 ); + BOOST_CHECK_EQUAL( symbolNames[0], parentSymbol1->GetName() ); + + symbolNames.Clear(); + libBuffer.GetSymbolNames( symbolNames, SYMBOL_NAME_FILTER::DERIVED_ONLY ); + BOOST_CHECK_EQUAL( symbolNames.GetCount(), 0 ); + + LIB_SYMBOL* childSymbol1 = new LIB_SYMBOL( wxS( "Child1" ) ); + childSymbol1->SetParent( parentSymbol1 ); + childSymbol1->GetValueField().SetText( childSymbol1->GetName() ); + libBuffer.CreateBuffer( childSymbol1, new SCH_SCREEN() ); + BOOST_CHECK( libBuffer.GetSymbol( childSymbol1->GetName() ) == childSymbol1 ); + BOOST_CHECK( libBuffer.HasDerivedSymbols( parentSymbol1->GetName() ) ); + BOOST_CHECK_EQUAL( libBuffer.GetHash(), 3 ); + + symbolNames.Clear(); + libBuffer.GetSymbolNames( symbolNames ); + BOOST_CHECK_EQUAL( symbolNames.GetCount(), 2 ); + BOOST_CHECK_EQUAL( symbolNames[0], parentSymbol1->GetName() ); + BOOST_CHECK_EQUAL( symbolNames[1], childSymbol1->GetName() ); + + symbolNames.Clear(); + libBuffer.GetSymbolNames( symbolNames, SYMBOL_NAME_FILTER::DERIVED_ONLY ); + BOOST_CHECK_EQUAL( symbolNames.GetCount(), 1 ); + BOOST_CHECK_EQUAL( symbolNames[0], childSymbol1->GetName() ); + + symbolNames.Clear(); + BOOST_CHECK_EQUAL( libBuffer.GetDerivedSymbolNames( parentSymbol1->GetName(), + symbolNames ), 1 ); + BOOST_CHECK_EQUAL( symbolNames[0], childSymbol1->GetName() ); + + std::shared_ptr buf = libBuffer.GetBuffer( parentSymbol1->GetName() ); + LIB_SYMBOL* tmp = new LIB_SYMBOL( *parentSymbol1 ); + tmp->GetDescriptionField().SetText( wxS( "Description" ) ); + libBuffer.UpdateBuffer( buf, tmp ); + BOOST_CHECK_EQUAL( libBuffer.GetHash(), 4 ); + BOOST_CHECK( *libBuffer.GetSymbol( parentSymbol1->GetName() ) == *tmp ); + + libBuffer.DeleteBuffer( buf ); + BOOST_CHECK( libBuffer.GetBuffers().empty() ); +} + + +BOOST_AUTO_TEST_SUITE_END()