From d30ac2967afabf211512c900fa81d5942d97c468 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Wed, 20 Jun 2018 21:12:11 -0700 Subject: [PATCH] eeschema: Rescue symbols with illegal chars When parsing component names, we need to account for the possibility of illegal characters (e.g. "/", ":") in the names from v4 libraries. They are fixed internally by the cache parser but if we don't fix them in the rescue routine, the symbol won't match it's cache name. This standardizes all schematic illegal character routines into LIB_ID Fixes: lp:1774774 * https://bugs.launchpad.net/kicad/+bug/1774774 --- common/lib_id.cpp | 17 +++++++++++------ eeschema/class_libentry.cpp | 16 +--------------- eeschema/lib_manager.cpp | 9 --------- eeschema/lib_manager.h | 5 ----- eeschema/libfield.cpp | 2 +- eeschema/project_rescue.cpp | 3 ++- eeschema/sch_eagle_plugin.cpp | 14 ++++++-------- eeschema/sch_legacy_plugin.cpp | 2 +- include/lib_id.h | 5 +++-- 9 files changed, 25 insertions(+), 48 deletions(-) diff --git a/common/lib_id.cpp b/common/lib_id.cpp index b10a82ce1f..f7b57e5c68 100644 --- a/common/lib_id.cpp +++ b/common/lib_id.cpp @@ -371,14 +371,17 @@ bool LIB_ID::HasIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ) } -UTF8 LIB_ID::FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ) +UTF8 LIB_ID::FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType, bool aLib ) { UTF8 fixedName; for( UTF8::uni_iter chIt = aLibItemName.ubegin(); chIt < aLibItemName.uend(); ++chIt ) { auto ch = *chIt; - fixedName += isLegalChar( ch, aType ) ? ch : '_'; + if( aLib ) + fixedName += isLegalLibNicknameChar( ch, aType ) ? ch : '_'; + else + fixedName += isLegalChar( ch, aType ) ? ch : '_'; } return fixedName; @@ -386,15 +389,17 @@ UTF8 LIB_ID::FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ) ///> Set of characters not accepted library entry names. -#define BASE_ILLEGAL_CHARS '\t', '\n', '\r', ':', '/', '\\' -const unsigned schIllegalChars[] = { BASE_ILLEGAL_CHARS, ' ' }; -const unsigned pcbIllegalChars[] = { BASE_ILLEGAL_CHARS, 0 }; +#define BASE_ILLEGAL_CHARS '\t', '\n', '\r', '/', '\\' +const unsigned schIllegalChars[] = { BASE_ILLEGAL_CHARS, ':', ' ' }; +const unsigned schAliasIllegalChars[] = { BASE_ILLEGAL_CHARS, 0, 0 }; +const unsigned pcbIllegalChars[] = { BASE_ILLEGAL_CHARS, ':', 0 }; #define ILL_CHAR_SIZE (sizeof(schIllegalChars) / sizeof(int)) bool LIB_ID::isLegalChar( unsigned aUniChar, LIB_ID_TYPE aType ) { const unsigned (&illegalChars)[ILL_CHAR_SIZE] = - aType == ID_SCH ? schIllegalChars : pcbIllegalChars; + aType == ID_SCH ? schIllegalChars : + aType == ID_ALIAS ? schAliasIllegalChars : pcbIllegalChars; for( const unsigned ch : illegalChars ) { diff --git a/eeschema/class_libentry.cpp b/eeschema/class_libentry.cpp index 8d67a4da07..84f2936e38 100644 --- a/eeschema/class_libentry.cpp +++ b/eeschema/class_libentry.cpp @@ -119,24 +119,10 @@ PART_LIB* LIB_ALIAS::GetLib() void LIB_ALIAS::SetName( const wxString& aName ) { - name = aName; - ValidateName( name ); + name = LIB_ID::FixIllegalChars( aName, LIB_ID::ID_ALIAS ); } -void LIB_ALIAS::ValidateName( wxString& aName ) -{ - // they are same as illegal filename chars, but the ':' is allowed - // only because it is used to create symbol names in lib cache - static const wxString illegalSymbolNameChars( "\\/\"<>|" ); - - for( wxString::iterator it = aName.begin(); it != aName.end(); ++it ) - { - if( illegalSymbolNameChars.Find( *it ) != wxNOT_FOUND ) - *it = '_'; - } -} - bool LIB_ALIAS::operator==( const wxChar* aName ) const { diff --git a/eeschema/lib_manager.cpp b/eeschema/lib_manager.cpp index d20209b4ff..a26e053cb0 100644 --- a/eeschema/lib_manager.cpp +++ b/eeschema/lib_manager.cpp @@ -569,15 +569,6 @@ bool LIB_MANAGER::LibraryExists( const wxString& aLibrary, bool aCheckEnabled ) return symTable()->HasLibrary( aLibrary, aCheckEnabled ); } - -wxString LIB_MANAGER::ValidateName( const wxString& aName ) -{ - wxString name( aName ); - name.Replace( " ", "_" ); - return name; -} - - wxString LIB_MANAGER::GetUniqueLibraryName() const { wxString name = "New_Library"; diff --git a/eeschema/lib_manager.h b/eeschema/lib_manager.h index ce524eec2e..371818e9fb 100644 --- a/eeschema/lib_manager.h +++ b/eeschema/lib_manager.h @@ -217,11 +217,6 @@ public: */ bool RevertLibrary( const wxString& aLibrary ); - /** - * Replaces all characters considered illegal in library/part names with underscores. - */ - static wxString ValidateName( const wxString& aName ); - /** * Returns a library name that is not currently in use. * Used for generating names for new libraries. diff --git a/eeschema/libfield.cpp b/eeschema/libfield.cpp index 1b17a9b327..cb9fbc6209 100644 --- a/eeschema/libfield.cpp +++ b/eeschema/libfield.cpp @@ -65,7 +65,7 @@ void LIB_EDIT_FRAME::EditField( LIB_FIELD* aField ) return; newFieldValue = dlg.GetText(); - LIB_ALIAS::ValidateName( newFieldValue ); + LIB_ID::FixIllegalChars( newFieldValue, LIB_ID::ID_SCH ); wxString oldFieldValue = aField->GetFullText( m_unit ); bool renamed = aField->GetId() == VALUE && newFieldValue != oldFieldValue; diff --git a/eeschema/project_rescue.cpp b/eeschema/project_rescue.cpp index a451605f7b..c4911d31c7 100644 --- a/eeschema/project_rescue.cpp +++ b/eeschema/project_rescue.cpp @@ -94,6 +94,7 @@ static void get_components( std::vector& aComponents ) static LIB_PART* find_component( const wxString& aName, PART_LIBS* aLibs, bool aCached ) { LIB_PART *part = NULL; + wxString new_name = LIB_ID::FixIllegalChars( aName, LIB_ID::ID_SCH ); for( PART_LIB& each_lib : *aLibs ) { @@ -103,7 +104,7 @@ static LIB_PART* find_component( const wxString& aName, PART_LIBS* aLibs, bool a if( !aCached && each_lib.IsCache() ) continue; - part = each_lib.FindPart( aName ); + part = each_lib.FindPart( new_name ); if( part ) break; diff --git a/eeschema/sch_eagle_plugin.cpp b/eeschema/sch_eagle_plugin.cpp index 983607b7fe..0b36e1f257 100644 --- a/eeschema/sch_eagle_plugin.cpp +++ b/eeschema/sch_eagle_plugin.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -134,8 +135,7 @@ wxString SCH_EAGLE_PLUGIN::getLibName() m_libName = "noname"; m_libName += "-eagle-import"; - // use ID_SCH as it is more restrictive - m_libName = LIB_ID::FixIllegalChars( m_libName, LIB_ID::ID_SCH ); + m_libName = LIB_ID::FixIllegalChars( m_libName, LIB_ID::ID_SCH, true ); } return m_libName; @@ -1064,22 +1064,20 @@ void SCH_EAGLE_PLUGIN::loadInstance( wxXmlNode* aInstanceNode ) wxString gatename = epart->deviceset + epart->device + einstance.gate; wxString symbolname = wxString( epart->deviceset + epart->device ); symbolname.Replace( "*", "" ); - LIB_ALIAS::ValidateName( symbolname ); + wxString kisymbolname = LIB_ID::FixIllegalChars( symbolname, LIB_ID::ID_SCH ); int unit = m_eagleLibs[libraryname].GateUnit[gatename]; wxString package; EAGLE_LIBRARY* elib = &m_eagleLibs[libraryname]; - auto p = elib->package.find( symbolname ); + auto p = elib->package.find( kisymbolname ); if( p != elib->package.end() ) { package = p->second; } - wxString kisymbolname = LIB_ID::FixIllegalChars( symbolname, LIB_ID::ID_SCH ); - LIB_ALIAS* alias = m_pi->LoadSymbol( getLibFileName().GetFullPath(), kisymbolname, m_properties.get() ); @@ -1138,7 +1136,7 @@ void SCH_EAGLE_PLUGIN::loadInstance( wxXmlNode* aInstanceNode ) if( epart->value ) component->GetField( VALUE )->SetText( *epart->value ); else - component->GetField( VALUE )->SetText( symbolname ); + component->GetField( VALUE )->SetText( kisymbolname ); // Set the visibility of fields. component->GetField( REFERENCE )->SetVisible( part->GetField( REFERENCE )->IsVisible() ); @@ -1262,7 +1260,7 @@ EAGLE_LIBRARY* SCH_EAGLE_PLUGIN::loadLibrary( wxXmlNode* aLibraryNode, wxString symbolName = edeviceset.name + edevice.name; symbolName.Replace( "*", "" ); wxASSERT( !symbolName.IsEmpty() ); - LIB_ALIAS::ValidateName( symbolName ); + symbolName = LIB_ID::FixIllegalChars( symbolName, LIB_ID::ID_SCH ); if( edevice.package ) aEagleLibrary->package[symbolName] = edevice.package.Get(); diff --git a/eeschema/sch_legacy_plugin.cpp b/eeschema/sch_legacy_plugin.cpp index 35c4a42d85..f1a7fc0f69 100644 --- a/eeschema/sch_legacy_plugin.cpp +++ b/eeschema/sch_legacy_plugin.cpp @@ -2417,7 +2417,7 @@ void SCH_LEGACY_PLUGIN_CACHE::loadDocs() SCH_PARSE_ERROR( "$CMP command expected", reader, line ); parseUnquotedString( aliasName, reader, line, &line ); // Alias name. - LIB_ALIAS::ValidateName( aliasName ); + LIB_ID::FixIllegalChars( aliasName, LIB_ID::ID_ALIAS ); LIB_ALIAS_MAP::iterator it = m_aliases.find( aliasName ); if( it == m_aliases.end() ) diff --git a/include/lib_id.h b/include/lib_id.h index 052f415904..308ff524c2 100644 --- a/include/lib_id.h +++ b/include/lib_id.h @@ -69,7 +69,7 @@ public: LIB_ID( const wxString& aId ); ///> Types of library identifiers - enum LIB_ID_TYPE { ID_SCH, ID_PCB }; + enum LIB_ID_TYPE { ID_SCH, ID_ALIAS, ID_PCB }; /** * This LIB_ID ctor is a special version which ignores the parsing due to symbol @@ -218,9 +218,10 @@ public: * * @param aLibItemName is the #LIB_ID item name to replace illegal characters. * @param aType is the library identifier type + * @param aLib True if we are checking library names, false if we are checking item names * @return the corrected version of \a aLibItemName. */ - static UTF8 FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ); + static UTF8 FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType, bool aLib = false ); /** * Looks for characters that are illegal in library nicknames.