Reconcile new Sheet Fields and new S-expr file format.

Fixes https://gitlab.com/kicad/code/kicad/issues/4327

eeschema/sch_field.cpp

@@ -51,14 +51,13 @@
 SCH_FIELD::SCH_FIELD( const wxPoint& aPos, int aFieldId, SCH_ITEM* aParent,
                       const wxString& aName ) :
     SCH_ITEM( aParent, SCH_FIELD_T ),
-    EDA_TEXT( wxEmptyString )
+    EDA_TEXT( wxEmptyString ),
+    m_id( 0 ),
+    m_name( aName )
 {
     SetTextPos( aPos );
-    m_id = aFieldId;
+    SetId( aFieldId );  // will also set the layer
-    m_name = aName;
-
     SetVisible( false );
-    SetLayer( LAYER_FIELDS );
 }
 
 
@@ -73,6 +72,32 @@ EDA_ITEM* SCH_FIELD::Clone() const
 }
 
 
+void SCH_FIELD::SetId( int aId )
+{
+    m_id = aId;
+
+    if( m_Parent && m_Parent->Type() == SCH_SHEET_T )
+    {
+        switch( m_id )
+        {
+        case SHEETNAME:     SetLayer( LAYER_SHEETNAME );     break;
+        case SHEETFILENAME: SetLayer( LAYER_SHEETFILENAME ); break;
+        default:            SetLayer( LAYER_SHEETFIELDS );   break;
+        }
+    }
+    else
+    {
+        switch( m_id )
+        {
+        case REFERENCE:     SetLayer( LAYER_REFERENCEPART ); break;
+        case VALUE:         SetLayer( LAYER_VALUEPART );     break;
+        default:            SetLayer( LAYER_FIELDS );        break;
+        }
+    }
+
+}
+
+
 wxString SCH_FIELD::GetShownText( int aDepth ) const
 {
     std::function<bool( wxString* )> symbolResolver =

eeschema/sch_field.h

@@ -113,7 +113,7 @@ public:
 
     int GetId() const { return m_id; }
 
-    void SetId( int aId ) { m_id = aId; }
+    void SetId( int aId );
 
     wxString GetShownText( int aDepth = 0 ) const override;
 

eeschema/sch_file_versions.h

@@ -37,4 +37,6 @@
 /**
  * Symbol library file version.
  */
-#define SEXPR_SCHEMATIC_FILE_VERSION   20200310  // Initial version.
+//#define SEXPR_SCHEMATIC_FILE_VERSION 20200310  // Initial version.  Sheet fields were named
+                                                 // incorectly (using symbol field vocabulary).
+#define SEXPR_SCHEMATIC_FILE_VERSION   20200506

eeschema/sch_legacy_plugin.cpp

@@ -1666,7 +1666,7 @@ SCH_COMPONENT* SCH_LEGACY_PLUGIN::loadComponent( LINE_READER& aReader )
                 // We freely renumber the index to fit the next available field slot.
                 index = component->GetFieldCount();  // new has this index after insertion
 
-                SCH_FIELD field( wxPoint( 0, 0 ), -1, component.get(), name );
+                SCH_FIELD field( wxPoint( 0, 0 ), index, component.get(), name );
                 component->AddField( field );
             }
 

eeschema/sch_sexpr_parser.cpp

@@ -1633,7 +1633,7 @@ void SCH_SEXPR_PARSER::parseTITLE_BLOCK( TITLE_BLOCK& aTitleBlock )
 }
 
 
-SCH_FIELD* SCH_SEXPR_PARSER::parseSchField( SCH_COMPONENT* aParentSymbol )
+SCH_FIELD* SCH_SEXPR_PARSER::parseSchField( SCH_ITEM* aParent )
 {
     wxCHECK_MSG( CurTok() == T_property, nullptr,
                  wxT( "Cannot parse " ) + GetTokenString( CurTok() ) +
@@ -1673,8 +1673,7 @@ SCH_FIELD* SCH_SEXPR_PARSER::parseSchField( SCH_COMPONENT* aParentSymbol )
     // Empty property values are valid.
     value = FromUTF8();
 
-    std::unique_ptr<SCH_FIELD> field( new SCH_FIELD( wxDefaultPosition, MANDATORY_FIELDS,
+    std::unique_ptr<SCH_FIELD> field( new SCH_FIELD( wxDefaultPosition, -1, aParent, name ) );
-                                                     aParentSymbol, name ) );
 
     field->SetText( value );
     field->SetVisible( true );
@@ -2112,21 +2111,10 @@ SCH_COMPONENT* SCH_SEXPR_PARSER::parseSchematicSymbol()
             // the field positions are set.
             field = parseSchField( symbol.get() );
 
-            if( field->GetId() == REFERENCE )
-            {
-                field->SetLayer( LAYER_REFERENCEPART );
-            }
-            else if( field->GetId() == VALUE )
-            {
-                field->SetLayer( LAYER_VALUEPART );
-            }
-            else if( field->GetId() >= MANDATORY_FIELDS )
-            {
-                symbol->AddField( *field );
-            }
-
             if( symbol->GetField( field->GetId() ) )
                 *symbol->GetField( field->GetId() ) = *field;
+            else
+                symbol->AddField( *field );
 
             delete field;
             break;
@@ -2262,25 +2250,19 @@ SCH_SHEET* SCH_SEXPR_PARSER::parseSheet()
             break;
 
         case T_property:
-            field = parseSchField( nullptr );
+            field = parseSchField( sheet.get() );
 
-            if( field->GetName() == "ki_sheet_name" )
+            if( m_requiredVersion <= 20200310 )
             {
-                field->SetId( SHEETNAME );
+                // Earlier versions had the wrong ids (and names) saved for sheet fields.
-                field->SetName( SCH_SHEET::GetDefaultFieldName( SHEETNAME ) );
+                // Fortunately they only saved the sheetname and sheetfilepath (and always
-            }
+                // in that order), so we can hack in a recovery.
-            else if( field->GetName() == "ki_sheet_file" )
+                if( fields.empty() )
-            {
+                    field->SetId( SHEETNAME );
-                field->SetId( SHEETFILENAME );
+                else
-                field->SetName( SCH_SHEET::GetDefaultFieldName( SHEETFILENAME ) );
+                    field->SetId( SHEETFILENAME );
-            }
-            else
-            {
-                field->SetId( m_fieldId );
-                m_fieldId += 1;
             }
 
-            field->SetParent( sheet.get() );
             fields.emplace_back( *field );
             delete field;
             break;

eeschema/sch_sexpr_parser.h

@@ -206,7 +206,7 @@ class SCH_SEXPR_PARSER : public SCHEMATIC_LEXER
     void parseSchSymbolInstances( SCH_SCREEN* aScreen );
 
     SCH_SHEET_PIN* parseSchSheetPin( SCH_SHEET* aSheet );
-    SCH_FIELD* parseSchField( SCH_COMPONENT* aParentSymbol );
+    SCH_FIELD* parseSchField( SCH_ITEM* aParent );
     SCH_COMPONENT* parseSchematicSymbol();
     SCH_BITMAP* parseImage();
     SCH_SHEET* parseSheet();

eeschema/sch_sexpr_plugin.cpp

@@ -416,7 +416,7 @@ void SCH_SEXPR_PLUGIN::init( KIWAY* aKiway, const PROPERTIES* aProperties )
     m_kiway = aKiway;
     m_cache = NULL;
     m_out = NULL;
-    m_fieldId = MANDATORY_FIELDS;
+    m_fieldId = 100;    // number arbitrarily > MANDATORY_FIELDS or SHEET_MANDATORY_FIELDS
 }
 
 
@@ -897,14 +897,20 @@ void SCH_SEXPR_PLUGIN::saveField( SCH_FIELD* aField, int aNestLevel )
 {
     wxCHECK_RET( aField != nullptr && m_out != nullptr, "" );
 
-    wxString fieldName;
+    wxString fieldName = aField->GetName();
 
     // For some reason (bug in legacy parser?) the field ID for non-mandatory fields is -1 so
     // check for this in order to correctly use the field name.
-    if( aField->GetId() >= 0 && aField->GetId() < MANDATORY_FIELDS )
+    if( aField->GetParent()->Type() == SCH_COMPONENT_T )
-        fieldName = TEMPLATE_FIELDNAME::GetDefaultFieldName( aField->GetId() );
+    {
-    else
+        if( aField->GetId() >= 0 && aField->GetId() < MANDATORY_FIELDS )
-        fieldName = aField->GetName();
+            fieldName = TEMPLATE_FIELDNAME::GetDefaultFieldName( aField->GetId() );
+    }
+    else if( aField->GetParent()->Type() == SCH_SHEET_T )
+    {
+        if( aField->GetId() >= 0 && aField->GetId() < SHEET_MANDATORY_FIELDS )
+            fieldName = SCH_SHEET::GetDefaultFieldName( aField->GetId() );
+    }
 
     if( aField->GetId() == -1 /* undefined ID */ )
     {
@@ -1009,6 +1015,8 @@ void SCH_SEXPR_PLUGIN::saveSheet( SCH_SHEET* aSheet, int aNestLevel )
     m_out->Print( aNestLevel + 1, "(uuid %s)", TO_UTF8( aSheet->m_Uuid.AsString() ) );
     m_out->Print( 0, "\n" );
 
+    m_fieldId = SHEET_MANDATORY_FIELDS;
+
     for( const SCH_FIELD& field : aSheet->GetFields() )
     {
         SCH_FIELD tmp = field;