← Back to team overview

kicad-developers team mailing list archive

[PATCH] Bugfix for component table

 

Bug noted here - https://lists.launchpad.net/kicad-developers/msg29485.html

Patch attached to this email fixes glitch when user adds custom field with
same name as a default field.

Users can now do this to their heart's content.
From 43106de69511427cf841700be10da2d42ce8b6dd Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walters@xxxxxxxxx>
Date: Tue, 23 May 2017 00:05:36 +1000
Subject: [PATCH] Fixed duplicate field names

- Now works correctly even if users overload default field names
---
 eeschema/bom_table_model.cpp | 32 ++++++++++++++++++++++++++------
 eeschema/sch_component.cpp   | 31 +++++++++++++++++++++----------
 eeschema/sch_component.h     |  4 ++--
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/eeschema/bom_table_model.cpp b/eeschema/bom_table_model.cpp
index 550b561..e2150d4 100644
--- a/eeschema/bom_table_model.cpp
+++ b/eeschema/bom_table_model.cpp
@@ -569,7 +569,7 @@ bool BOM_TABLE_COMPONENT::AddUnit( SCH_REFERENCE aUnit )
 
             // User fields
             default:
-                value = cmp->GetFieldText( column->Title() );
+                value = cmp->GetFieldText( column->Title(), false );
                 break;
             }
 
@@ -705,7 +705,8 @@ void BOM_TABLE_COMPONENT::ApplyFieldChanges()
                     field = cmp->GetField( DATASHEET );
                     break;
                 default:
-                    field = cmp->FindField( column->Title() );
+                    // Find the field by name (but ignore default fields)
+                    field = cmp->FindField( column->Title(), false );
                     break;
                 }
 
@@ -985,12 +986,31 @@ void BOM_TABLE_MODEL::AddComponentFields( SCH_COMPONENT* aCmp )
 
         fieldName = field->GetName();
 
-        auto existing = ColumnList.GetColumnByTitle( fieldName );
+        bool userMatchFound = false;
 
-        // As columns are sorted by ID, we can allow user to
-        // create a column with a "special" name
-        if( existing && existing->Id() >= MANDATORY_FIELDS)
+        // Search for the column within the existing columns
+        for( auto col : ColumnList.Columns )
+        {
+            if( !col )
+            {
+                continue;
+            }
+
+            if( col->Title().Cmp( fieldName ) == 0 )
+            {
+                if( col->Id() >= BOM_COL_ID_USER )
+                {
+                    userMatchFound = true;
+                    break;
+                }
+            }
+        }
+
+        // If a user-made column already exists with the same name, abort
+        if( userMatchFound )
+        {
             continue;
+        }
 
         ColumnList.AddColumn( new BOM_COLUMN( ColumnList.NextFieldId(),
                                               BOM_COL_TYPE_USER,
diff --git a/eeschema/sch_component.cpp b/eeschema/sch_component.cpp
index 8c9f64b..edf8e46 100644
--- a/eeschema/sch_component.cpp
+++ b/eeschema/sch_component.cpp
@@ -769,20 +769,23 @@ SCH_FIELD* SCH_COMPONENT::GetField( int aFieldNdx ) const
     return (SCH_FIELD*) field;
 }
 
-wxString SCH_COMPONENT::GetFieldText( wxString aFieldName ) const
+wxString SCH_COMPONENT::GetFieldText( wxString aFieldName, bool aIncludeDefaultFields ) const
 {
-
     // Field name for comparison
     wxString cmpFieldName;
 
-    // Default field names
-    for ( unsigned int i=0; i<MANDATORY_FIELDS; i++)
+    if( aIncludeDefaultFields )
     {
-        cmpFieldName = TEMPLATE_FIELDNAME::GetDefaultFieldName( i );
 
-        if( cmpFieldName.Cmp( aFieldName ) == 0 )
+        // Default field names
+        for ( unsigned int i=0; i<MANDATORY_FIELDS; i++)
         {
-            return m_Fields[i].GetText();
+            cmpFieldName = TEMPLATE_FIELDNAME::GetDefaultFieldName( i );
+
+            if( cmpFieldName.Cmp( aFieldName ) == 0 )
+            {
+                return m_Fields[i].GetText();
+            }
         }
     }
 
@@ -819,13 +822,21 @@ SCH_FIELD* SCH_COMPONENT::AddField( const SCH_FIELD& aField )
     return &m_Fields[newNdx];
 }
 
-
-SCH_FIELD* SCH_COMPONENT::FindField( const wxString& aFieldName )
+/*
+ * Find and return compnent field with the given name
+ * @aFieldName is the name of the field to search for
+ * @aIncludeDefaultFields excludes default fields from search if set to false
+ */
+SCH_FIELD* SCH_COMPONENT::FindField( const wxString& aFieldName, bool aIncludeDefaultFields )
 {
-    for( unsigned i = 0;  i<m_Fields.size();  ++i )
+    unsigned start = aIncludeDefaultFields ? 0 : MANDATORY_FIELDS;
+
+    for( unsigned i = start;  i<m_Fields.size();  ++i )
     {
         if( aFieldName == m_Fields[i].GetName( false ) )
+        {
             return &m_Fields[i];
+        }
     }
 
     return NULL;
diff --git a/eeschema/sch_component.h b/eeschema/sch_component.h
index 958b845..45db94e 100644
--- a/eeschema/sch_component.h
+++ b/eeschema/sch_component.h
@@ -302,7 +302,7 @@ public:
      * Returns text associated with a given field (if such a field exists)
      * @aFieldName is the name of the field
      */
-    wxString GetFieldText( wxString aFieldName ) const;
+    wxString GetFieldText( wxString aFieldName, bool aIncludeDefaultFields = true ) const;
 
     /**
      * Function GetFields
@@ -325,7 +325,7 @@ public:
      * Function FindField
      * searches for SCH_FIELD with \a aFieldName and returns it if found, else NULL.
      */
-    SCH_FIELD* FindField( const wxString& aFieldName );
+    SCH_FIELD* FindField( const wxString& aFieldName, bool aIncludeDefaultFields = true );
 
     void SetFields( const SCH_FIELDS& aFields )
     {
-- 
2.7.4


Follow ups