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

Fix schematic symbol rescue issues. 

This fix makes some fundamental changes to the way symbols are rescued:

* The new symbol library file format is used when rescuing symbols defined
  in the symbol library table.
* The original library nickname is appended to the symbol name rather than
  the rescue library nickname when rescuing symbol defined in the symbol
  library table.
* Escaping strings has been removed from legacy library rescues since the
  legacy library file format never supported it.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/12525

Fixes https://gitlab.com/kicad/code/kicad/-/issues/12624
This commit is contained in:
Wayne Stambaugh 2022-10-11 16:16:11 -04:00
parent 84cbf38194
commit 148467d2a1
4 changed files with 80 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

136
eeschema/project_rescue.cpp
View File

@ -150,7 +150,6 @@ void RESCUE_CASE_CANDIDATE::FindRescues( RESCUER& aRescuer,
for( SCH_SYMBOL* eachSymbol : *( aRescuer.GetSymbols() ) )
{
symbol_name = eachSymbol->GetLibId().GetLibItemName();
search_name = EscapeString ( symbol_name, CTX_LIBID );
if( last_symbol_name != symbol_name )
{
@ -159,25 +158,28 @@ void RESCUE_CASE_CANDIDATE::FindRescues( RESCUER& aRescuer,
last_symbol_name = symbol_name;
case_insensitive_matches.clear();
LIB_ID id( wxEmptyString, search_name );
LIB_ID id( wxEmptyString, symbol_name );
case_sensitive_match = aRescuer.GetPrj()->SchLibs()->FindLibSymbol( id );
if( case_sensitive_match )
continue;
// If the case sensitive match failed, try a case insensitive match.
if( !case_sensitive_match )
aRescuer.GetPrj()->SchLibs()->FindLibraryNearEntries( case_insensitive_matches,
search_name );
aRescuer.GetPrj()->SchLibs()->FindLibraryNearEntries( case_insensitive_matches,
search_name );
// If there are not case insensitive matches either, the symbol cannot be rescued.
if( !case_insensitive_matches.size() )
continue;
RESCUE_CASE_CANDIDATE candidate( symbol_name, case_insensitive_matches[0]->GetName(),
case_insensitive_matches[0],
eachSymbol->GetUnit(),
eachSymbol->GetConvert() );
candidate_map[symbol_name] = candidate;
}
if( case_sensitive_match || !( case_insensitive_matches.size() ) )
continue;
RESCUE_CASE_CANDIDATE candidate( symbol_name, case_insensitive_matches[0]->GetName(),
case_insensitive_matches[0],
eachSymbol->GetUnit(),
eachSymbol->GetConvert() );
candidate_map[symbol_name] = candidate;
}
// Now, dump the map into aCandidates
@ -249,24 +251,31 @@ void RESCUE_CACHE_CANDIDATE::FindRescues( RESCUER& aRescuer,
LIB_SYMBOL* cache_match = nullptr;
LIB_SYMBOL* lib_match = nullptr;
wxString symbol_name;
wxString search_name;
wxString old_symbol_name;
for( SCH_SYMBOL* eachSymbol : *( aRescuer.GetSymbols() ) )
{
symbol_name = eachSymbol->GetLibId().GetLibItemName();
search_name = EscapeString ( symbol_name, CTX_LIBID );
symbol_name = eachSymbol->GetLibId().GetUniStringLibItemName();
if( old_symbol_name != symbol_name )
{
// A new symbol name is found (a new group starts here).
// Search the symbol names candidates only once for this group:
old_symbol_name = symbol_name;
cache_match = findSymbol( search_name, aRescuer.GetPrj()->SchLibs(), true );
lib_match = findSymbol( search_name, aRescuer.GetPrj()->SchLibs(), false );
cache_match = findSymbol( symbol_name, aRescuer.GetPrj()->SchLibs(), true );
lib_match = findSymbol( symbol_name, aRescuer.GetPrj()->SchLibs(), false );
if( !cache_match && !lib_match )
continue;
// At some point during V5 development, the LIB_ID delimiter character ':' was
// replaced by '_' when writing the symbol cache library so we have to test for
// the LIB_NICKNAME_LIB_SYMBOL_NAME case.
if( !cache_match && eachSymbol->GetLibId().IsValid() )
{
wxString tmp;
tmp = eachSymbol->GetLibId().GetLibNickname().wx_str() + wxT( "_" ) +
eachSymbol->GetLibId().GetLibItemName().wx_str();
cache_match = findSymbol( tmp, aRescuer.GetPrj()->SchLibs(), true );
}
// Test whether there is a conflict or if the symbol can only be found in the cache
// and the symbol name does not have any illegal characters.
@ -278,7 +287,7 @@ void RESCUE_CACHE_CANDIDATE::FindRescues( RESCUER& aRescuer,
continue;
// Check if the symbol has already been rescued.
RESCUE_CACHE_CANDIDATE candidate( symbol_name, search_name, cache_match, lib_match,
RESCUE_CACHE_CANDIDATE candidate( symbol_name, symbol_name, cache_match, lib_match,
eachSymbol->GetUnit(),
eachSymbol->GetConvert() );
@ -378,7 +387,7 @@ void RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::FindRescues(
LIB_SYMBOL* lib_match = nullptr;
LIB_ID old_symbol_id;
wxString escapedSymbolName;
wxString symbolName;
for( SCH_SYMBOL* eachSymbol : *( aRescuer.GetSymbols() ) )
{
@ -390,23 +399,20 @@ void RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::FindRescues(
// Search the symbol names candidates only once for this group:
old_symbol_id = symbol_id;
escapedSymbolName = EscapeString( symbol_id.Format().wx_str(), CTX_LIBID );
symbolName = symbol_id.Format().wx_str();
// Get the library symbol from the cache library. It will be a flattened
// symbol by default (no inheritance).
cache_match = findSymbol( escapedSymbolName, aRescuer.GetPrj()->SchLibs(),
true );
cache_match = findSymbol( symbolName, aRescuer.GetPrj()->SchLibs(), true );
// At some point during V5 development, the LIB_ID delimiter character ':' was
// replaced by '_' when writing the symbol cache library so we have to test for
// the LIB_NICKNAME_LIB_SYMBOL_NAME case.
if( !cache_match )
{
escapedSymbolName = EscapeString( symbol_id.GetLibNickname().wx_str() +
wxT( "_" ) + symbol_id.GetLibItemName().wx_str(),
CTX_LIBID );
cache_match = findSymbol( escapedSymbolName, aRescuer.GetPrj()->SchLibs(),
true );
symbolName = symbol_id.GetLibNickname().wx_str() + wxT( "_" ) +
symbol_id.GetLibItemName().wx_str();
cache_match = findSymbol( symbolName, aRescuer.GetPrj()->SchLibs(), true );
}
// Get the library symbol from the symbol library table.
@ -442,15 +448,13 @@ void RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::FindRescues(
}
// Fix illegal LIB_ID name characters.
wxString new_name = LIB_ID::FixIllegalChars( symbol_id.GetLibItemName(), false );
wxString new_name = EscapeString( symbol_id.GetLibItemName(), CTX_LIBID );
// Differentiate symbol name in the rescue library by appending the symbol library
// table nickname to the symbol name to prevent name clashes in the rescue library.
// Differentiate symbol name in the rescue library by appending the original symbol
// library table nickname to the symbol name to prevent name clashes in the rescue
// library.
wxString libNickname = GetRescueLibraryFileName( aRescuer.Schematic() ).GetName();
// Spaces in the file name will break the symbol name because they are not
// quoted in the symbol library file format.
libNickname.Replace( " ", "-" );
LIB_ID new_id( libNickname, new_name + "-" + symbol_id.GetLibNickname().wx_str() );
RESCUE_SYMBOL_LIB_TABLE_CANDIDATE candidate( symbol_id, new_id, cache_match, lib_match,
@ -477,19 +481,19 @@ wxString RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::GetActionDescription() const
{
action.Printf( _( "Cannot rescue symbol %s which is not available in any library or "
"the cache." ),
m_requested_id.GetLibItemName().wx_str() );
UnescapeString( m_requested_id.GetLibItemName().wx_str() ) );
}
else if( m_cache_candidate && !m_lib_candidate )
{
action.Printf( _( "Rescue symbol %s found only in cache library to %s." ),
m_requested_id.Format().wx_str(),
m_new_id.Format().wx_str() );
UnescapeString( m_requested_id.Format().wx_str() ),
UnescapeString( m_new_id.Format().wx_str() ) );
}
else
{
action.Printf( _( "Rescue modified symbol %s to %s" ),
m_requested_id.Format().wx_str(),
m_new_id.Format().wx_str() );
UnescapeString( m_requested_id.Format().wx_str() ),
UnescapeString( m_new_id.Format().wx_str() ) );
}
return action;
@ -808,15 +812,19 @@ void SYMBOL_LIB_TABLE_RESCUER::InvokeDialog( wxWindow* aParent, bool aAskShowAga
void SYMBOL_LIB_TABLE_RESCUER::OpenRescueLibrary()
{
m_pi.set( SCH_IO_MGR::FindPlugin( SCH_IO_MGR::SCH_LEGACY ) );
(*m_properties)[ SCH_LEGACY_PLUGIN::PropBuffering ] = "";
wxFileName fn = GetRescueLibraryFileName( m_schematic );
SYMBOL_LIB_TABLE_ROW* row = m_prj->SchSymbolLibTable()->FindRow( fn.GetName() );
// If a rescue library already exists copy the contents of that library so we do not
// lose any previous rescues.
if( m_prj->SchSymbolLibTable()->HasLibrary( fn.GetName() ) )
if( row )
{
if( SCH_IO_MGR::EnumFromStr( row->GetType() ) == SCH_IO_MGR::SCH_KICAD )
fn.SetExt( KiCadSymbolLibFileExtension );
std::vector<LIB_SYMBOL*> symbols;
try
@ -829,7 +837,7 @@ void SYMBOL_LIB_TABLE_RESCUER::OpenRescueLibrary()
}
for( LIB_SYMBOL* symbol : symbols )
AddSymbol( symbol );
m_rescueLibSymbols.emplace_back( std::make_unique<LIB_SYMBOL>( *symbol ) );
}
}
@ -838,10 +846,19 @@ bool SYMBOL_LIB_TABLE_RESCUER::WriteRescueLibrary( wxWindow *aParent )
{
wxString msg;
wxFileName fn = GetRescueLibraryFileName( m_schematic );
SYMBOL_LIB_TABLE_ROW* row = m_prj->SchSymbolLibTable()->FindRow( fn.GetName() );
fn.SetExt( KiCadSymbolLibFileExtension );
SCH_PLUGIN::SCH_PLUGIN_RELEASER pi;
pi.set( SCH_IO_MGR::FindPlugin( SCH_IO_MGR::SCH_KICAD ) );
try
{
m_pi->SaveLibrary( fn.GetFullPath() );
for( const std::unique_ptr<LIB_SYMBOL>& symbol : m_rescueLibSymbols )
pi->SaveSymbol( fn.GetFullPath(), new LIB_SYMBOL( *symbol.get() ), m_properties.get() );
pi->SaveLibrary( fn.GetFullPath() );
}
catch( const IO_ERROR& ioe )
{
@ -852,17 +869,17 @@ bool SYMBOL_LIB_TABLE_RESCUER::WriteRescueLibrary( wxWindow *aParent )
// If the rescue library already exists in the symbol library table no need save it to add
// it to the table.
if( !m_prj->SchSymbolLibTable()->HasLibrary( fn.GetName() ) )
if( !row || ( SCH_IO_MGR::EnumFromStr( row->GetType() ) == SCH_IO_MGR::SCH_LEGACY ) )
{
if( row && SCH_IO_MGR::EnumFromStr( row->GetType() ) == SCH_IO_MGR::SCH_LEGACY )
{
m_prj->SchSymbolLibTable()->RemoveRow( row );
}
wxString uri = "${KIPRJMOD}/" + fn.GetFullName();
wxString libNickname = fn.GetName();
// Spaces in the file name will break the symbol name because they are not
// quoted in the symbol library file format.
libNickname.Replace( " ", "-" );
SYMBOL_LIB_TABLE_ROW* row = new SYMBOL_LIB_TABLE_ROW( libNickname, uri,
wxString( "Legacy" ) );
row = new SYMBOL_LIB_TABLE_ROW( libNickname, uri, wxT( "KiCad" ) );
m_prj->SchSymbolLibTable()->InsertRow( row );
fn = wxFileName( m_prj->GetProjectPath(), SYMBOL_LIB_TABLE::GetSymbolLibTableFileName() );
@ -879,7 +896,6 @@ bool SYMBOL_LIB_TABLE_RESCUER::WriteRescueLibrary( wxWindow *aParent )
}
}
// Relaod the symbol library table.
m_prj->SetElem( PROJECT::ELEM_SYMBOL_LIB_TABLE, nullptr );
// This can only happen if the symbol library table file was corrupted on write.
@ -897,13 +913,5 @@ void SYMBOL_LIB_TABLE_RESCUER::AddSymbol( LIB_SYMBOL* aNewSymbol )
{
wxCHECK_RET( aNewSymbol, "Invalid LIB_SYMBOL pointer." );
wxFileName fn = GetRescueLibraryFileName( m_schematic );
try
{
m_pi->SaveSymbol( fn.GetFullPath(), new LIB_SYMBOL( *aNewSymbol ), m_properties.get() );
}
catch( ... /* IO_ERROR */ )
{
}
m_rescueLibSymbols.emplace_back( std::make_unique<LIB_SYMBOL>( *aNewSymbol ) );
}

6
eeschema/project_rescue.h
View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2015 Chris Pavlina <pavlina.chris@gmail.com>
* Copyright (C) 2015-2021 KiCad Developers, see change_log.txt for contributors.
* Copyright (C) 2015-2022 KiCad Developers, see change_log.txt for contributors.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@ -379,9 +379,9 @@ public:
virtual void AddSymbol( LIB_SYMBOL* aNewSymbol ) override;
private:
SCH_PLUGIN::SCH_PLUGIN_RELEASER m_pi;
std::vector<std::unique_ptr<LIB_SYMBOL>> m_rescueLibSymbols;
std::unique_ptr< PROPERTIES > m_properties; ///< Library plugin properties
std::unique_ptr< PROPERTIES > m_properties; ///< Library plugin properties.
};
#endif // _LIB_CACHE_RESCUE_H_

9
eeschema/sch_plugins/legacy/sch_legacy_lib_plugin_cache.cpp
View File

@ -303,11 +303,10 @@ LIB_SYMBOL* SCH_LEGACY_PLUGIN_CACHE::LoadPart( LINE_READER& aReader, int aMajorV
name = tokens.GetNextToken();
// Don't escape symbol library name if it's already been escaped. Given that the original
// change to escape the symbol library name has resulted in rescue libraries with escaped
// names, we will have to live with the consequences.
if( name == UnescapeString( name ) )
name = EscapeString( name, CTX_LIBID );
// This fixes a dubious decision to escape LIB_ID characters. Escaped LIB_IDs broke rescue
// library look up. Legacy LIB_IDs should not be escaped.
if( name != UnescapeString( name ) )
name = UnescapeString( name );
pos += name.size() + 1;

2
eeschema/sch_screen.cpp
View File

@ -934,7 +934,7 @@ void SCH_SCREEN::UpdateSymbolLinks( REPORTER* aReporter )
if( !tmp && legacyLibs && legacyLibs->GetLibraryCount() )
{
SYMBOL_LIB& legacyCacheLib = legacyLibs->at( 0 );
SYMBOL_LIB& legacyCacheLib = legacyLibs->back();
// It better be the cache library.
wxCHECK2( legacyCacheLib.IsCache(), continue );