Eeschema: minor s-expression file format improvements.

I could not find a reasonable solution to the mandatory field canonical
name issue so field indices are back in play as much as I did not want
to use them.  They really only have meaning for the mandatory fields.
For all other fields, the index number is meaningless and the field name
is the primary means for searching.

Fix a broken field size bug in the symbol library formatter.

eeschema/sch_sexpr_parser.cpp

@@ -619,7 +619,8 @@ void SCH_SEXPR_PARSER::parseHeader( TSCHEMATIC_T::T aHeaderType, int aFileVersio
 void SCH_SEXPR_PARSER::parsePinNames( std::unique_ptr<LIB_PART>& aSymbol )
 {
     wxCHECK_RET( CurTok() == T_pin_names,
-                 wxT( "Cannot parse " ) + GetTokenString( CurTok() ) + wxT( " as a pin_name token." ) );
+                 wxT( "Cannot parse " ) + GetTokenString( CurTok() ) +
+                 wxT( " as a pin_name token." ) );
 
     wxString error;
 
@@ -661,7 +662,7 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
     wxString error;
     wxString name;
     wxString value;
-    std::unique_ptr<LIB_FIELD> tmp( new LIB_FIELD( MANDATORY_FIELDS ) );
+    std::unique_ptr<LIB_FIELD> field( new LIB_FIELD( MANDATORY_FIELDS ) );
 
     T token = NextTok();
 
@@ -681,6 +682,7 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
         THROW_IO_ERROR( error );
     }
 
+    field->SetName( name );
     token = NextTok();
 
     if( !IsSymbol( token ) )
@@ -693,39 +695,61 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
     // Empty property values are valid.
     value = FromUTF8();
 
-    LIB_FIELD* field;
+    field->SetText( value );
 
-    if( name == "ki_reference" )
-    {
-        field = &aSymbol->GetReferenceField();
-        field->SetText( value );
-    }
-    else if( name == "ki_value" )
-    {
-        field = &aSymbol->GetValueField();
-        field->SetText( value );
-    }
-    else if( name == "ki_footprint" )
+    for( token = NextTok();  token != T_RIGHT;  token = NextTok() )
     {
-        field = &aSymbol->GetFootprintField();
-        field->SetText( value );
+        if( token != T_LEFT )
+            Expecting( T_LEFT );
+
+        token = NextTok();
+
+        switch( token )
+        {
+        case T_id:
+            field->SetId( parseInt( "field ID" ) );
+            NeedRIGHT();
+            break;
+
+        case T_at:
+            field->SetPosition( parseXY() );
+            field->SetTextAngle( static_cast<int>( parseDouble( "text angle" ) * 10.0 ) );
+            NeedRIGHT();
+            break;
+
+        case T_effects:
+            parseEDA_TEXT( static_cast<EDA_TEXT*>( field.get() ) );
+            break;
+
+        default:
+            Expecting( "id, at or effects" );
+        }
     }
-    else if( name == "ki_datasheet" )
+
+    LIB_FIELD* existingField;
+
+    if( field->GetId() < MANDATORY_FIELDS )
     {
-        field = aSymbol->GetField( DATASHEET );
-        aSymbol->SetDocFileName( value );
+        existingField = aSymbol->GetField( field->GetId() );
+
+        /// @todo Remove this once the legacy file format is deprecated.
+        if( field->GetId() == DATASHEET )
+        {
+            aSymbol->SetDocFileName( value );
+            field->SetText( wxEmptyString );
+        }
+
+        *existingField = *field;
     }
     else if( name == "ki_keywords" )
     {
         // Not a LIB_FIELD object yet.
         aSymbol->SetKeyWords( value );
-        field = tmp.get();
     }
     else if( name == "ki_description" )
     {
         // Not a LIB_FIELD object yet.
         aSymbol->SetDescription( value );
-        field = tmp.get();
     }
     else if( name == "ki_fp_filters" )
     {
@@ -737,48 +761,24 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
             filters.Add( tokenizer.GetNextToken() );
 
         aSymbol->SetFootprintFilters( filters );
-        field = tmp.get();
     }
     else if( name == "ki_locked" )
     {
+        // This is a temporary LIB_FIELD object until interchangeable units are determined on
+        // the fly.
         aSymbol->LockUnits( true );
-        field = tmp.get();
     }
     else
     {
-        field = aSymbol->FindField( name );
+        existingField = aSymbol->GetField( field->GetId() );
 
-        if( !field )
+        if( !existingField )
         {
-            field = new LIB_FIELD( m_fieldId, name );
-            aSymbol->AddDrawItem( field );
-            m_fieldId += 1;
+            aSymbol->AddDrawItem( field.release() );
         }
-
-        field->SetText( value );
-    }
-
-    for( token = NextTok();  token != T_RIGHT;  token = NextTok() )
-    {
-        if( token != T_LEFT )
-            Expecting( T_LEFT );
-
-        token = NextTok();
-
-        switch( token )
+        else
         {
-        case T_at:
-            field->SetPosition( parseXY() );
-            field->SetTextAngle( static_cast<int>( parseDouble( "text angle" ) * 10.0 ) );
-            NeedRIGHT();
-            break;
-
-        case T_effects:
-            parseEDA_TEXT( static_cast<EDA_TEXT*>( field ) );
-            break;
-
-        default:
-            Expecting( "at or effects" );
+            *existingField = *field;
         }
     }
 }
@@ -1646,6 +1646,11 @@ SCH_FIELD* SCH_SEXPR_PARSER::parseSchField()
 
         switch( token )
         {
+        case T_id:
+            field->SetId( parseInt( "field ID" ) );
+            NeedRIGHT();
+            break;
+
         case T_at:
             field->SetPosition( parseXY() );
             field->SetTextAngle( static_cast<int>( parseDouble( "text angle" ) * 10.0 ) );
@@ -1984,46 +1989,23 @@ SCH_COMPONENT* SCH_SEXPR_PARSER::parseSchematicSymbol()
 
         case T_property:
         {
-            int fieldIndex;
-
             field = parseSchField();
             field->SetParent( symbol.get() );
 
-            if( field->GetName() == "ki_reference" )
+            if( field->GetId() == REFERENCE )
             {
-                fieldIndex = REFERENCE;
-                field->SetId( REFERENCE );
                 field->SetLayer( LAYER_REFERENCEPART );
-                field->SetName( TEMPLATE_FIELDNAME::GetDefaultFieldName( REFERENCE ) );
             }
-            else if( field->GetName() == "ki_value" )
+            else if( field->GetId() == VALUE )
             {
-                fieldIndex = VALUE;
-                field->SetId( VALUE );
                 field->SetLayer( LAYER_VALUEPART );
-                field->SetName( TEMPLATE_FIELDNAME::GetDefaultFieldName( VALUE ) );
-            }
-            else if( field->GetName() == "ki_footprint" )
-            {
-                fieldIndex = FOOTPRINT;
-                field->SetId( FOOTPRINT );
-                field->SetName( TEMPLATE_FIELDNAME::GetDefaultFieldName( FOOTPRINT ) );
-            }
-            else if( field->GetName() == "ki_datasheet" )
-            {
-                fieldIndex = DATASHEET;
-                field->SetId( DATASHEET );
-                field->SetName( TEMPLATE_FIELDNAME::GetDefaultFieldName( DATASHEET ) );
             }
-            else
+            else if( field->GetId() >= MANDATORY_FIELDS )
             {
-                fieldIndex = m_fieldId;
-                field->SetId( m_fieldId );
                 symbol->AddField( *field );
-                m_fieldId += 1;
             }
 
-            *symbol->GetField( fieldIndex ) = *field;
+            *symbol->GetField( field->GetId() ) = *field;
             delete field;
             break;
         }

eeschema/sch_sexpr_plugin.cpp

@@ -1440,7 +1440,7 @@ void SCH_SEXPR_PLUGIN_CACHE::SaveSymbol( LIB_PART* aSymbol, OUTPUTFORMATTER& aFo
             lastFieldId += 1;
         }
 
-        saveDcmInfoAsFields( aSymbol, aFormatter, aNestLevel, fields.back().GetId() + 1 );
+        saveDcmInfoAsFields( aSymbol, aFormatter, aNestLevel, lastFieldId );
 
         // Save the draw items grouped by units.
         std::vector<PART_UNITS> units = aSymbol->GetUnitDrawItems();
@@ -1741,21 +1741,15 @@ void SCH_SEXPR_PLUGIN_CACHE::saveField( LIB_FIELD* aField,
 {
     wxCHECK_RET( aField && aField->Type() == LIB_FIELD_T, "Invalid LIB_FIELD object." );
 
-    wxString fieldName = aField->GetCanonicalName();
-
-    // When saving legacy fields, prefix the field name with "ki_" to prevent name clashes
-    // with exisiting user defined fields.
-    if( aField->IsMandatory() && !fieldName.StartsWith( "ki_" ) )
-        fieldName = "ki_" + fieldName.Lower();
-
-    aFormatter.Print( aNestLevel, "(property %s %s (at %s %s %g)",
-                      aFormatter.Quotew( fieldName ).c_str(),
+    aFormatter.Print( aNestLevel, "(property %s %s (id %d) (at %s %s %g)",
+                      aFormatter.Quotew( aField->GetName() ).c_str(),
                       aFormatter.Quotew( aField->GetText() ).c_str(),
+                      aField->GetId(),
                       FormatInternalUnits( aField->GetPosition().x ).c_str(),
                       FormatInternalUnits( aField->GetPosition().y ).c_str(),
                       static_cast<double>( aField->GetTextAngle() ) / 10.0 );
 
-    if( aField->IsDefaultFormatting() )
+    if( aField->IsDefaultFormatting() && ( aField->GetTextHeight() == GetDefaultTextSize() ) )
     {
         aFormatter.Print( 0, ")\n" );           // Close property token if no font effects.
     }
@@ -1784,30 +1778,47 @@ void SCH_SEXPR_PLUGIN_CACHE::savePin( LIB_PIN* aPin,
                       FormatAngle( getPinAngle( aPin->GetOrientation() ) * 10.0 ).c_str(),
                       FormatInternalUnits( aPin->GetLength() ).c_str() );
 
-    aFormatter.Print( 0, " (name %s",
-                      aFormatter.Quotew( aPin->GetName() ).c_str() );
+    int nestLevel = 0;
 
-    // This follows the EDA_TEXT effects formatting for future expansion.
-    if( aPin->GetNameTextSize() != Mils2iu( DEFAULTPINNAMESIZE ) )
-        aFormatter.Print( 0, " (effects (font (size %s %s)))",
-                          FormatInternalUnits( aPin->GetNameTextSize() ).c_str(),
-                          FormatInternalUnits( aPin->GetNameTextSize() ).c_str() );
+    if( aPin->GetNameTextSize() != Mils2iu( DEFAULTPINNAMESIZE )
+      || aPin->GetNumberTextSize() != Mils2iu( DEFAULTPINNUMSIZE ) )
+    {
+        aFormatter.Print( 0, "\n" );
+        aFormatter.Print( aNestLevel + 1, "(name %s",
+                          aFormatter.Quotew( aPin->GetName() ).c_str() );
 
-    aFormatter.Print( 0, ")" );
-    aFormatter.Print( 0, " (number %s",
-                      aFormatter.Quotew( aPin->GetNumber() ).c_str() );
+        // This follows the EDA_TEXT effects formatting for future expansion.
+        if( aPin->GetNameTextSize() != Mils2iu( DEFAULTPINNAMESIZE ) )
+            aFormatter.Print( 0, " (effects (font (size %s %s)))",
+                              FormatInternalUnits( aPin->GetNameTextSize() ).c_str(),
+                              FormatInternalUnits( aPin->GetNameTextSize() ).c_str() );
 
-    // This follows the EDA_TEXT effects formatting for future expansion.
-    if( aPin->GetNumberTextSize() != Mils2iu( DEFAULTPINNUMSIZE ) )
-        aFormatter.Print( 0, " (effects (font (size %s %s)))",
-                          FormatInternalUnits( aPin->GetNumberTextSize() ).c_str(),
-                          FormatInternalUnits( aPin->GetNumberTextSize() ).c_str() );
-    aFormatter.Print( 0, ")" );
+        aFormatter.Print( 0, ")\n" );
+        aFormatter.Print( aNestLevel + 1, "(number %s",
+                          aFormatter.Quotew( aPin->GetNumber() ).c_str() );
+
+        // This follows the EDA_TEXT effects formatting for future expansion.
+        if( aPin->GetNumberTextSize() != Mils2iu( DEFAULTPINNUMSIZE ) )
+            aFormatter.Print( 0, " (effects (font (size %s %s)))",
+                              FormatInternalUnits( aPin->GetNumberTextSize() ).c_str(),
+                              FormatInternalUnits( aPin->GetNumberTextSize() ).c_str() );
+        aFormatter.Print( 0, ")\n" );
+        nestLevel = aNestLevel + 1;
+    }
+    else
+    {
+        aFormatter.Print( 0, " (name %s) (number %s)",
+                          aFormatter.Quotew( aPin->GetName() ).c_str(),
+                          aFormatter.Quotew( aPin->GetNumber() ).c_str() );
+    }
 
     if( !aPin->IsVisible() )
-        aFormatter.Print( 0, " hide" );
+        aFormatter.Print( nestLevel, " hide" );
 
-    aFormatter.Print( 0, ")\n" );
+    if( nestLevel )
+        nestLevel -= 1;
+
+    aFormatter.Print( nestLevel, ")\n" );
 }
 
 

eeschema/schematic.keywords

@@ -35,6 +35,7 @@ hide
 hierarchical_label
 hint_alt_swap
 hint_pin_swap
+id
 image
 input
 input_low