From ff5a30938654fe611386ef2f94d4c26b8ae125a3 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 3 Feb 2025 12:21:38 -0800 Subject: [PATCH] Show inherited fields in symbols When working with derived symbols, make sure that we show all of the fields, not just the ones that are defined in the current level since they are all attached to the output. Inherited fields are shown in italics until changed Fixes https://gitlab.com/kicad/code/kicad/-/issues/11422 --- .../dialogs/dialog_lib_symbol_properties.cpp | 127 ++++++++++++---- .../dialogs/dialog_lib_symbol_properties.h | 2 + eeschema/fields_grid_table.cpp | 141 +++++++++++++++--- eeschema/fields_grid_table.h | 32 ++++ 4 files changed, 251 insertions(+), 51 deletions(-) diff --git a/eeschema/dialogs/dialog_lib_symbol_properties.cpp b/eeschema/dialogs/dialog_lib_symbol_properties.cpp index 820fb88527..103ffafebc 100644 --- a/eeschema/dialogs/dialog_lib_symbol_properties.cpp +++ b/eeschema/dialogs/dialog_lib_symbol_properties.cpp @@ -82,8 +82,48 @@ DIALOG_LIB_SYMBOL_PROPERTIES::DIALOG_LIB_SYMBOL_PROPERTIES( SYMBOL_EDIT_FRAME* a } ) ); m_grid->SetSelectionMode( wxGrid::wxGridSelectRows ); - // Load the FIELDS_GRID_TABLE - m_libEntry->CopyFields( *m_fields ); + // Load the FIELDS_GRID_TABLE -- ensure we are calling the overloaded push_back method + std::vector fields; + m_libEntry->CopyFields( fields ); + + for( const SCH_FIELD& f : fields ) + m_fields->push_back( f ); + + if( m_libEntry->IsDerived() ) + { + if( LIB_SYMBOL_SPTR parent = m_libEntry->GetParent().lock() ) + { + std::vector parentFields; + parent->GetFields( parentFields ); + + for( size_t ii = 0; ii < parentFields.size(); ++ii ) + { + SCH_FIELD* pf = parentFields[ii]; + bool found = false; + + if( pf->IsMandatory() ) + continue; // Don't inherit mandatory fields + + for( size_t jj = 0; jj < m_fields->size(); ++jj ) + { + SCH_FIELD& f = m_fields->at( jj ); + + if( f.IsMandatory() ) + continue; // Don't inherit mandatory fields + + if( f.GetCanonicalName() == pf->GetCanonicalName() ) + { + m_fields->SetFieldInherited( jj, *pf ); + found = true; + break; + } + } + + if( !found ) + m_fields->AddInheritedField( *pf ); + } + } + } // Show/hide columns according to the user's preference SYMBOL_EDITOR_SETTINGS* cfg = m_Parent->GetSettings(); @@ -115,6 +155,10 @@ DIALOG_LIB_SYMBOL_PROPERTIES::DIALOG_LIB_SYMBOL_PROPERTIES( SYMBOL_EDIT_FRAME* a // wxFormBuilder doesn't include this event... m_grid->Connect( wxEVT_GRID_CELL_CHANGING, wxGridEventHandler( DIALOG_LIB_SYMBOL_PROPERTIES::OnGridCellChanging ), nullptr, this ); + m_grid->Connect( wxEVT_GRID_CELL_CHANGED, + wxGridEventHandler( DIALOG_LIB_SYMBOL_PROPERTIES::OnGridCellChanged ), nullptr, this ); + m_grid->GetGridWindow()->Bind( wxEVT_MOTION, &DIALOG_LIB_SYMBOL_PROPERTIES::OnGridMotion, this ); + // Forward the delete button to the tricks m_deleteFilterButton->Bind( wxEVT_BUTTON, @@ -163,6 +207,9 @@ DIALOG_LIB_SYMBOL_PROPERTIES::~DIALOG_LIB_SYMBOL_PROPERTIES() m_grid->Disconnect( wxEVT_GRID_CELL_CHANGING, wxGridEventHandler( DIALOG_LIB_SYMBOL_PROPERTIES::OnGridCellChanging ), nullptr, this ); + m_grid->Disconnect( wxEVT_GRID_CELL_CHANGED, + wxGridEventHandler( DIALOG_LIB_SYMBOL_PROPERTIES::OnGridCellChanged ), nullptr, this ); + m_grid->GetGridWindow()->Unbind( wxEVT_MOTION, &DIALOG_LIB_SYMBOL_PROPERTIES::OnGridMotion, this ); // Delete the GRID_TRICKS. m_grid->PopEventHandler( true ); @@ -450,42 +497,39 @@ bool DIALOG_LIB_SYMBOL_PROPERTIES::TransferDataFromWindow() // The Y axis for components in lib is from bottom to top while the screen axis is top // to bottom: we must change the y coord sign when writing back to the library - for( SCH_FIELD& field : *m_fields ) - { - VECTOR2I pos = field.GetPosition(); - pos.y = -pos.y; - field.SetPosition( pos ); - } - + std::vector fieldsToSave; int ordinal = 42; // Arbitrarily larger than any mandatory FIELD_T ids. - for( SCH_FIELD& field : *m_fields ) - { - if( !field.IsMandatory() ) - field.SetOrdinal( ordinal++ ); - } - - for( int ii = m_fields->GetNumberRows() - 1; ii >= 0; ii-- ) + for( size_t ii = 0; ii < m_fields->size(); ++ii ) { SCH_FIELD& field = m_fields->at( ii ); - if( field.IsMandatory() ) - continue; + VECTOR2I pos = field.GetPosition(); + pos.y = -pos.y; + field.SetPosition( pos ); - const wxString& fieldName = field.GetCanonicalName(); + if( !field.IsMandatory() ) + field.SetOrdinal( ordinal++ ); + + wxString fieldName = field.GetCanonicalName(); + + if( m_fields->IsInherited( ii ) && field == m_fields->ParentField( ii ) ) + continue; // Skip inherited fields if( field.GetText().IsEmpty() ) { if( fieldName.IsEmpty() || m_addedTemplateFields.contains( fieldName ) ) - m_fields->erase( m_fields->begin() + ii ); + continue; // Skip empty fields that are not mandatory or template fields } else if( fieldName.IsEmpty() ) { - field.SetName( _( "untitled" ) ); + field.SetName( _( "untitled" ) ); // Set a default name for unnamed fields } + + fieldsToSave.push_back( field ); } - m_libEntry->SetFields( *m_fields ); + m_libEntry->SetFields( fieldsToSave ); // Update the parent for inherited symbols if( m_libEntry->IsDerived() ) @@ -576,6 +620,27 @@ bool DIALOG_LIB_SYMBOL_PROPERTIES::TransferDataFromWindow() } +void DIALOG_LIB_SYMBOL_PROPERTIES::OnGridMotion( wxMouseEvent& aEvent ) +{ + aEvent.Skip(); + + wxPoint pos = aEvent.GetPosition(); + wxPoint unscolled_pos = m_grid->CalcUnscrolledPosition( pos ); + int row = m_grid->YToRow( unscolled_pos.y ); + int col = m_grid->XToCol( unscolled_pos.x ); + + if( row == wxNOT_FOUND || col == wxNOT_FOUND || !m_fields->IsInherited( row ) ) + { + m_grid->SetToolTip( "" ); + return; + } + + m_grid->SetToolTip( + wxString::Format( _( "This field is inherited from '%s'." ), + m_fields->ParentField( row ).GetName() ) ); +} + + void DIALOG_LIB_SYMBOL_PROPERTIES::OnGridCellChanging( wxGridEvent& event ) { wxGridCellEditor* editor = m_grid->GetCellEditor( event.GetRow(), event.GetCol() ); @@ -613,6 +678,13 @@ void DIALOG_LIB_SYMBOL_PROPERTIES::OnGridCellChanging( wxGridEvent& event ) } +void DIALOG_LIB_SYMBOL_PROPERTIES::OnGridCellChanged( wxGridEvent& event ) +{ + m_grid->ForceRefresh(); + OnModify(); +} + + void DIALOG_LIB_SYMBOL_PROPERTIES::OnSymbolNameText( wxCommandEvent& event ) { if( m_OptionPower->IsChecked() ) @@ -683,7 +755,8 @@ void DIALOG_LIB_SYMBOL_PROPERTIES::OnDeleteField( wxCommandEvent& event ) }, [&]( int row ) { - m_fields->erase( m_fields->begin() + row ); + if( !m_fields->EraseRow( row ) ) + return; // notify the grid wxGridTableMessage msg( m_fields, wxGRIDTABLE_NOTIFY_ROWS_DELETED, row, 1 ); @@ -703,7 +776,7 @@ void DIALOG_LIB_SYMBOL_PROPERTIES::OnMoveUp( wxCommandEvent& event ) }, [&]( int row ) { - std::swap( *( m_fields->begin() + row ), *( m_fields->begin() + row - 1 ) ); + m_fields->SwapRows( row, row - 1 ); m_grid->ForceRefresh(); OnModify(); } ); @@ -719,9 +792,9 @@ void DIALOG_LIB_SYMBOL_PROPERTIES::OnMoveDown( wxCommandEvent& event ) }, [&]( int row ) { - std::swap( *( m_fields->begin() + row ), *( m_fields->begin() + row + 1 ) ); - m_grid->ForceRefresh(); - OnModify(); + m_fields->SwapRows( row, row + 1 ); + m_grid->ForceRefresh(); + OnModify(); } ); } diff --git a/eeschema/dialogs/dialog_lib_symbol_properties.h b/eeschema/dialogs/dialog_lib_symbol_properties.h index e17a8a268e..cea5a090d0 100644 --- a/eeschema/dialogs/dialog_lib_symbol_properties.h +++ b/eeschema/dialogs/dialog_lib_symbol_properties.h @@ -68,6 +68,8 @@ private: void OnEditFootprintFilter( wxCommandEvent& event ) override; void OnSizeGrid( wxSizeEvent& event ) override; void OnGridCellChanging( wxGridEvent& event ); + void OnGridCellChanged( wxGridEvent& event ); + void OnGridMotion( wxMouseEvent& event ); void OnEditSpiceModel( wxCommandEvent& event ) override; void OnUpdateUI( wxUpdateUIEvent& event ) override; void OnCancelButtonClick( wxCommandEvent& event ) override; diff --git a/eeschema/fields_grid_table.cpp b/eeschema/fields_grid_table.cpp index bb410cce8f..569820b76d 100644 --- a/eeschema/fields_grid_table.cpp +++ b/eeschema/fields_grid_table.cpp @@ -225,6 +225,15 @@ int FIELDS_GRID_TABLE::GetMandatoryRowCount() const } +void FIELDS_GRID_TABLE::push_back( const SCH_FIELD& aField ) +{ + std::vector::push_back( aField ); + + m_isInherited.resize( size() ); + m_parentFields.resize( size() ); +} + + void FIELDS_GRID_TABLE::initGrid( WX_GRID* aGrid ) { // Build the various grid cell attributes. @@ -399,6 +408,9 @@ FIELDS_GRID_TABLE::~FIELDS_GRID_TABLE() m_fontAttr->DecRef(); m_colorAttr->DecRef(); + for( SCH_FIELD& field : m_parentFields ) + field.SetParent( nullptr ); + m_frame->Unbind( EDA_EVT_UNITS_CHANGED, &FIELDS_GRID_TABLE::onUnitsChanged, this ); } @@ -538,33 +550,34 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr:: wxCHECK( aRow < GetNumberRows(), nullptr ); const SCH_FIELD& field = getField( aRow ); - wxGridCellAttr* tmp; + wxGridCellAttr* attr = nullptr; switch( aCol ) { case FDC_NAME: if( field.IsMandatory() ) { - tmp = m_fieldNameAttr->Clone(); - tmp->SetReadOnly( true ); - return enhanceAttr( tmp, aRow, aCol, aKind ); + attr = m_fieldNameAttr->Clone(); + attr->SetReadOnly( true ); } else { m_fieldNameAttr->IncRef(); - return enhanceAttr( m_fieldNameAttr, aRow, aCol, aKind ); + attr = m_fieldNameAttr; } + break; + case FDC_VALUE: if( field.GetId() == FIELD_T::REFERENCE ) { m_referenceAttr->IncRef(); - return enhanceAttr( m_referenceAttr, aRow, aCol, aKind ); + attr = m_referenceAttr; } else if( field.GetId() == FIELD_T::VALUE ) { m_valueAttr->IncRef(); - return enhanceAttr( m_valueAttr, aRow, aCol, aKind ); + attr = m_valueAttr; } else if( field.GetId() == FIELD_T::FOOTPRINT ) { @@ -574,34 +587,34 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr:: if( m_part && m_part->IsPower() ) { m_readOnlyAttr->IncRef(); - return enhanceAttr( m_readOnlyAttr, aRow, aCol, aKind ); + attr = m_readOnlyAttr; } else { m_footprintAttr->IncRef(); - return enhanceAttr( m_footprintAttr, aRow, aCol, aKind ); + attr = m_footprintAttr; } } else if( field.GetId() == FIELD_T::DATASHEET ) { m_urlAttr->IncRef(); - return enhanceAttr( m_urlAttr, aRow, aCol, aKind ); + attr = m_urlAttr; } else if( field.GetId() == FIELD_T::SHEET_NAME ) { m_referenceAttr->IncRef(); - return enhanceAttr( m_referenceAttr, aRow, aCol, aKind ); + attr = m_referenceAttr; } else if( field.GetId() == FIELD_T::SHEET_FILENAME ) { m_filepathAttr->IncRef(); - return enhanceAttr( m_filepathAttr, aRow, aCol, aKind ); + attr = m_filepathAttr; } else if( ( m_parentType == SCH_LABEL_LOCATE_ANY_T ) && field.GetCanonicalName() == wxT( "Netclass" ) ) { m_netclassAttr->IncRef(); - return enhanceAttr( m_netclassAttr, aRow, aCol, aKind ); + attr = m_netclassAttr; } else { @@ -615,31 +628,36 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr:: if( ( templateFn && templateFn->m_URL ) || field.IsHypertext() ) { m_urlAttr->IncRef(); - return enhanceAttr( m_urlAttr, aRow, aCol, aKind ); + attr = m_urlAttr; } else { m_nonUrlAttr->IncRef(); - return enhanceAttr( m_nonUrlAttr, aRow, aCol, aKind ); + attr = m_nonUrlAttr; } } + break; + case FDC_TEXT_SIZE: case FDC_POSX: case FDC_POSY: - return enhanceAttr( nullptr, aRow, aCol, aKind ); + break; case FDC_H_ALIGN: m_hAlignAttr->IncRef(); - return enhanceAttr( m_hAlignAttr, aRow, aCol, aKind ); + attr = m_hAlignAttr; + break; case FDC_V_ALIGN: m_vAlignAttr->IncRef(); - return enhanceAttr( m_vAlignAttr, aRow, aCol, aKind ); + attr = m_vAlignAttr; + break; case FDC_ORIENTATION: m_orientationAttr->IncRef(); - return enhanceAttr( m_orientationAttr, aRow, aCol, aKind ); + attr = m_orientationAttr; + break; case FDC_SHOWN: case FDC_SHOW_NAME: @@ -648,20 +666,50 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr:: case FDC_ALLOW_AUTOPLACE: case FDC_PRIVATE: m_boolAttr->IncRef(); - return enhanceAttr( m_boolAttr, aRow, aCol, aKind ); + attr = m_boolAttr; + break; case FDC_FONT: m_fontAttr->IncRef(); - return enhanceAttr( m_fontAttr, aRow, aCol, aKind ); + attr = m_fontAttr; + break; case FDC_COLOR: m_colorAttr->IncRef(); - return enhanceAttr( m_colorAttr, aRow, aCol, aKind ); + attr = m_colorAttr; + break; default: - wxFAIL; - return enhanceAttr( nullptr, aRow, aCol, aKind ); + attr = nullptr; + break; } + + if( !attr ) + return nullptr; + + attr = enhanceAttr( attr, aRow, aCol, aKind ); + + if( IsInherited( aRow ) ) + { + wxGridCellAttr* text_attr = attr ? attr->Clone() : new wxGridCellAttr; + wxFont font; + + if( !text_attr->HasFont() ) + font = wxSystemSettings::GetFont( wxSYS_DEFAULT_GUI_FONT ); + else + font = text_attr->GetFont(); + + font.MakeItalic(); + text_attr->SetFont( font ); + text_attr->SetTextColour( wxSystemSettings::GetColour( wxSYS_COLOUR_GRAYTEXT ) ); + + if( attr ) + attr->DecRef(); + + attr = text_attr; + } + + return attr; } @@ -1053,11 +1101,56 @@ int FIELDS_GRID_TABLE::GetFieldRow( FIELD_T aFieldId ) return -1; } +void FIELDS_GRID_TABLE::AddInheritedField( const SCH_FIELD& aParent ) +{ + push_back( aParent ); + back().SetParent( m_part ); + m_isInherited.back() = true; + m_parentFields.back() = aParent; +} + +bool FIELDS_GRID_TABLE::EraseRow( size_t aRow ) +{ + if( m_isInherited.size() > aRow ) + { + // You can't erase inherited fields, but you can reset them to the parent value. + if( m_isInherited[aRow] ) + { + at( aRow ) = m_parentFields[aRow]; + return false; + } + + m_isInherited.erase( m_isInherited.begin() + aRow ); + } + + if( m_parentFields.size() > aRow ) + m_parentFields.erase( m_parentFields.begin() + aRow ); + + std::vector::erase( begin() + aRow ); + return true; +} + +void FIELDS_GRID_TABLE::SwapRows( size_t a, size_t b ) +{ + wxCHECK( a < this->size() && b < this->size(), /*void*/ ); + + std::swap( at( a ), at( b ) ); + + bool tmpInherited = m_isInherited[a]; + m_isInherited[a] = m_isInherited[b]; + m_isInherited[b] = tmpInherited; + + std::swap( m_parentFields[a], m_parentFields[b] ); +} + void FIELDS_GRID_TABLE::DetachFields() { for( SCH_FIELD& field : *this ) field.SetParent( nullptr ); + + for( SCH_FIELD& field : m_parentFields ) + field.SetParent( nullptr ); } diff --git a/eeschema/fields_grid_table.h b/eeschema/fields_grid_table.h index fe43d21966..ca83e1a390 100644 --- a/eeschema/fields_grid_table.h +++ b/eeschema/fields_grid_table.h @@ -28,6 +28,7 @@ #include #include #include +#include class SCH_BASE_FRAME; class DIALOG_SHIM; @@ -125,6 +126,34 @@ public: SCH_FIELD* GetField( FIELD_T aFieldId ); int GetFieldRow( FIELD_T aFieldId ); + void AddInheritedField( const SCH_FIELD& aParent ); + + void SetFieldInherited( size_t aRow, const SCH_FIELD& aParent ) + { + m_isInherited.resize( aRow + 1, false ); + m_parentFields.resize( aRow + 1 ); + m_parentFields[aRow] = aParent; + m_isInherited[aRow] = true; + } + + + bool IsInherited( size_t aRow ) const + { + if( aRow >= m_isInherited.size() || aRow >= m_parentFields.size() ) + return false; + + return m_isInherited[aRow] && m_parentFields[aRow].GetText() == at( aRow ).GetText(); + } + + const SCH_FIELD& ParentField( size_t row ) const { return m_parentFields[row]; } + + void push_back( const SCH_FIELD& field ); + // For std::vector compatibility, but we don't use it directly. + void emplace_back( const SCH_FIELD& field ) { push_back( field ); } + + bool EraseRow( size_t row ); + void SwapRows( size_t a, size_t b ); + void DetachFields(); protected: @@ -169,6 +198,9 @@ private: wxGridCellAttr* m_fontAttr; wxGridCellAttr* m_colorAttr; + std::vector m_isInherited; + std::vector m_parentFields; + std::unique_ptr m_eval; std::map< std::pair, wxString > m_evalOriginal; };