From edad0c2af1eb36b4e699e3fba1e822e5d802c388 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Sun, 9 Feb 2025 16:58:45 -0500 Subject: [PATCH] Move library table editing dialog to new system --- .../dialogs/panel_design_block_lib_table.cpp | 83 ++++- common/grid_tricks.cpp | 2 +- common/lib_table_grid_tricks.cpp | 6 + common/libraries/library_manager.cpp | 148 ++++++++- common/libraries/library_table.cpp | 94 +++--- common/libraries/library_table_parser.cpp | 40 +++ common/richio.cpp | 11 +- eeschema/dialogs/panel_sym_lib_table.cpp | 294 ++++++++---------- eeschema/dialogs/panel_sym_lib_table.h | 8 +- .../symbol_library_manager_adapter.cpp | 18 +- include/lib_table_grid.h | 88 +++--- include/libraries/library_manager.h | 19 +- include/libraries/library_table.h | 34 +- include/libraries/library_table_parser.h | 2 + include/richio.h | 7 +- include/xnode.h | 8 + pcbnew/dialogs/panel_fp_lib_table.cpp | 84 +++-- qa/tests/common/test_library_tables.cpp | 5 +- 18 files changed, 626 insertions(+), 325 deletions(-) diff --git a/common/dialogs/panel_design_block_lib_table.cpp b/common/dialogs/panel_design_block_lib_table.cpp index a4133574cc..0fd2a2c50c 100644 --- a/common/dialogs/panel_design_block_lib_table.cpp +++ b/common/dialogs/panel_design_block_lib_table.cpp @@ -65,7 +65,84 @@ #include #include +// clang-format off +/** + * Container that describes file type info for the add a library options + */ +struct SUPPORTED_FILE_TYPE +{ + wxString m_Description; ///< Description shown in the file picker dialog. + wxString m_FileFilter; ///< Filter used for file pickers if m_IsFile is true. + wxString m_FolderSearchExtension; ///< In case of folders it stands for extensions of files + ///< stored inside. + bool m_IsFile; ///< Whether the library is a folder or a file. + DESIGN_BLOCK_IO_MGR::DESIGN_BLOCK_FILE_T m_Plugin; +}; + +// clang-format on + +/** + * Traverser implementation that looks to find any and all "folder" libraries by looking for files + * with a specific extension inside folders + */ +class LIBRARY_TRAVERSER : public wxDirTraverser +{ +public: + LIBRARY_TRAVERSER( std::vector aSearchExtensions, wxString aInitialDir ) : + m_searchExtensions( aSearchExtensions ), m_currentDir( aInitialDir ) + { + } + + virtual wxDirTraverseResult OnFile( const wxString& aFileName ) override + { + wxFileName file( aFileName ); + + for( const std::string& ext : m_searchExtensions ) + { + if( file.GetExt().IsSameAs( ext, false ) ) + m_foundDirs.insert( { m_currentDir, 1 } ); + } + + return wxDIR_CONTINUE; + } + + virtual wxDirTraverseResult OnOpenError( const wxString& aOpenErrorName ) override + { + m_failedDirs.insert( { aOpenErrorName, 1 } ); + return wxDIR_IGNORE; + } + + bool HasDirectoryOpenFailures() { return m_failedDirs.size() > 0; } + + virtual wxDirTraverseResult OnDir( const wxString& aDirName ) override + { + m_currentDir = aDirName; + return wxDIR_CONTINUE; + } + + void GetPaths( wxArrayString& aPathArray ) + { + for( std::pair& foundDirsPair : m_foundDirs ) + aPathArray.Add( foundDirsPair.first ); + } + + void GetFailedPaths( wxArrayString& aPathArray ) + { + for( std::pair& failedDirsPair : m_failedDirs ) + aPathArray.Add( failedDirsPair.first ); + } + +private: + std::vector m_searchExtensions; + wxString m_currentDir; + std::unordered_map m_foundDirs; + std::unordered_map m_failedDirs; +}; + + +// TODO(JE) library tables +#if 0 /** * This class builds a wxGridTableBase by wrapping an #DESIGN_BLOCK_LIB_TABLE object. */ @@ -1065,7 +1142,7 @@ void PANEL_DESIGN_BLOCK_LIB_TABLE::populateEnvironReadOnlyTable() //-------------------------------------- - +#endif size_t PANEL_DESIGN_BLOCK_LIB_TABLE::m_pageNdx = 0; @@ -1081,12 +1158,14 @@ void InvokeEditDesignBlockLibTable( KIWAY* aKiway, wxWindow *aParent ) if( aKiway->Prj().IsNullProject() ) projectTable = nullptr; - + // TODO(JE) library tables +#if 0 dlg.InstallPanel( new PANEL_DESIGN_BLOCK_LIB_TABLE( &dlg, &aKiway->Prj(), globalTable, globalTablePath, projectTable, projectTablePath, aKiway->Prj().GetProjectPath() ) ); +#endif if( dlg.ShowModal() == wxID_CANCEL ) return; diff --git a/common/grid_tricks.cpp b/common/grid_tricks.cpp index a6f3454fb7..8da752cb6e 100644 --- a/common/grid_tricks.cpp +++ b/common/grid_tricks.cpp @@ -699,7 +699,7 @@ void GRID_TRICKS::paste_clipboard() { wxLogNull doNotLog; // disable logging of failed clipboard actions - if( m_grid->IsEditable() && wxTheClipboard->Open() ) + if( m_grid->IsEditable() && ( wxTheClipboard->IsOpened() || wxTheClipboard->Open() ) ) { if( wxTheClipboard->IsSupported( wxDF_TEXT ) || wxTheClipboard->IsSupported( wxDF_UNICODETEXT ) ) diff --git a/common/lib_table_grid_tricks.cpp b/common/lib_table_grid_tricks.cpp index 2a60f68e57..8142717d7c 100644 --- a/common/lib_table_grid_tricks.cpp +++ b/common/lib_table_grid_tricks.cpp @@ -89,6 +89,8 @@ void LIB_TABLE_GRID_TRICKS::showPopupMenu( wxMenu& menu, wxGridEvent& aEvent ) bool showSettings = false; + // TODO(JE) library tables +#if 0 if( m_sel_row_count == 1 && tbl->At( m_sel_row_start )->SupportsSettingsDialog() ) { showSettings = true; @@ -96,6 +98,7 @@ void LIB_TABLE_GRID_TRICKS::showPopupMenu( wxMenu& menu, wxGridEvent& aEvent ) wxString::Format( _( "Library settings for %s..." ), tbl->GetValue( m_sel_row_start, 2 ) ) ); } +#endif if( showActivate || showDeactivate || showSetVisible || showUnsetVisible || showSettings ) menu.AppendSeparator(); @@ -137,9 +140,12 @@ void LIB_TABLE_GRID_TRICKS::doPopupSelection( wxCommandEvent& event ) } else if( menu_id == LIB_TABLE_GRID_TRICKS_LIBRARY_SETTINGS ) { + // TODO(JE) library tables +#if 0 LIB_TABLE_ROW* row = tbl->At( m_sel_row_start ); row->Refresh(); row->ShowSettingsDialog( m_grid->GetParent() ); +#endif } else { diff --git a/common/libraries/library_manager.cpp b/common/libraries/library_manager.cpp index a85fcbc0b5..61720ac638 100644 --- a/common/libraries/library_manager.cpp +++ b/common/libraries/library_manager.cpp @@ -20,9 +20,11 @@ #include #include +#include #include #include +#include #include #include @@ -47,7 +49,7 @@ LIBRARY_MANAGER::~LIBRARY_MANAGER() = default; void LIBRARY_MANAGER::loadTables( const wxString& aTablePath, LIBRARY_TABLE_SCOPE aScope ) { auto getTarget = - [&]() -> std::vector>& + [&]() -> std::map>& { switch( aScope ) { @@ -55,14 +57,14 @@ void LIBRARY_MANAGER::loadTables( const wxString& aTablePath, LIBRARY_TABLE_SCOP return m_tables; case LIBRARY_TABLE_SCOPE::PROJECT: - return m_project_tables; + return m_projectTables; default: wxCHECK_MSG( false, m_tables, "Invalid scope passed to loadTables" ); } }; - std::vector>& aTarget = getTarget(); + std::map>& aTarget = getTarget(); aTarget.clear(); @@ -73,13 +75,62 @@ void LIBRARY_MANAGER::loadTables( const wxString& aTablePath, LIBRARY_TABLE_SCOP wxFileName fn( aTablePath, name ); if( fn.IsFileReadable() ) - aTarget.emplace_back( std::make_unique( fn, aScope ) ); + { + auto table = std::make_unique( fn, aScope ); + aTarget[table->Type()] = std::move( table ); + } else + { wxLogTrace( traceLibraries, "No library table found at %s", fn.GetFullPath() ); + } } - for( const std::unique_ptr& t : aTarget ) - t->LoadNestedTables(); + for( const std::unique_ptr& t : aTarget | std::views::values ) + loadNestedTables( *t ); +} + + +void LIBRARY_MANAGER::loadNestedTables( LIBRARY_TABLE& aRootTable ) +{ + std::unordered_set seenTables; + + std::function processOneTable = + [&]( LIBRARY_TABLE& aTable ) + { + seenTables.insert( aTable.Path() ); + + for( LIBRARY_TABLE_ROW& row : aTable.Rows() ) + { + if( row.Type() == wxT( "Table" ) ) + { + wxFileName file( row.URI() ); + + // URI may be relative to parent + file.MakeAbsolute( wxFileName( aTable.Path() ).GetPath() ); + + WX_FILENAME::ResolvePossibleSymlinks( file ); + wxString src = file.GetFullPath(); + + if( seenTables.contains( src ) ) + { + wxLogTrace( traceLibraries, "Library table %s has already been loaded!", + src ); + row.SetOk( false ); + row.SetErrorDescription( + _( "A reference to this library table already exists" ) ); + continue; + } + + auto child = std::make_unique( file, aRootTable.Scope() ); + + processOneTable( *child ); + + m_childTables.insert( { row.URI(), std::move( child ) } ); + } + } + }; + + processOneTable( aRootTable ); } @@ -116,6 +167,29 @@ std::optional LIBRARY_MANAGER::Adapter( LIBRARY_TABLE_ } +std::optional LIBRARY_MANAGER::Table( LIBRARY_TABLE_TYPE aType, + LIBRARY_TABLE_SCOPE aScope ) const +{ + switch( aScope ) + { + case LIBRARY_TABLE_SCOPE::BOTH: + case LIBRARY_TABLE_SCOPE::UNINITIALIZED: + wxCHECK_MSG( false, std::nullopt, "Table() requires a single scope" ); + + case LIBRARY_TABLE_SCOPE::GLOBAL: + wxCHECK( m_tables.contains( aType ), std::nullopt ); + return m_tables.at( aType ).get(); + + case LIBRARY_TABLE_SCOPE::PROJECT: + // TODO: handle multiple projects + wxCHECK( m_projectTables.contains( aType ), std::nullopt ); + return m_projectTables.at( aType ).get(); + } + + return std::nullopt; +} + + std::vector LIBRARY_MANAGER::Rows( LIBRARY_TABLE_TYPE aType, LIBRARY_TABLE_SCOPE aScope, bool aIncludeInvalid ) const @@ -123,7 +197,9 @@ std::vector LIBRARY_MANAGER::Rows( LIBRARY_TABLE_TYPE std::map rows; std::vector rowOrder; - std::list>>> tables; + std::list> + >> tables; switch( aScope ) { @@ -132,11 +208,11 @@ std::vector LIBRARY_MANAGER::Rows( LIBRARY_TABLE_TYPE break; case LIBRARY_TABLE_SCOPE::PROJECT: - tables = { std::views::all( m_project_tables ) }; + tables = { std::views::all( m_projectTables ) }; break; case LIBRARY_TABLE_SCOPE::BOTH: - tables = { std::views::all( m_tables ), std::views::all( m_project_tables ) }; + tables = { std::views::all( m_tables ), std::views::all( m_projectTables ) }; break; case LIBRARY_TABLE_SCOPE::UNINITIALIZED: @@ -157,8 +233,8 @@ std::vector LIBRARY_MANAGER::Rows( LIBRARY_TABLE_TYPE { if( row.Type() == "Table" ) { - wxCHECK2( aTable->Children().contains( row.Nickname() ), continue ); - processTable( aTable->Children().at( row.Nickname() ) ); + wxCHECK2( m_childTables.contains( row.URI() ), continue ); + processTable( m_childTables.at( row.URI() ) ); } else { @@ -172,8 +248,11 @@ std::vector LIBRARY_MANAGER::Rows( LIBRARY_TABLE_TYPE } }; - for( const std::unique_ptr& table : std::views::join( tables ) ) + for( const std::unique_ptr& table : + std::views::join( tables ) | std::views::values ) + { processTable( table ); + } std::vector ret; @@ -206,7 +285,7 @@ void LIBRARY_MANAGER::LoadProjectTables( const wxString& aProjectPath ) } else { - m_project_tables.clear(); + m_projectTables.clear(); wxLogTrace( traceLibraries, "New project path %s is not readable, not loading project tables", aProjectPath ); @@ -214,6 +293,41 @@ void LIBRARY_MANAGER::LoadProjectTables( const wxString& aProjectPath ) } +LIBRARY_RESULT LIBRARY_MANAGER::Save( LIBRARY_TABLE* aTable ) const +{ + wxCHECK( aTable, tl::unexpected( LIBRARY_ERROR( "Internal error" ) ) ); + + // TODO(JE) clean this up; shouldn't need to iterate + for( const std::unique_ptr& t : m_tables | std::views::values ) + { + if( t.get() == aTable ) + { + wxLogTrace( traceLibraries, "Saving %s", aTable->Path() ); + wxFileName fn( aTable->Path() ); + // This should already be normalized, but just in case... + fn.Normalize( FN_NORMALIZE_FLAGS | wxPATH_NORM_ENV_VARS ); + + try + { + PRETTIFIED_FILE_OUTPUTFORMATTER formatter( fn.GetFullPath(), KICAD_FORMAT::FORMAT_MODE::LIBRARY_TABLE ); + aTable->Format( &formatter ); + } + catch( IO_ERROR& e ) + { + wxLogTrace( traceLibraries, "Exception while saving: %s", e.What() ); + return tl::unexpected( LIBRARY_ERROR( e.What() ) ); + } + + return LIBRARY_RESULT(); + } + } + + // Unmanaged table? TODO(JE) should this happen? + wxLogTrace( traceLibraries, "Can't save %s; unmanaged library", aTable->Path() ); + return tl::unexpected( LIBRARY_ERROR( "Internal error" ) ); +} + + std::optional LIBRARY_MANAGER::GetFullURI( LIBRARY_TABLE_TYPE aType, const wxString& aNickname, bool aSubstituted ) const @@ -232,6 +346,14 @@ std::optional LIBRARY_MANAGER::GetFullURI( LIBRARY_TABLE_TYPE aType, } +wxString LIBRARY_MANAGER::ExpandURI( const wxString& aShortURI, const PROJECT& aProject ) +{ + wxFileName path( ExpandEnvVarSubstitutions( aShortURI, &aProject ) ); + path.MakeAbsolute(); + return path.GetFullPath(); +} + + bool LIBRARY_MANAGER::UrisAreEquivalent( const wxString& aURI1, const wxString& aURI2 ) { // Avoid comparing filenames as wxURIs diff --git a/common/libraries/library_table.cpp b/common/libraries/library_table.cpp index d99f6211f4..44c6b368d0 100644 --- a/common/libraries/library_table.cpp +++ b/common/libraries/library_table.cpp @@ -18,8 +18,6 @@ * with this program. If not, see . */ -#include - #include #include @@ -31,6 +29,19 @@ #include +bool LIBRARY_TABLE_ROW::operator==( const LIBRARY_TABLE_ROW& aOther ) const +{ + return m_scope == aOther.m_scope + && m_nickname == aOther.m_nickname + && m_uri == aOther.m_uri + && m_type == aOther.m_type + && m_options == aOther.m_options + && m_description == aOther.m_description + && m_disabled == aOther.m_disabled + && m_hidden == aOther.m_hidden; +} + + std::map LIBRARY_TABLE_ROW::GetOptionsMap() const { return LIB_TABLE::ParseOptions( TO_UTF8( m_options ) ); @@ -46,8 +57,7 @@ LIBRARY_TABLE::LIBRARY_TABLE( const wxFileName &aPath, LIBRARY_TABLE_SCOPE aScop WX_FILENAME::ResolvePossibleSymlinks( file ); m_path = file.GetAbsolutePath(); - tl::expected ir = - parser.Parse( m_path.ToStdString() ); + tl::expected ir = parser.Parse( m_path.ToStdString() ); if( ir.has_value() ) { @@ -62,10 +72,33 @@ LIBRARY_TABLE::LIBRARY_TABLE( const wxFileName &aPath, LIBRARY_TABLE_SCOPE aScop LIBRARY_TABLE::LIBRARY_TABLE( const wxString &aBuffer, LIBRARY_TABLE_SCOPE aScope ) : + m_path( wxEmptyString ), m_scope( aScope ) { - m_ok = false; - // TODO + LIBRARY_TABLE_PARSER parser; + + tl::expected ir = + parser.ParseBuffer( aBuffer.ToStdString() ); + + if( ir.has_value() ) + { + m_ok = initFromIR( *ir ); + } + else + { + m_ok = false; + m_errorDescription = ir.error().description; + } +} + + +bool LIBRARY_TABLE::operator==( const LIBRARY_TABLE& aOther ) const +{ + return m_path == aOther.m_path + && m_scope == aOther.m_scope + && m_type == aOther.m_type + && m_version == aOther.m_version + && m_rows == aOther.m_rows; } @@ -108,56 +141,9 @@ bool LIBRARY_TABLE::addRowFromIR( const LIBRARY_TABLE_ROW_IR& aIR ) } -// TODO(JE) this shouldn't be called from ctor and probably shouldn't be in LIBRARY_TABLE at all, -// but instead in LIBRARY_MANAGER - just opening one LIBRARY_TABLE shouldn't necessarily attempt a -// load on all the child tables -void LIBRARY_TABLE::LoadNestedTables() -{ - std::unordered_set seenTables; - - std::function processOneTable = - [&]( LIBRARY_TABLE& aTable ) - { - seenTables.insert( aTable.Path() ); - - for( LIBRARY_TABLE_ROW& row : aTable.m_rows ) - { - if( row.m_type == wxT( "Table" ) ) - { - wxFileName file( row.m_uri ); - - // URI may be relative to parent - file.MakeAbsolute( wxFileName( aTable.Path() ).GetPath() ); - - WX_FILENAME::ResolvePossibleSymlinks( file ); - wxString src = file.GetFullPath(); - - if( seenTables.contains( src ) ) - { - wxLogTrace( traceLibraries, "Library table %s has already been loaded!", - src ); - row.m_ok = false; - row.m_errorDescription = _( "A reference to this library table already exists" ); - continue; - } - - auto child = std::make_unique( file, m_scope ); - - processOneTable( *child ); - - aTable.m_children.insert( { row.m_nickname, std::move( child ) } ); - } - } - }; - - processOneTable( *this ); -} - - void LIBRARY_TABLE::Format( OUTPUTFORMATTER* aOutput ) const { - if( !IsOk() ) - return; + wxCHECK_MSG( IsOk(), /* void */, "Don't attempt to format a table that isn't OK!" ); static const std::map types = { { LIBRARY_TABLE_TYPE::SYMBOL, "sym_lib_table" }, diff --git a/common/libraries/library_table_parser.cpp b/common/libraries/library_table_parser.cpp index af01bebb47..c3a1af4705 100644 --- a/common/libraries/library_table_parser.cpp +++ b/common/libraries/library_table_parser.cpp @@ -192,3 +192,43 @@ tl::expected LIBRARY_TABLE_PARSER::Parse( return state.model; } + + +tl::expected LIBRARY_TABLE_PARSER::ParseBuffer( + const std::string& aBuffer ) +{ + memory_input in( aBuffer, "" ); + LIBRARY_TABLE_PARSER_STATE state; + wxLogTrace( traceLibraries, "LIBRARY_TABLE_PARSER::Parse from string buffer" ); + + try + { + if( !parse( in, state ) ) + { + wxLogTrace( traceLibraries, "Parsing failed without throwing" ); + wxString msg = + wxString::Format( _( "An unexpected error occurred while reading library table") ); + return tl::unexpected( LIBRARY_PARSE_ERROR( { .description = msg } ) ); + } + } + catch( const parse_error& e ) + { + const auto& p = e.positions().front(); + std::string msg = fmt::format( "Error at line {}, column {}:\n{}\n{:>{}}\n{}", + p.line, p.column, in.line_at( p ), "^", p.column, + e.message() ); + + wxLogTrace( traceLibraries, "%s", msg.c_str() ); + + wxString description = wxString::Format( _( "Syntax error at line %zu, column %zu" ), + p.line, p.column ); + + return tl::unexpected( LIBRARY_PARSE_ERROR( { + .description = description, + .line = p.line, + .column = p.column + } ) ); + } + + return state.model; +} diff --git a/common/richio.cpp b/common/richio.cpp index b4cc50ce33..ff740fa592 100644 --- a/common/richio.cpp +++ b/common/richio.cpp @@ -609,10 +609,15 @@ void FILE_OUTPUTFORMATTER::write( const char* aOutBuf, int aCount ) PRETTIFIED_FILE_OUTPUTFORMATTER::PRETTIFIED_FILE_OUTPUTFORMATTER( const wxString& aFileName, + KICAD_FORMAT::FORMAT_MODE aFormatMode, const wxChar* aMode, char aQuoteChar ) : - OUTPUTFORMATTER( OUTPUTFMTBUFZ, aQuoteChar ) + OUTPUTFORMATTER( OUTPUTFMTBUFZ, aQuoteChar ), + m_mode( aFormatMode ) { + if( ADVANCED_CFG::GetCfg().m_CompactSave && m_mode == KICAD_FORMAT::FORMAT_MODE::NORMAL ) + m_mode = KICAD_FORMAT::FORMAT_MODE::COMPACT_TEXT_PROPERTIES; + m_fp = wxFopen( aFileName, aMode ); if( !m_fp ) @@ -636,9 +641,7 @@ bool PRETTIFIED_FILE_OUTPUTFORMATTER::Finish() if( !m_fp ) return false; - KICAD_FORMAT::Prettify( m_buf, ADVANCED_CFG::GetCfg().m_CompactSave - ? KICAD_FORMAT::FORMAT_MODE::COMPACT_TEXT_PROPERTIES - : KICAD_FORMAT::FORMAT_MODE::NORMAL ); + KICAD_FORMAT::Prettify( m_buf, m_mode ); if( fwrite( m_buf.c_str(), m_buf.length(), 1, m_fp ) != 1 ) THROW_IO_ERROR( strerror( errno ) ); diff --git a/eeschema/dialogs/panel_sym_lib_table.cpp b/eeschema/dialogs/panel_sym_lib_table.cpp index e8b6581276..b5227d8bc9 100644 --- a/eeschema/dialogs/panel_sym_lib_table.cpp +++ b/eeschema/dialogs/panel_sym_lib_table.cpp @@ -29,8 +29,8 @@ #include #include #include -#include -#include +#include +#include #include #include #include @@ -40,10 +40,13 @@ #include #include #include +#include +#include #include #include #include #include +#include #include #include #include @@ -56,7 +59,6 @@ #include - // clang-format off /** @@ -84,38 +86,39 @@ enum { // clang-format on -/** - * Build a wxGridTableBase by wrapping an #SYMBOL_LIB_TABLE object. - */ -class SYMBOL_LIB_TABLE_GRID : public LIB_TABLE_GRID, public SYMBOL_LIB_TABLE +class SYMBOL_LIB_TABLE_GRID : public LIB_TABLE_GRID { friend class PANEL_SYM_LIB_TABLE; friend class SYMBOL_GRID_TRICKS; protected: - LIB_TABLE_ROW* at( size_t aIndex ) override { return &m_rows.at( aIndex ); } + LIBRARY_TABLE_ROW& at( size_t aIndex ) override { return m_table.Rows().at( aIndex ); } - size_t size() const override { return m_rows.size(); } + size_t size() const override { return m_table.Rows().size(); } - LIB_TABLE_ROW* makeNewRow() override + LIBRARY_TABLE_ROW makeNewRow() override { - return dynamic_cast< LIB_TABLE_ROW* >( new SYMBOL_LIB_TABLE_ROW ); + return LIBRARY_TABLE_ROW(); } - LIB_TABLE_ROWS_ITER begin() override { return m_rows.begin(); } + LIBRARY_TABLE_ROWS_ITER begin() override { return m_table.Rows().begin(); } - LIB_TABLE_ROWS_ITER insert( LIB_TABLE_ROWS_ITER aIterator, LIB_TABLE_ROW* aRow ) override + LIBRARY_TABLE_ROWS_ITER insert( LIBRARY_TABLE_ROWS_ITER aIterator, + const LIBRARY_TABLE_ROW& aRow ) override { - return m_rows.insert( aIterator, aRow ); + return m_table.Rows().insert( aIterator, aRow ); } - void push_back( LIB_TABLE_ROW* aRow ) override { m_rows.push_back( aRow ); } + void push_back( const LIBRARY_TABLE_ROW& aRow ) override { m_table.Rows().push_back( aRow ); } - LIB_TABLE_ROWS_ITER erase( LIB_TABLE_ROWS_ITER aFirst, LIB_TABLE_ROWS_ITER aLast ) override + LIBRARY_TABLE_ROWS_ITER erase( LIBRARY_TABLE_ROWS_ITER aFirst, + LIBRARY_TABLE_ROWS_ITER aLast ) override { - return m_rows.erase( aFirst, aLast ); + return m_table.Rows().erase( aFirst, aLast ); } + LIBRARY_TABLE& Table() { return m_table; } + public: void SetValue( int aRow, int aCol, const wxString &aValue ) override { @@ -126,10 +129,9 @@ public: // If setting a filepath, attempt to auto-detect the format if( aCol == COL_URI ) { - LIB_TABLE_ROW* row = at( (size_t) aRow ); - wxString fullURI = row->GetFullURI( true ); - - SCH_IO_MGR::SCH_FILE_T pluginType = SCH_IO_MGR::GuessPluginTypeFromLibPath( fullURI ); + LIBRARY_TABLE_ROW& row = at( static_cast(aRow) ); + wxString uri = LIBRARY_MANAGER::ExpandURI( row.URI(), Pgm().GetSettingsManager().Prj() ); + SCH_IO_MGR::SCH_FILE_T pluginType = SCH_IO_MGR::GuessPluginTypeFromLibPath( uri ); if( pluginType == SCH_IO_MGR::SCH_FILE_UNKNOWN ) pluginType = SCH_IO_MGR::SCH_KICAD; @@ -139,12 +141,17 @@ public: } - SYMBOL_LIB_TABLE_GRID( const SYMBOL_LIB_TABLE& aTableToEdit ) + SYMBOL_LIB_TABLE_GRID( const LIBRARY_TABLE& aTableToEdit ) : + m_table( aTableToEdit ) { - m_rows = aTableToEdit.m_rows; } + +private: + /// Working copy of a table + LIBRARY_TABLE m_table; }; + class SYMBOL_GRID_TRICKS : public LIB_TABLE_GRID_TRICKS { public: @@ -164,27 +171,27 @@ public: protected: DIALOG_EDIT_LIBRARY_TABLES* m_dialog; - virtual void optionsEditor( int aRow ) override + void optionsEditor( int aRow ) override { SYMBOL_LIB_TABLE_GRID* tbl = (SYMBOL_LIB_TABLE_GRID*) m_grid->GetTable(); if( tbl->GetNumberRows() > aRow ) { - LIB_TABLE_ROW* row = tbl->at( (size_t) aRow ); - const wxString& options = row->GetOptions(); - wxString result = options; + LIBRARY_TABLE_ROW& row = tbl->at( static_cast( aRow ) ); + const wxString& options = row.Options(); + wxString result = options; std::map choices; - SCH_IO_MGR::SCH_FILE_T pi_type = SCH_IO_MGR::EnumFromStr( row->GetType() ); + SCH_IO_MGR::SCH_FILE_T pi_type = SCH_IO_MGR::EnumFromStr( row.Type() ); IO_RELEASER pi( SCH_IO_MGR::FindPlugin( pi_type ) ); pi->GetLibraryOptions( &choices ); - DIALOG_PLUGIN_OPTIONS dlg( m_dialog, row->GetNickName(), choices, options, &result ); + DIALOG_PLUGIN_OPTIONS dlg( m_dialog, row.Nickname(), choices, options, &result ); dlg.ShowModal(); if( options != result ) { - row->SetOptions( result ); + row.SetOptions( result ); m_grid->Refresh(); } } @@ -193,42 +200,30 @@ protected: /// handle specialized clipboard text, with leading "(sym_lib_table" or /// spreadsheet formatted text. - virtual void paste_text( const wxString& cb_text ) override + void paste_text( const wxString& cb_text ) override { - SYMBOL_LIB_TABLE_GRID* tbl = (SYMBOL_LIB_TABLE_GRID*) m_grid->GetTable(); - size_t ndx = cb_text.find( "(sym_lib_table" ); + SYMBOL_LIB_TABLE_GRID* tbl = static_cast( m_grid->GetTable() ); - if( ndx != std::string::npos ) + if( size_t ndx = cb_text.find( "(sym_lib_table" ); ndx != std::string::npos ) { // paste the SYMBOL_LIB_TABLE_ROWs of s-expression (sym_lib_table), starting // at column 0 regardless of current cursor column. - STRING_LINE_READER slr( TO_UTF8( cb_text ), wxS( "Clipboard" ) ); - LIB_TABLE_LEXER lexer( &slr ); - SYMBOL_LIB_TABLE tmp_tbl; - bool parsed = true; - - try + if( LIBRARY_TABLE tempTable( cb_text, tbl->Table().Scope() ); tempTable.IsOk() ) { - tmp_tbl.Parse( &lexer ); + std::ranges::copy( tempTable.Rows(), + std::inserter( tbl->Table().Rows(), tbl->Table().Rows().begin() ) ); + + if( tbl->GetView() ) + { + wxGridTableMessage msg( tbl, wxGRIDTABLE_NOTIFY_ROWS_INSERTED, 0, 0 ); + tbl->GetView()->ProcessTableMessage( msg ); + } } - catch( PARSE_ERROR& pe ) + else { - DisplayError( m_dialog, pe.What() ); - parsed = false; + DisplayError( m_dialog, tempTable.ErrorDescription() ); } - - if( parsed ) - { - // make sure the table is big enough... - if( tmp_tbl.GetCount() > (unsigned) tbl->GetNumberRows() ) - tbl->AppendRows( tmp_tbl.GetCount() - tbl->GetNumberRows() ); - - for( unsigned i = 0; i < tmp_tbl.GetCount(); ++i ) - tbl->ReplaceRow( i, tmp_tbl.At( i ).clone() ); - } - - m_grid->AutoSizeColumns( false ); } else { @@ -250,6 +245,8 @@ protected: m_grid->AutoSizeColumns( false ); } + + m_grid->AutoSizeColumns( false ); } bool supportsVisibilityColumn() override @@ -288,10 +285,11 @@ void PANEL_SYM_LIB_TABLE::setupGrid( WX_GRID* aGrid ) m_parent, aGrid, &cfg->m_lastSymbolLibDir, true, m_project->GetProjectPath(), []( WX_GRID* grid, int row ) -> wxString { - auto* libTable = static_cast( grid->GetTable() ); - auto* tableRow = static_cast( libTable->at( row ) ); + auto libTable = static_cast( grid->GetTable() ); + LIBRARY_TABLE_ROW& tableRow = libTable->at( row ); + SCH_IO_MGR::SCH_FILE_T pi_type = SCH_IO_MGR::EnumFromStr( tableRow.Type() ); - IO_RELEASER pi( SCH_IO_MGR::FindPlugin( tableRow->GetFileType() ) ); + IO_RELEASER pi( SCH_IO_MGR::FindPlugin( pi_type ) ); if( pi ) { @@ -335,20 +333,17 @@ void PANEL_SYM_LIB_TABLE::setupGrid( WX_GRID* aGrid ) }; -PANEL_SYM_LIB_TABLE::PANEL_SYM_LIB_TABLE( DIALOG_EDIT_LIBRARY_TABLES* aParent, PROJECT* aProject, - SYMBOL_LIB_TABLE* aGlobalTable, - const wxString& aGlobalTablePath, - SYMBOL_LIB_TABLE* aProjectTable, - const wxString& aProjectTablePath ) : +PANEL_SYM_LIB_TABLE::PANEL_SYM_LIB_TABLE( DIALOG_EDIT_LIBRARY_TABLES* aParent, PROJECT* aProject ) : PANEL_SYM_LIB_TABLE_BASE( aParent ), - m_globalTable( aGlobalTable ), - m_projectTable( aProjectTable ), m_project( aProject ), m_parent( aParent ) { + std::optional table = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::SYMBOL, LIBRARY_TABLE_SCOPE::GLOBAL ); + wxASSERT( table ); // wxGrid only supports user owned tables if they exist past end of ~wxGrid(), // so make it a grid owned table. - m_global_grid->SetTable( new SYMBOL_LIB_TABLE_GRID( *m_globalTable ), true ); + m_global_grid->SetTable( new SYMBOL_LIB_TABLE_GRID( *table.value() ) ); for( const SCH_IO_MGR::SCH_FILE_T& type : SCH_IO_MGR::SCH_FILE_T_vector ) { @@ -371,9 +366,12 @@ PANEL_SYM_LIB_TABLE::PANEL_SYM_LIB_TABLE( DIALOG_EDIT_LIBRARY_TABLES* aParent, P setupGrid( m_global_grid ); - if( m_projectTable ) + std::optional projectTable = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::SYMBOL, LIBRARY_TABLE_SCOPE::PROJECT ); + + if( projectTable ) { - m_project_grid->SetTable( new SYMBOL_LIB_TABLE_GRID( *m_projectTable ), true ); + m_project_grid->SetTable( new SYMBOL_LIB_TABLE_GRID( *projectTable.value() ), true ); setupGrid( m_project_grid ); } else @@ -567,45 +565,6 @@ bool PANEL_SYM_LIB_TABLE::verifyTables() } } - for( SYMBOL_LIB_TABLE* table : { global_model(), project_model() } ) - { - if( !table ) - continue; - - for( unsigned int r = 0; r < table->GetCount(); ++r ) - { - SYMBOL_LIB_TABLE_ROW& row = dynamic_cast( table->At( r ) ); - - // Newly-added rows won't have set this yet - row.SetParent( table ); - - if( !row.GetIsEnabled() ) - continue; - - try - { - if( row.Refresh() ) - { - if( table == global_model() ) - m_parent->m_GlobalTableChanged = true; - else - m_parent->m_ProjectTableChanged = true; - } - } - catch( const IO_ERROR& ioe ) - { - msg.Printf( _( "Symbol library '%s' failed to load." ), row.GetNickName() ); - - wxWindow* topLevelParent = wxGetTopLevelParent( this ); - wait.reset(); - wxMessageDialog errdlg( topLevelParent, msg + wxS( "\n" ) + ioe.What(), _( "Error Loading Library" ) ); - errdlg.ShowModal(); - - return true; - } - } - } - return true; } @@ -809,7 +768,15 @@ void PANEL_SYM_LIB_TABLE::moveUpHandler( wxCommandEvent& event ) m_cur_grid->OnMoveRowUp( [&]( int row ) { - cur_model()->ChangeRowOrder( row, -1 ); + SYMBOL_LIB_TABLE_GRID* tbl = cur_model(); + int curRow = m_cur_grid->GetGridCursorRow(); + + std::vector& rows = tbl->Table().Rows(); + + auto current = rows.begin() + curRow; + auto prev = rows.begin() + curRow - 1; + + std::iter_swap( current, prev ); // Update the wxGrid wxGridTableMessage msg( cur_model(), wxGRIDTABLE_NOTIFY_ROWS_INSERTED, row - 1, 0 ); @@ -823,7 +790,14 @@ void PANEL_SYM_LIB_TABLE::moveDownHandler( wxCommandEvent& event ) m_cur_grid->OnMoveRowDown( [&]( int row ) { - cur_model()->ChangeRowOrder( row, 1 ); + SYMBOL_LIB_TABLE_GRID* tbl = cur_model(); + int curRow = m_cur_grid->GetGridCursorRow(); + std::vector& rows = tbl->Table().Rows(); + + auto current = rows.begin() + curRow; + auto next = rows.begin() + curRow + 1; + + std::iter_swap( current, next ); // Update the wxGrid wxGridTableMessage msg( cur_model(), wxGRIDTABLE_NOTIFY_ROWS_INSERTED, row, 0 ); @@ -845,7 +819,6 @@ void PANEL_SYM_LIB_TABLE::onReset( wxCommandEvent& event ) return; } - DIALOG_GLOBAL_SYM_LIB_TABLE_CONFIG dlg( m_parent ); if( dlg.ShowModal() == wxID_OK ) @@ -855,7 +828,12 @@ void PANEL_SYM_LIB_TABLE::onReset( wxCommandEvent& event ) wxGridTableBase* table = m_global_grid->GetTable(); m_global_grid->DestroyTable( table ); - m_global_grid->SetTable( new SYMBOL_LIB_TABLE_GRID( *m_globalTable ), true ); + std::optional newTable = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::SYMBOL, + LIBRARY_TABLE_SCOPE::GLOBAL ); + wxASSERT( newTable ); + + m_global_grid->SetTable( new SYMBOL_LIB_TABLE_GRID( *newTable.value() ) ); m_global_grid->PopEventHandler( true ); setupGrid( m_global_grid ); m_parent->m_GlobalTableChanged = true; @@ -1001,16 +979,29 @@ bool PANEL_SYM_LIB_TABLE::TransferDataFromWindow() if( !verifyTables() ) return false; - if( *global_model() != *m_globalTable ) + std::optional optTable = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::SYMBOL, LIBRARY_TABLE_SCOPE::GLOBAL ); + wxCHECK( optTable, false ); + LIBRARY_TABLE* globalTable = *optTable; + + if( global_model()->Table() != *globalTable ) { m_parent->m_GlobalTableChanged = true; - m_globalTable->TransferRows( global_model()->m_rows ); + *globalTable = global_model()->Table(); } - if( project_model() && *project_model() != *m_projectTable ) + optTable = Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::SYMBOL, + LIBRARY_TABLE_SCOPE::PROJECT ); + + if( optTable && project_model() ) { - m_parent->m_ProjectTableChanged = true; - m_projectTable->TransferRows( project_model()->m_rows ); + LIBRARY_TABLE* projectTable = *optTable; + + if( project_model()->Table() != *projectTable ) + { + m_parent->m_ProjectTableChanged = true; + *projectTable = project_model()->Table(); + } } return true; @@ -1057,7 +1048,7 @@ void PANEL_SYM_LIB_TABLE::populateEnvironReadOnlyTable() // not used yet. It is automatically set by KiCad to the directory holding // the current project. unique.insert( PROJECT_VAR_NAME ); - unique.insert( SYMBOL_LIB_TABLE::GlobalPathEnvVariableName() ); + unique.insert( ENV_VAR::GetVersionedEnvVarName( wxS( "SYMBOL_DIR" ) ) ); for( const wxString& evName : unique ) { @@ -1098,19 +1089,19 @@ void PANEL_SYM_LIB_TABLE::onSizeGrid( wxSizeEvent& event ) SYMBOL_LIB_TABLE_GRID* PANEL_SYM_LIB_TABLE::global_model() const { - return (SYMBOL_LIB_TABLE_GRID*) m_global_grid->GetTable(); + return static_cast( m_global_grid->GetTable() ); } SYMBOL_LIB_TABLE_GRID* PANEL_SYM_LIB_TABLE::project_model() const { - return m_project_grid ? (SYMBOL_LIB_TABLE_GRID*) m_project_grid->GetTable() : nullptr; + return m_project_grid ? static_cast( m_project_grid->GetTable() ) : nullptr; } SYMBOL_LIB_TABLE_GRID* PANEL_SYM_LIB_TABLE::cur_model() const { - return (SYMBOL_LIB_TABLE_GRID*) m_cur_grid->GetTable(); + return static_cast( m_cur_grid->GetTable() ); } @@ -1119,25 +1110,12 @@ size_t PANEL_SYM_LIB_TABLE::m_pageNdx = 0; void InvokeSchEditSymbolLibTable( KIWAY* aKiway, wxWindow *aParent ) { - auto* symbolEditor = (SYMBOL_EDIT_FRAME*) aKiway->Player( FRAME_SCH_SYMBOL_EDITOR, false ); - - SYMBOL_LIB_TABLE* globalTable = &SYMBOL_LIB_TABLE::GetGlobalLibTable(); - wxString globalTablePath = SYMBOL_LIB_TABLE::GetGlobalTableFileName(); - SYMBOL_LIB_TABLE* projectTable = nullptr; - wxString projectPath = aKiway->Prj().GetProjectPath(); - wxFileName projectTableFn( projectPath, SYMBOL_LIB_TABLE::GetSymbolLibTableFileName() ); - wxString msg; - wxString currentLib; - - // TODO(JE) - // Don't allow editing project tables if no project is open - // if( !aKiway->Prj().IsNullProject() ) - // projectTable = PROJECT_SCH::SchSymbolLibTable( &aKiway->Prj() ); + auto symbolEditor = static_cast( aKiway->Player( FRAME_SCH_SYMBOL_EDITOR, + false ) ); + wxString msg; if( symbolEditor ) { - currentLib = symbolEditor->GetCurLib(); - // This prevents an ugly crash on OSX (https://bugs.launchpad.net/kicad/+bug/1765286) symbolEditor->FreezeLibraryTree(); @@ -1159,8 +1137,7 @@ void InvokeSchEditSymbolLibTable( KIWAY* aKiway, wxWindow *aParent ) DIALOG_EDIT_LIBRARY_TABLES dlg( aParent, _( "Symbol Libraries" ) ); dlg.SetKiway( &dlg, aKiway ); - dlg.InstallPanel( new PANEL_SYM_LIB_TABLE( &dlg, &aKiway->Prj(), globalTable, globalTablePath, - projectTable, projectTableFn.GetFullPath() ) ); + dlg.InstallPanel( new PANEL_SYM_LIB_TABLE( &dlg, &aKiway->Prj() ) ); if( dlg.ShowModal() == wxID_CANCEL ) { @@ -1172,28 +1149,31 @@ void InvokeSchEditSymbolLibTable( KIWAY* aKiway, wxWindow *aParent ) if( dlg.m_GlobalTableChanged ) { - try - { - globalTable->Save( globalTablePath ); - } - catch( const IO_ERROR& ioe ) - { - msg.Printf( _( "Error saving global library table:\n\n%s" ), ioe.What() ); - wxMessageBox( msg, _( "File Save Error" ), wxOK | wxICON_ERROR ); - } + std::optional optTable = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::SYMBOL, LIBRARY_TABLE_SCOPE::GLOBAL ); + wxCHECK( optTable, /* void */ ); + LIBRARY_TABLE* globalTable = *optTable; + + Pgm().GetLibraryManager().Save( globalTable ).map_error( + []( const LIBRARY_ERROR& aError ) + { + wxMessageBox( wxString::Format( _( "Error saving global library table:\n\n%s" ), aError.message ), + _( "File Save Error" ), wxOK | wxICON_ERROR ); + } ); } + std::optional projectTable = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::SYMBOL, LIBRARY_TABLE_SCOPE::PROJECT ); + if( projectTable && dlg.m_ProjectTableChanged ) { - try - { - projectTable->Save( projectTableFn.GetFullPath() ); - } - catch( const IO_ERROR& ioe ) - { - msg.Printf( _( "Error saving project-specific library table:\n\n%s" ), ioe.What() ); - wxMessageBox( msg, _( "File Save Error" ), wxOK | wxICON_ERROR ); - } + Pgm().GetLibraryManager().Save( *projectTable ).map_error( + []( const LIBRARY_ERROR& aError ) + { + wxMessageBox( wxString::Format( _( "Error saving project-specific library table:\n\n%s" ), + aError.message ), + _( "File Save Error" ), wxOK | wxICON_ERROR ); + } ); } if( symbolEditor ) diff --git a/eeschema/dialogs/panel_sym_lib_table.h b/eeschema/dialogs/panel_sym_lib_table.h index bc89118cf4..f6e73164f9 100644 --- a/eeschema/dialogs/panel_sym_lib_table.h +++ b/eeschema/dialogs/panel_sym_lib_table.h @@ -36,9 +36,7 @@ class PANEL_SYM_LIB_TABLE : public PANEL_SYM_LIB_TABLE_BASE { public: - PANEL_SYM_LIB_TABLE( DIALOG_EDIT_LIBRARY_TABLES* aParent, PROJECT* m_project, - SYMBOL_LIB_TABLE* aGlobal, const wxString& aGlobalTablePath, - SYMBOL_LIB_TABLE* aProject, const wxString& aProjectTablePath ); + PANEL_SYM_LIB_TABLE( DIALOG_EDIT_LIBRARY_TABLES* aParent, PROJECT* m_project ); virtual ~PANEL_SYM_LIB_TABLE(); private: @@ -84,10 +82,6 @@ private: */ bool allowAutomaticPluginTypeSelection( wxString& aLibraryPath ); -private: - // Caller's tables are modified only on OK button and successful verification. - SYMBOL_LIB_TABLE* m_globalTable; - SYMBOL_LIB_TABLE* m_projectTable; PROJECT* m_project; DIALOG_EDIT_LIBRARY_TABLES* m_parent; diff --git a/eeschema/libraries/symbol_library_manager_adapter.cpp b/eeschema/libraries/symbol_library_manager_adapter.cpp index d48075d08a..c6ae0f99ce 100644 --- a/eeschema/libraries/symbol_library_manager_adapter.cpp +++ b/eeschema/libraries/symbol_library_manager_adapter.cpp @@ -165,9 +165,7 @@ std::optional SYMBOL_LIBRARY_MANAGER_ADAPTER::fetchIfLoaded( wxString SYMBOL_LIBRARY_MANAGER_ADAPTER::getUri( const LIBRARY_TABLE_ROW* aRow ) { - wxFileName path( ExpandEnvVarSubstitutions( aRow->URI(), &Pgm().GetSettingsManager().Prj() ) ); - path.MakeAbsolute(); - return path.GetFullPath(); + return LIBRARY_MANAGER::ExpandURI( aRow->URI(), Pgm().GetSettingsManager().Prj() ); } @@ -367,7 +365,7 @@ void SYMBOL_LIBRARY_MANAGER_ADAPTER::AsyncLoad() { lib->plugin->EnumerateSymbolLib( dummyList, getUri( lib->row ), &options ); // TODO(JE) remove testing delay - std::this_thread::sleep_for( std::chrono::milliseconds( 500 ) ); + //std::this_thread::sleep_for( std::chrono::milliseconds( 500 ) ); lib->status.load_status = LOAD_STATUS::LOADED; } catch( IO_ERROR& e ) @@ -504,8 +502,8 @@ std::optional SYMBOL_LIBRARY_MANAGER_ADAPTER::GetLibraryDescription( c bool SYMBOL_LIBRARY_MANAGER_ADAPTER::HasLibrary( const wxString& aNickname, bool aCheckEnabled ) const { - if( std::optional r = fetchIfLoaded( aNickname ); const LIB_DATA* lib = *r ) - return !aCheckEnabled || !lib->row->Disabled(); + if( std::optional r = fetchIfLoaded( aNickname ); r.has_value() ) + return !aCheckEnabled || !( *r )->row->Disabled(); return false; } @@ -513,9 +511,11 @@ bool SYMBOL_LIBRARY_MANAGER_ADAPTER::HasLibrary( const wxString& aNickname, std::optional SYMBOL_LIBRARY_MANAGER_ADAPTER::GetLibraryStatus( const wxString& aNickname ) const { - // TODO(JE) should return status even if not loaded, so don't use fetchIfLoaded - if( std::optional result = fetchIfLoaded( aNickname ) ) - return ( *result )->status; + if( m_libraries.contains( aNickname ) ) + return m_libraries.at( aNickname ).status; + + if( GlobalLibraries.contains( aNickname ) ) + return GlobalLibraries.at( aNickname ).status; return std::nullopt; } diff --git a/include/lib_table_grid.h b/include/lib_table_grid.h index acbd44b015..3b5182e721 100644 --- a/include/lib_table_grid.h +++ b/include/lib_table_grid.h @@ -20,7 +20,7 @@ #ifndef __LIB_TABLE_GRID_H__ #define __LIB_TABLE_GRID_H__ -#include +#include #include #include @@ -61,19 +61,22 @@ public: wxString GetValue( int aRow, int aCol ) override { - if( aRow < (int) size() ) + wxCHECK( aRow >= 0, wxEmptyString ); + size_t row = static_cast( aRow ); + + if( row < size() ) { - const LIB_TABLE_ROW* r = at( (size_t) aRow ); + const LIBRARY_TABLE_ROW& r = at( row ); switch( aCol ) { - case COL_NICKNAME: return UnescapeString( r->GetNickName() ); - case COL_URI: return r->GetFullURI(); - case COL_TYPE: return r->GetType(); - case COL_OPTIONS: return r->GetOptions(); - case COL_DESCR: return r->GetDescr(); - case COL_ENABLED: return r->GetIsEnabled() ? wxT( "1" ) : wxT( "0" ); - case COL_VISIBLE: return r->GetIsVisible() ? wxT( "1" ) : wxT( "0" ); + case COL_NICKNAME: return UnescapeString( r.Nickname() ); + case COL_URI: return r.URI(); + case COL_TYPE: return r.Type(); + case COL_OPTIONS: return r.Options(); + case COL_DESCR: return r.Description(); + case COL_ENABLED: return r.Disabled() ? wxT( "0" ) : wxT( "1" ); + case COL_VISIBLE: return r.Hidden() ? wxT( "0" ) : wxT( "1" ); default: return wxEmptyString; } } @@ -101,39 +104,48 @@ public: bool GetValueAsBool( int aRow, int aCol ) override { - if( aRow < (int) size() && aCol == COL_ENABLED ) - return at( (size_t) aRow )->GetIsEnabled(); - else if( aRow < (int) size() && aCol == COL_VISIBLE ) - return at( (size_t) aRow )->GetIsVisible(); + wxCHECK( aRow >= 0, false ); + size_t row = static_cast( aRow ); + + if( row < size() && aCol == COL_ENABLED ) + return !at( row ).Disabled(); + else if( row < size() && aCol == COL_VISIBLE ) + return !at( row ).Hidden(); else return false; } void SetValue( int aRow, int aCol, const wxString& aValue ) override { - if( aRow < (int) size() ) + wxCHECK( aRow >= 0, /* void */ ); + size_t row = static_cast( aRow ); + + if( row < size() ) { - LIB_TABLE_ROW* r = at( (size_t) aRow ); + LIBRARY_TABLE_ROW& r = at( row ); switch( aCol ) { - case COL_NICKNAME: r->SetNickName( EscapeString( aValue, CTX_LIBID ) ); break; - case COL_URI: r->SetFullURI( aValue ); break; - case COL_TYPE: r->SetType( aValue ); break; - case COL_OPTIONS: r->SetOptions( aValue ); break; - case COL_DESCR: r->SetDescr( aValue ); break; - case COL_ENABLED: r->SetEnabled( aValue == wxT( "1" ) ); break; - case COL_VISIBLE: r->SetVisible( aValue == wxT( "1" ) ); break; + case COL_NICKNAME: r.SetNickname( EscapeString( aValue, CTX_LIBID ) ); break; + case COL_URI: r.SetURI( aValue ); break; + case COL_TYPE: r.SetType( aValue ); break; + case COL_OPTIONS: r.SetOptions( aValue ); break; + case COL_DESCR: r.SetDescription( aValue ); break; + case COL_ENABLED: r.SetDisabled( aValue == wxT( "0" ) ); break; + case COL_VISIBLE: r.SetHidden( aValue == wxT( "0" ) ); break; } } } void SetValueAsBool( int aRow, int aCol, bool aValue ) override { - if( aRow < (int) size() && aCol == COL_ENABLED ) - at( (size_t) aRow )->SetEnabled( aValue ); - else if( aRow < (int) size() && aCol == COL_VISIBLE ) - at( (size_t) aRow )->SetVisible( aValue ); + wxCHECK( aRow >= 0, /* void */ ); + size_t row = static_cast( aRow ); + + if( row < size() && aCol == COL_ENABLED ) + at( row ).SetDisabled( !aValue ); + else if( row < size() && aCol == COL_VISIBLE ) + at( row ).SetHidden( !aValue ); } bool IsEmptyCell( int aRow, int aCol ) override @@ -188,7 +200,7 @@ public: // aPos+aNumRows may wrap here, so both ends of the range are tested. if( aPos < size() && aPos + aNumRows <= size() ) { - LIB_TABLE_ROWS_ITER start = begin() + aPos; + LIBRARY_TABLE_ROWS_ITER start = begin() + aPos; erase( start, start + aNumRows ); if( GetView() ) @@ -225,33 +237,35 @@ public: { for( size_t i = 0; i < size(); ++i ) { - LIB_TABLE_ROW* row = at( i ); + LIBRARY_TABLE_ROW& row = at( i ); - if( row->GetNickName() == aNickname ) + if( row.Nickname() == aNickname ) return true; } return false; } - LIB_TABLE_ROW* At( size_t aIndex ) + LIBRARY_TABLE_ROW& At( size_t aIndex ) { return at( aIndex ); } protected: - virtual LIB_TABLE_ROW* at( size_t aIndex ) = 0; + virtual LIBRARY_TABLE_ROW& at( size_t aIndex ) = 0; virtual size_t size() const = 0; - virtual LIB_TABLE_ROW* makeNewRow() = 0; + virtual LIBRARY_TABLE_ROW makeNewRow() = 0; - virtual LIB_TABLE_ROWS_ITER begin() = 0; + virtual LIBRARY_TABLE_ROWS_ITER begin() = 0; - virtual LIB_TABLE_ROWS_ITER insert( LIB_TABLE_ROWS_ITER aIterator, LIB_TABLE_ROW* aRow ) = 0; + virtual LIBRARY_TABLE_ROWS_ITER insert( LIBRARY_TABLE_ROWS_ITER aIterator, + const LIBRARY_TABLE_ROW& aRow ) = 0; - virtual void push_back( LIB_TABLE_ROW* aRow ) = 0; + virtual void push_back( const LIBRARY_TABLE_ROW& aRow ) = 0; - virtual LIB_TABLE_ROWS_ITER erase( LIB_TABLE_ROWS_ITER aFirst, LIB_TABLE_ROWS_ITER aLast ) = 0; + virtual LIBRARY_TABLE_ROWS_ITER erase( LIBRARY_TABLE_ROWS_ITER aFirst, + LIBRARY_TABLE_ROWS_ITER aLast ) = 0; }; diff --git a/include/libraries/library_manager.h b/include/libraries/library_manager.h index 7c1c6c22d9..3123e93536 100644 --- a/include/libraries/library_manager.h +++ b/include/libraries/library_manager.h @@ -37,6 +37,7 @@ template using LIBRARY_RESULT = tl::expected; class LIBRARY_MANAGER_ADAPTER; +class PROJECT; class KICOMMON_API LIBRARY_MANAGER @@ -56,6 +57,9 @@ public: std::optional Adapter( LIBRARY_TABLE_TYPE aType ) const; + std::optional Table( LIBRARY_TABLE_TYPE aType, + LIBRARY_TABLE_SCOPE aScope ) const; + /** * Returns a flattened list of libraries of the given type * @param aType determines which type of libraries to return (symbol, footprint, ...) @@ -80,6 +84,8 @@ public: void LoadProjectTables( const wxString& aProjectPath ); + LIBRARY_RESULT Save( LIBRARY_TABLE* aTable ) const; + /** * Return the full location specifying URI for the LIB, either in original UI form or * in environment variable expanded form. @@ -92,13 +98,22 @@ public: std::optional GetFullURI( LIBRARY_TABLE_TYPE aType, const wxString& aNickname, bool aSubstituted = false ) const; + static wxString ExpandURI( const wxString& aShortURI, const PROJECT& aProject ); + static bool UrisAreEquivalent( const wxString& aURI1, const wxString& aURI2 ); private: void loadTables( const wxString& aTablePath, LIBRARY_TABLE_SCOPE aScope ); - std::vector> m_tables; - std::vector> m_project_tables; + void loadNestedTables( LIBRARY_TABLE& aTable ); + + std::map> m_tables; + + /// Map of full URI to table object for tables that are referenced by global or project tables + std::map> m_childTables; + + // TODO: support multiple projects + std::map> m_projectTables; std::map> m_adapters; }; diff --git a/include/libraries/library_table.h b/include/libraries/library_table.h index e29944b3bb..3bb5fc1dd5 100644 --- a/include/libraries/library_table.h +++ b/include/libraries/library_table.h @@ -57,18 +57,38 @@ public: LIBRARY_TABLE_ROW() = default; + bool operator==( const LIBRARY_TABLE_ROW& aOther ) const; + + void SetNickname( const wxString& aNickname ) { m_nickname = aNickname; } const wxString& Nickname() const { return m_nickname; } + + void SetURI( const wxString& aUri ) { m_uri = aUri; } const wxString& URI() const { return m_uri; } + + void SetType( const wxString& aType ) { m_type = aType; } const wxString& Type() const { return m_type; } + + void SetOptions( const wxString& aOptions ) { m_options = aOptions; } const wxString& Options() const { return m_options; } + + void SetDescription( const wxString& aDescription ) { m_description = aDescription; } const wxString& Description() const { return m_description; } + + void SetScope( LIBRARY_TABLE_SCOPE aScope ) { m_scope = aScope; } LIBRARY_TABLE_SCOPE Scope() const { return m_scope; } + + void SetDisabled( bool aDisabled = true ) { m_disabled = aDisabled; } bool Disabled() const { return m_disabled; } + + void SetHidden( bool aHidden = true ) { m_hidden = aHidden; } bool Hidden() const { return m_hidden; } std::map GetOptionsMap() const; + void SetOk( bool aOk = true ) { m_ok = aOk; } bool IsOk() const { return m_ok; } + + void SetErrorDescription( const wxString& aDescription ) { m_errorDescription = aDescription; } const wxString& ErrorDescription() const { return m_errorDescription; } private: @@ -86,6 +106,10 @@ private: }; +typedef std::vector::iterator LIBRARY_TABLE_ROWS_ITER; +typedef std::vector::const_iterator LIBRARY_TABLE_ROWS_CITER; + + class KICOMMON_API LIBRARY_TABLE { public: @@ -105,8 +129,7 @@ public: ~LIBRARY_TABLE() = default; - // TODO move out of this class - void LoadNestedTables(); + bool operator==( const LIBRARY_TABLE& aOther ) const; const wxString& Path() const { return m_path; } void SetPath( const wxString &aPath ) { m_path = aPath; } @@ -114,6 +137,9 @@ public: LIBRARY_TABLE_TYPE Type() const { return m_type; } void SetType( const LIBRARY_TABLE_TYPE aType ) { m_type = aType; } + void SetScope( LIBRARY_TABLE_SCOPE aScope ) { m_scope = aScope; } + LIBRARY_TABLE_SCOPE Scope() const { return m_scope; } + std::optional Version() const{ return m_version; } void SetVersion( const std::optional &aVersion ) { m_version = aVersion; } @@ -123,8 +149,6 @@ public: const std::vector& Rows() const { return m_rows; } std::vector& Rows() { return m_rows; } - const std::map>& Children() { return m_children; } - void Format( OUTPUTFORMATTER* aOutput ) const; private: @@ -146,8 +170,6 @@ private: wxString m_errorDescription; std::vector m_rows; - - std::map> m_children; }; #endif //LIBRARY_TABLE_H diff --git a/include/libraries/library_table_parser.h b/include/libraries/library_table_parser.h index e16157f10a..e8e1016271 100644 --- a/include/libraries/library_table_parser.h +++ b/include/libraries/library_table_parser.h @@ -65,6 +65,8 @@ public: tl::expected Parse( const std::filesystem::path& aPath ); + tl::expected ParseBuffer( const std::string& aBuffer ); + private: }; diff --git a/include/richio.h b/include/richio.h index 0842b3b240..e70a24d9cd 100644 --- a/include/richio.h +++ b/include/richio.h @@ -40,6 +40,7 @@ #include #include +#include /** * This is like sprintf() but the output is appended to a std::string instead of to a @@ -520,8 +521,9 @@ protected: class KICOMMON_API PRETTIFIED_FILE_OUTPUTFORMATTER : public OUTPUTFORMATTER { public: - PRETTIFIED_FILE_OUTPUTFORMATTER( const wxString& aFileName, const wxChar* aMode = wxT( "wt" ), - char aQuoteChar = '"' ); + PRETTIFIED_FILE_OUTPUTFORMATTER( const wxString& aFileName, + KICAD_FORMAT::FORMAT_MODE aFormatMode = KICAD_FORMAT::FORMAT_MODE::NORMAL, + const wxChar* aMode = wxT( "wt" ), char aQuoteChar = '"' ); ~PRETTIFIED_FILE_OUTPUTFORMATTER(); @@ -537,6 +539,7 @@ protected: private: FILE* m_fp; std::string m_buf; + KICAD_FORMAT::FORMAT_MODE m_mode; }; diff --git a/include/xnode.h b/include/xnode.h index 82385dde1c..2e37a0aaf4 100644 --- a/include/xnode.h +++ b/include/xnode.h @@ -38,6 +38,14 @@ class OUTPUTFORMATTER; class KIID; + +/** + * An extension of wxXmlAttribute that stores a variant type rather than just a string. + * Technically, XML requires that all attribute values be strings, but since XNODE is + * primarily used for s-expression formatting rather than XML formatting, and KiCad's + * s-expression format permits integer and floating-point numeric values in lists, this + * class allows storage of the source value so that it can be properly formatted in the output. + */ class KICOMMON_API XATTR : public wxXmlAttribute { public: diff --git a/pcbnew/dialogs/panel_fp_lib_table.cpp b/pcbnew/dialogs/panel_fp_lib_table.cpp index 65dc2dd478..dd9f49f910 100644 --- a/pcbnew/dialogs/panel_fp_lib_table.cpp +++ b/pcbnew/dialogs/panel_fp_lib_table.cpp @@ -74,46 +74,49 @@ #include #include #include +#include #include /** * This class builds a wxGridTableBase by wrapping an #FP_LIB_TABLE object. */ -class FP_LIB_TABLE_GRID : public LIB_TABLE_GRID, public FP_LIB_TABLE +class FP_LIB_TABLE_GRID : public LIB_TABLE_GRID { friend class PANEL_FP_LIB_TABLE; friend class FP_GRID_TRICKS; protected: - LIB_TABLE_ROW* at( size_t aIndex ) override { return &m_rows.at( aIndex ); } + LIBRARY_TABLE_ROW& at( size_t aIndex ) override { return m_table.Rows().at( aIndex ); } - size_t size() const override { return m_rows.size(); } + size_t size() const override { return m_table.Rows().size(); } - LIB_TABLE_ROW* makeNewRow() override + LIBRARY_TABLE_ROW makeNewRow() override { - return dynamic_cast< LIB_TABLE_ROW* >( new FP_LIB_TABLE_ROW ); + return LIBRARY_TABLE_ROW(); } - LIB_TABLE_ROWS_ITER begin() override { return m_rows.begin(); } + LIBRARY_TABLE_ROWS_ITER begin() override { return m_table.Rows().begin(); } - LIB_TABLE_ROWS_ITER insert( LIB_TABLE_ROWS_ITER aIterator, LIB_TABLE_ROW* aRow ) override + LIBRARY_TABLE_ROWS_ITER insert( LIBRARY_TABLE_ROWS_ITER aIterator, + const LIBRARY_TABLE_ROW& aRow ) override { - return m_rows.insert( aIterator, aRow ); + return m_table.Rows().insert( aIterator, aRow ); } - void push_back( LIB_TABLE_ROW* aRow ) override { m_rows.push_back( aRow ); } + void push_back( const LIBRARY_TABLE_ROW& aRow ) override { m_table.Rows().push_back( aRow ); } - LIB_TABLE_ROWS_ITER erase( LIB_TABLE_ROWS_ITER aFirst, LIB_TABLE_ROWS_ITER aLast ) override + LIBRARY_TABLE_ROWS_ITER erase( LIBRARY_TABLE_ROWS_ITER aFirst, + LIBRARY_TABLE_ROWS_ITER aLast ) override { - return m_rows.erase( aFirst, aLast ); + return m_table.Rows().erase( aFirst, aLast ); } public: - FP_LIB_TABLE_GRID( const FP_LIB_TABLE& aTableToEdit ) + FP_LIB_TABLE_GRID( const LIBRARY_TABLE& aTableToEdit ) : + m_table( aTableToEdit ) { - m_rows = aTableToEdit.m_rows; } void SetValue( int aRow, int aCol, const wxString &aValue ) override @@ -125,10 +128,9 @@ public: // If setting a filepath, attempt to auto-detect the format if( aCol == COL_URI ) { - LIB_TABLE_ROW* row = at( (size_t) aRow ); - wxString fullURI = row->GetFullURI( true ); - - PCB_IO_MGR::PCB_FILE_T pluginType = PCB_IO_MGR::GuessPluginTypeFromLibPath( fullURI ); + LIBRARY_TABLE_ROW& row = at( (size_t) aRow ); + wxString uri = LIBRARY_MANAGER::ExpandURI( row.URI(), Pgm().GetSettingsManager().Prj() ); + PCB_IO_MGR::PCB_FILE_T pluginType = PCB_IO_MGR::GuessPluginTypeFromLibPath( uri ); if( pluginType == PCB_IO_MGR::FILE_TYPE_NONE ) pluginType = PCB_IO_MGR::KICAD_SEXP; @@ -136,6 +138,10 @@ public: SetValue( aRow, COL_TYPE, PCB_IO_MGR::ShowType( pluginType ) ); } } + +private: + /// Working copy of a table + LIBRARY_TABLE m_table; }; @@ -163,21 +169,21 @@ protected: if( tbl->GetNumberRows() > aRow ) { - LIB_TABLE_ROW* row = tbl->at( (size_t) aRow ); - const wxString& options = row->GetOptions(); + LIBRARY_TABLE_ROW& row = tbl->at( static_cast( aRow ) ); + const wxString& options = row.Options(); wxString result = options; std::map choices; - PCB_IO_MGR::PCB_FILE_T pi_type = PCB_IO_MGR::EnumFromStr( row->GetType() ); + PCB_IO_MGR::PCB_FILE_T pi_type = PCB_IO_MGR::EnumFromStr( row.Type() ); IO_RELEASER pi( PCB_IO_MGR::PluginFind( pi_type ) ); pi->GetLibraryOptions( &choices ); - DIALOG_PLUGIN_OPTIONS dlg( m_dialog, row->GetNickName(), choices, options, &result ); + DIALOG_PLUGIN_OPTIONS dlg( m_dialog, row.Nickname(), choices, options, &result ); dlg.ShowModal(); if( options != result ) { - row->SetOptions( result ); + row.SetOptions( result ); m_grid->Refresh(); } } @@ -187,6 +193,8 @@ protected: /// spreadsheet formatted text. void paste_text( const wxString& cb_text ) override { + // TODO(JE) library tables +#if 0 FP_LIB_TABLE_GRID* tbl = (FP_LIB_TABLE_GRID*) m_grid->GetTable(); size_t ndx = cb_text.find( "(fp_lib_table" ); @@ -242,6 +250,7 @@ protected: m_grid->AutoSizeColumns( false ); } +#endif } @@ -290,8 +299,8 @@ void PANEL_FP_LIB_TABLE::setupGrid( WX_GRID* aGrid ) [this]( WX_GRID* grid, int row ) -> wxString { auto* libTable = static_cast( grid->GetTable() ); - auto* tableRow = static_cast( libTable->at( row ) ); - PCB_IO_MGR::PCB_FILE_T fileType = tableRow->GetFileType(); + LIBRARY_TABLE_ROW& tableRow = libTable->at( row ); + PCB_IO_MGR::PCB_FILE_T fileType = PCB_IO_MGR::EnumFromStr( tableRow.Type() ); const IO_BASE::IO_FILE_DESC& pluginDesc = m_supportedFpFiles.at( fileType ); if( pluginDesc.m_IsFile ) @@ -345,7 +354,12 @@ PANEL_FP_LIB_TABLE::PANEL_FP_LIB_TABLE( DIALOG_EDIT_LIBRARY_TABLES* aParent, PRO m_projectBasePath( aProjectBasePath ), m_parent( aParent ) { - m_global_grid->SetTable( new FP_LIB_TABLE_GRID( *aGlobalTable ), true ); + std::optional table = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::FOOTPRINT, + LIBRARY_TABLE_SCOPE::GLOBAL ); + wxASSERT( table ); + + m_global_grid->SetTable( new FP_LIB_TABLE_GRID( *table.value() ), true ); // add Cut, Copy, and Paste to wxGrids m_path_subs_grid->PushEventHandler( new GRID_TRICKS( m_path_subs_grid ) ); @@ -369,7 +383,10 @@ PANEL_FP_LIB_TABLE::PANEL_FP_LIB_TABLE( DIALOG_EDIT_LIBRARY_TABLES* aParent, PRO if( aProjectTable ) { + // TODO(JE) library tables +#if 0 m_project_grid->SetTable( new FP_LIB_TABLE_GRID( *aProjectTable ), true ); +#endif setupGrid( m_project_grid ); } else @@ -688,6 +705,8 @@ void PANEL_FP_LIB_TABLE::moveUpHandler( wxCommandEvent& event ) m_cur_grid->OnMoveRowUp( [&]( int row ) { + // TODO(JE) library tables +#if 0 FP_LIB_TABLE_GRID* tbl = cur_model(); boost::ptr_vector::auto_type move_me = tbl->m_rows.release( tbl->m_rows.begin() + row ); @@ -696,6 +715,7 @@ void PANEL_FP_LIB_TABLE::moveUpHandler( wxCommandEvent& event ) // Update the wxGrid wxGridTableMessage msg( tbl, wxGRIDTABLE_NOTIFY_ROWS_INSERTED, row - 1, 0 ); tbl->GetView()->ProcessTableMessage( msg ); +#endif } ); } @@ -705,6 +725,8 @@ void PANEL_FP_LIB_TABLE::moveDownHandler( wxCommandEvent& event ) m_cur_grid->OnMoveRowDown( [&]( int row ) { + // TODO(JE) library tables +#if 0 FP_LIB_TABLE_GRID* tbl = cur_model(); boost::ptr_vector::auto_type move_me = tbl->m_rows.release( tbl->m_rows.begin() + row ); @@ -713,6 +735,7 @@ void PANEL_FP_LIB_TABLE::moveDownHandler( wxCommandEvent& event ) // Update the wxGrid wxGridTableMessage msg( tbl, wxGRIDTABLE_NOTIFY_ROWS_INSERTED, row, 0 ); tbl->GetView()->ProcessTableMessage( msg ); +#endif } ); } @@ -1001,7 +1024,12 @@ void PANEL_FP_LIB_TABLE::onReset( wxCommandEvent& event ) wxGridTableBase* table = m_global_grid->GetTable(); m_global_grid->DestroyTable( table ); - m_global_grid->SetTable( new FP_LIB_TABLE_GRID( *m_globalTable ), true ); + std::optional newTable = + Pgm().GetLibraryManager().Table( LIBRARY_TABLE_TYPE::FOOTPRINT, + LIBRARY_TABLE_SCOPE::GLOBAL ); + wxASSERT( newTable ); + + m_global_grid->SetTable( new FP_LIB_TABLE_GRID( *newTable.value() ), true ); m_global_grid->PopEventHandler( true ); setupGrid( m_global_grid ); m_parent->m_GlobalTableChanged = true; @@ -1035,6 +1063,8 @@ bool PANEL_FP_LIB_TABLE::TransferDataFromWindow() if( verifyTables() ) { + // TODO(JE) library tables +#if 0 if( *global_model() != *m_globalTable ) { m_parent->m_GlobalTableChanged = true; @@ -1046,7 +1076,7 @@ bool PANEL_FP_LIB_TABLE::TransferDataFromWindow() m_parent->m_ProjectTableChanged = true; m_projectTable->TransferRows( project_model()->m_rows ); } - +#endif return true; } diff --git a/qa/tests/common/test_library_tables.cpp b/qa/tests/common/test_library_tables.cpp index 051b36851f..355657fb32 100644 --- a/qa/tests/common/test_library_tables.cpp +++ b/qa/tests/common/test_library_tables.cpp @@ -103,9 +103,6 @@ BOOST_AUTO_TEST_CASE( ParseAndConstruct ) wxString::Format( "Expected error '%s' but got '%s'", expected_error, table.ErrorDescription() ) ); - // TODO this should move to manager test suite - table.LoadNestedTables(); - // Non-parsed tables can't be formatted if( !table.IsOk() ) continue; @@ -141,7 +138,7 @@ BOOST_AUTO_TEST_CASE( Manager ) LIBRARY_MANAGER manager; manager.LoadGlobalTables(); - BOOST_REQUIRE( manager.Rows( LIBRARY_TABLE_TYPE::SYMBOL ).size() == 2 ); + BOOST_REQUIRE( manager.Rows( LIBRARY_TABLE_TYPE::SYMBOL ).size() == 3 ); BOOST_REQUIRE( manager.Rows( LIBRARY_TABLE_TYPE::FOOTPRINT ).size() == 146 ); }