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

Clean up assumption that field_id < MANDATORY means its mandatory. 

It looks good, but non-mandatory fields have an ID of -1, so it
doesn't actually work.  Some places got around this by converting
the ID to unsigned, but this just hides the real issue from
unsuspecting coders.

Fixes https://gitlab.com/kicad/code/kicad/issues/4140
This commit is contained in:
Jeff Young 2020-04-01 15:00:40 +01:00
parent 029b1b0b22
commit 9a67dc56f9
11 changed files with 96 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
eeschema/class_libentry.cpp
View File

@ -584,7 +584,7 @@ void LIB_PART::RemoveDrawItem( LIB_ITEM* aItem )
// omitted when saving to disk.
if( aItem->Type() == LIB_FIELD_T )
{
if( static_cast<LIB_FIELD*>( aItem )->GetId() < MANDATORY_FIELDS )
if( static_cast<LIB_FIELD*>( aItem )->IsMandatory() )
return;
}
@ -845,7 +845,7 @@ void LIB_PART::GetFields( LIB_FIELDS& aList )
{
field = ( LIB_FIELD* ) &item;
if( (unsigned) field->GetId() < MANDATORY_FIELDS )
if( field->IsMandatory() )
continue; // was added above
aList.push_back( *field );

2
eeschema/dialogs/dialog_edit_component_in_lib.cpp
View File

@ -142,7 +142,7 @@ bool DIALOG_EDIT_COMPONENT_IN_LIBRARY::TransferDataToWindow()
if( !wxDialog::TransferDataToWindow() )
return false;
// Push a copy of each field into m_fields
// Push a copy of each field into m_updateFields
m_libEntry->GetFields( *m_fields );
// Copy the data sheet field from the old alias document file name if it's not empty.

2
eeschema/dialogs/dialog_edit_component_in_schematic.cpp
View File

@ -143,7 +143,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::TransferDataToWindow()
std::set<wxString> defined;
// Push a copy of each field into m_fields
// Push a copy of each field into m_updateFields
for( int i = 0; i < m_cmp->GetFieldCount(); ++i )
{
SCH_FIELD field( *m_cmp->GetField( i ) );

2
eeschema/dialogs/dialog_sch_sheet_props.cpp
View File

@ -120,7 +120,7 @@ bool DIALOG_SCH_SHEET_PROPS::TransferDataToWindow()
if( !wxDialog::TransferDataToWindow() )
return false;
// Push a copy of each field into m_fields
// Push a copy of each field into m_updateFields
for( SCH_FIELD& field : m_sheet->GetFields() )
{
SCH_FIELD field_copy( field );

136
eeschema/dialogs/dialog_update_fields.cpp
View File

@ -57,12 +57,12 @@ bool DIALOG_UPDATE_FIELDS::TransferDataFromWindow()
// Create the set of fields to be updated
m_fields.clear();
m_updateFields.clear();
for( unsigned i = 0; i < m_fieldsBox->GetCount(); ++i )
{
if( m_fieldsBox->IsChecked( i ) )
m_fields.insert( m_fieldsBox->GetString( i ) );
m_updateFields.insert( m_fieldsBox->GetString( i ) );
}
@ -111,7 +111,7 @@ bool DIALOG_UPDATE_FIELDS::TransferDataToWindow()
const LIB_FIELD* field = static_cast<const LIB_FIELD*>( &( *it ) );
if( field->GetId() >= MANDATORY_FIELDS )
m_fields.insert( field->GetName() );
m_updateFields.insert( field->GetName() );
}
}
}
@ -127,9 +127,9 @@ bool DIALOG_UPDATE_FIELDS::TransferDataToWindow()
m_fieldsBox->Check( i, true );
}
for( const auto& field : m_fields )
for( const wxString& fieldName : m_updateFields )
{
int idx = m_fieldsBox->Append( field );
int idx = m_fieldsBox->Append( fieldName );
m_fieldsBox->Check( idx, true );
}
@ -142,7 +142,6 @@ bool DIALOG_UPDATE_FIELDS::TransferDataToWindow()
void DIALOG_UPDATE_FIELDS::updateFields( SCH_COMPONENT* aComponent )
{
std::vector<SCH_FIELD*> oldFields;
SCH_FIELDS newFields;
std::unique_ptr< LIB_PART >& libPart = aComponent->GetPartRef();
@ -152,79 +151,100 @@ void DIALOG_UPDATE_FIELDS::updateFields( SCH_COMPONENT* aComponent )
LIB_PART* alias = m_frame->GetLibPart( aComponent->GetLibId() );
aComponent->GetFields( oldFields, false );
for( auto compField : oldFields )
for( const SCH_FIELD& existingField : aComponent->GetFields() )
{
if( existingField.GetId() >= 0 && existingField.GetId() < MANDATORY_FIELDS )
{
newFields.push_back( existingField );
continue;
}
// If requested, transfer only fields that occur also in the original library part
// and obviously mandatory fields
if( compField->GetId() < MANDATORY_FIELDS
|| !m_removeExtraBox->IsChecked()
|| libPart->FindField( compField->GetName() ) )
newFields.push_back( *compField );
if( m_removeExtraBox->IsChecked() && !libPart->FindField( existingField.GetName() ) )
continue;
newFields.push_back( existingField );
}
// Update the requested field values
for( const auto& partField : m_fields )
// Update the requested fields
for( const wxString& fieldName : m_updateFields )
{
LIB_FIELD* libField = libPart->FindField( partField );
LIB_FIELD* libField = libPart->FindField( fieldName );
if( !libField )
continue;
SCH_FIELD* field = nullptr;
auto it = std::find_if( newFields.begin(), newFields.end(), [&] ( const SCH_FIELD& f )
{ return f.GetName() == partField; } );
auto it = std::find_if( newFields.begin(), newFields.end(),
[&] ( const SCH_FIELD& f )
{
return f.GetName() == fieldName;
} );
if( it != newFields.end() )
{
field = &*it;
SCH_FIELD* newField = &*it;
wxString fieldValue = libField->GetText();
if( alias )
{
if( fieldName == TEMPLATE_FIELDNAME::GetDefaultFieldName( VALUE ) )
fieldValue = alias->GetName();
else if( fieldName == TEMPLATE_FIELDNAME::GetDefaultFieldName( DATASHEET ) )
fieldValue = alias->GetDocFileName();
}
if( fieldValue.IsEmpty() )
{
// If the library field is empty an update would clear an existing entry.
// Check if this is the desired behavior.
if( m_resetEmpty->IsChecked() )
newField->SetText( wxEmptyString );
}
else
{
newField->SetText( fieldValue );
}
if( m_resetVisibility->IsChecked() )
{
newField->SetVisible( libField->IsVisible() );
}
if( m_resetPosition->IsChecked() )
{
newField->SetTextAngle( libField->GetTextAngle() );
// Schematic fields are schematic-relative; symbol editor fields are component-relative
if( m_createUndo )
newField->SetTextPos( libField->GetTextPos() + aComponent->GetPosition() );
else
newField->SetTextPos( libField->GetTextPos() );
}
if( m_resetSizeAndStyle->IsChecked() )
{
newField->SetHorizJustify( libField->GetHorizJustify() );
newField->SetVertJustify( libField->GetVertJustify() );
newField->SetTextSize( libField->GetTextSize() );
newField->SetItalic( libField->IsItalic() );
newField->SetBold( libField->IsBold() );
}
}
else
{
// Missing field, it has to be added to the component
SCH_FIELD f( wxPoint( 0, 0 ), newFields.size(), aComponent, partField );
newFields.push_back( f );
field = &newFields.back();
}
SCH_FIELD newField( wxPoint( 0, 0 ), newFields.size(), aComponent, fieldName );
wxString fieldValue = libField->GetText();
newField.ImportValues( *libField );
newField.SetText( libField->GetText() );
if( alias )
{
if( partField == TEMPLATE_FIELDNAME::GetDefaultFieldName( VALUE ) )
fieldValue = alias->GetName();
else if( partField == TEMPLATE_FIELDNAME::GetDefaultFieldName( DATASHEET ) )
fieldValue = alias->GetDocFileName();
}
// If the library field is empty an update would clear an existing entry.
// Check if this is the desired behavior.
if( !fieldValue.empty() || m_resetEmpty->IsChecked() )
field->SetText( fieldValue );
if( m_resetVisibility->IsChecked() )
field->SetVisible( libField->IsVisible() );
if( m_resetPosition->IsChecked() )
{
field->SetTextAngle( libField->GetTextAngle() );
// Board fields are board-relative; symbol editor fields are component-relative
// Schematic fields are schematic-relative; symbol editor fields are component-relative
if( m_createUndo )
field->SetTextPos( libField->GetTextPos() + aComponent->GetPosition() );
newField.SetTextPos( libField->GetTextPos() + aComponent->GetPosition() );
else
field->SetTextPos( libField->GetTextPos() );
}
newField.SetTextPos( libField->GetTextPos() );
if( m_resetSizeAndStyle->IsChecked() )
{
field->SetHorizJustify( libField->GetHorizJustify() );
field->SetVertJustify( libField->GetVertJustify() );
field->SetTextSize( libField->GetTextSize() );
field->SetItalic( libField->IsItalic() );
field->SetBold( libField->IsBold() );
newFields.push_back( newField );
}
}

2
eeschema/dialogs/dialog_update_fields.h
View File

@ -70,7 +70,7 @@ private:
SCH_EDIT_FRAME* m_frame;
///> Set of field names that should have values updated
set<wxString> m_fields;
set<wxString> m_updateFields;
///> Components to update
list<SCH_COMPONENT*> m_components;

15
eeschema/lib_field.cpp
View File

@ -386,10 +386,10 @@ COLOR4D LIB_FIELD::GetDefaultColor()
wxString LIB_FIELD::GetName( bool aUseDefaultName ) const
{
if( !m_name.IsEmpty() )
return m_name;
else if( aUseDefaultName )
if( m_name.IsEmpty() && aUseDefaultName )
return TEMPLATE_FIELDNAME::GetDefaultFieldName( m_id );
return m_name;
}
@ -410,12 +410,9 @@ wxString LIB_FIELD::GetCanonicalName() const
void LIB_FIELD::SetName( const wxString& aName )
{
// Mandatory field names are fixed.
// So what? Why should the low level code be in charge of such a policy issue?
// Besides, m_id is a relic that is untrustworthy now.
if( m_id >=0 && m_id < MANDATORY_FIELDS )
if( IsMandatory() )
{
DBG(printf( "trying to set a MANDATORY_FIELD's name\n" );)
wxFAIL_MSG( "trying to set a MANDATORY_FIELD's name\n" );
return;
}
@ -479,5 +476,5 @@ BITMAP_DEF LIB_FIELD::GetMenuImage() const
bool LIB_FIELD::IsMandatory() const
{
return m_id < MANDATORY_FIELDS;
return m_id >= 0 && m_id < MANDATORY_FIELDS;
}

2
eeschema/sch_component.cpp
View File

@ -812,7 +812,7 @@ void SCH_COMPONENT::UpdateFields( bool aResetStyle, bool aResetRef )
if( idx == REFERENCE && !aResetRef )
continue;
if( (unsigned) idx < MANDATORY_FIELDS )
if( idx >= 0 && idx < MANDATORY_FIELDS )
schField = GetField( idx );
else
{

4
eeschema/sch_legacy_plugin.cpp
View File

@ -2982,7 +2982,7 @@ void SCH_LEGACY_PLUGIN_CACHE::loadField( std::unique_ptr<LIB_PART>& aPart,
LIB_FIELD* field;
if( (unsigned) id < MANDATORY_FIELDS )
if( id >= 0 && id < MANDATORY_FIELDS )
{
field = aPart->GetField( id );
@ -3096,7 +3096,7 @@ void SCH_LEGACY_PLUGIN_CACHE::loadField( std::unique_ptr<LIB_PART>& aPart,
}
// Fields in RAM must always have names.
if( (unsigned) id < MANDATORY_FIELDS )
if( id >= 0 && id < MANDATORY_FIELDS )
{
// Fields in RAM must always have names, because we are trying to get
// less dependent on field ids and more dependent on names.

2
eeschema/sch_sexpr_plugin.cpp
View File

@ -945,7 +945,7 @@ void SCH_SEXPR_PLUGIN::saveField( SCH_FIELD* aField, int aNestLevel )
wxString fieldName;
if( aField->GetId() < MANDATORY_FIELDS )
if( aField->GetId() >= 0 && aField->GetId() < MANDATORY_FIELDS )
fieldName = "ki_" + TEMPLATE_FIELDNAME::GetDefaultFieldName( aField->GetId() ).Lower();
else
fieldName = aField->GetName();

8
eeschema/sch_view.cpp
View File

@ -142,9 +142,7 @@ void SCH_VIEW::DisplayComponent( LIB_PART* aPart )
if( item.Type() != LIB_FIELD_T )
continue;
LIB_FIELD* field = static_cast< LIB_FIELD* >( &item );
if( field->GetId() < MANDATORY_FIELDS )
if( static_cast< LIB_FIELD* >( &item )->IsMandatory() )
Add( &item );
}
@ -163,9 +161,7 @@ void SCH_VIEW::DisplayComponent( LIB_PART* aPart )
// The mandatory fields are already in place so we only add user defined fields.
if( item.Type() == LIB_FIELD_T )
{
LIB_FIELD* field = static_cast< LIB_FIELD* >( &item );
if( field->GetId() < MANDATORY_FIELDS )
if( static_cast< LIB_FIELD* >( &item )->IsMandatory() )
continue;
}