Properly handle improper symbols when pasting in symbol editor

Before, an improper symbol (one without the starting toekn) weren't
detected and reported to the user properly and would instead assert. Now
properly detect these and pass the error up the stack to the tool.

(Sentry issue KICAD-21J).
This commit is contained in:
Ian McInerney 2023-06-12 23:13:23 +01:00
parent 1c138b4f3e
commit d96580c651
6 changed files with 74 additions and 52 deletions

View File

@ -152,7 +152,7 @@ void SCH_SEXPR_PARSER::ParseLib( LIB_SYMBOL_MAP& aSymbolLibMap )
{ {
m_unit = 1; m_unit = 1;
m_convert = 1; m_convert = 1;
LIB_SYMBOL* symbol = ParseSymbol( aSymbolLibMap, m_requiredVersion ); LIB_SYMBOL* symbol = parseLibSymbol( aSymbolLibMap );
aSymbolLibMap[symbol->GetName()] = symbol; aSymbolLibMap[symbol->GetName()] = symbol;
} }
else else
@ -164,6 +164,34 @@ void SCH_SEXPR_PARSER::ParseLib( LIB_SYMBOL_MAP& aSymbolLibMap )
LIB_SYMBOL* SCH_SEXPR_PARSER::ParseSymbol( LIB_SYMBOL_MAP& aSymbolLibMap, int aFileVersion ) LIB_SYMBOL* SCH_SEXPR_PARSER::ParseSymbol( LIB_SYMBOL_MAP& aSymbolLibMap, int aFileVersion )
{
LIB_SYMBOL* newSymbol = nullptr;
NextTok();
// If there actually isn't anything here, don't throw just return a nullptr
if( CurTok() == T_LEFT )
{
NextTok();
if( CurTok() == T_symbol )
{
m_requiredVersion = aFileVersion;
newSymbol = parseLibSymbol( aSymbolLibMap );
}
else
{
wxString msg = wxString::Format( _( "Cannot parse %s as a symbol" ),
GetTokenString( CurTok() ) );
THROW_PARSE_ERROR( msg, CurSource(), CurLine(), CurLineNumber(), CurOffset() );
}
}
return newSymbol;
}
LIB_SYMBOL* SCH_SEXPR_PARSER::parseLibSymbol( LIB_SYMBOL_MAP& aSymbolLibMap )
{ {
wxCHECK_MSG( CurTok() == T_symbol, nullptr, wxCHECK_MSG( CurTok() == T_symbol, nullptr,
wxT( "Cannot parse " ) + GetTokenString( CurTok() ) + wxT( " as a symbol." ) ); wxT( "Cannot parse " ) + GetTokenString( CurTok() ) + wxT( " as a symbol." ) );
@ -176,7 +204,6 @@ LIB_SYMBOL* SCH_SEXPR_PARSER::ParseSymbol( LIB_SYMBOL_MAP& aSymbolLibMap, int aF
LIB_ITEM* item; LIB_ITEM* item;
std::unique_ptr<LIB_SYMBOL> symbol = std::make_unique<LIB_SYMBOL>( wxEmptyString ); std::unique_ptr<LIB_SYMBOL> symbol = std::make_unique<LIB_SYMBOL>( wxEmptyString );
m_requiredVersion = aFileVersion;
symbol->SetUnitCount( 1 ); symbol->SetUnitCount( 1 );
m_fieldId = MANDATORY_FIELDS; m_fieldId = MANDATORY_FIELDS;
@ -2398,7 +2425,7 @@ void SCH_SEXPR_PARSER::ParseSchematic( SCH_SHEET* aSheet, bool aIsCopyableOnly,
switch( token ) switch( token )
{ {
case T_symbol: case T_symbol:
symbol = ParseSymbol( symbolLibMap, m_requiredVersion ); symbol = parseLibSymbol( symbolLibMap );
symbol->UpdateFieldOrdinals(); symbol->UpdateFieldOrdinals();
screen->AddLibSymbol( symbol ); screen->AddLibSymbol( symbol );
break; break;

View File

@ -83,6 +83,9 @@ public:
void ParseLib( LIB_SYMBOL_MAP& aSymbolLibMap ); void ParseLib( LIB_SYMBOL_MAP& aSymbolLibMap );
/**
* Parse internal #LINE_READER object into symbols and return all found.
*/
LIB_SYMBOL* ParseSymbol( LIB_SYMBOL_MAP& aSymbolLibMap, LIB_SYMBOL* ParseSymbol( LIB_SYMBOL_MAP& aSymbolLibMap,
int aFileVersion = SEXPR_SYMBOL_LIB_FILE_VERSION ); int aFileVersion = SEXPR_SYMBOL_LIB_FILE_VERSION );
@ -155,6 +158,8 @@ private:
bool parseBool(); bool parseBool();
LIB_SYMBOL* parseLibSymbol( LIB_SYMBOL_MAP& aSymbolLibMap );
/** /**
* Parse stroke definition \a aStroke. * Parse stroke definition \a aStroke.
* *

View File

@ -1658,16 +1658,31 @@ void SCH_SEXPR_PLUGIN::GetDefaultSymbolFields( std::vector<wxString>& aNames )
} }
LIB_SYMBOL* SCH_SEXPR_PLUGIN::ParseLibSymbol( LINE_READER& aReader, int aFileVersion ) std::vector<LIB_SYMBOL*> SCH_SEXPR_PLUGIN::ParseLibSymbols( std::string& aSymbolText,
std::string aSource,
int aFileVersion )
{ {
LOCALE_IO toggle; // toggles on, then off, the C locale. LOCALE_IO toggle; // toggles on, then off, the C locale.
LIB_SYMBOL* newSymbol = nullptr;
LIB_SYMBOL_MAP map; LIB_SYMBOL_MAP map;
SCH_SEXPR_PARSER parser( &aReader );
parser.NeedLEFT(); std::vector<LIB_SYMBOL*> newSymbols;
parser.NextTok(); std::unique_ptr<STRING_LINE_READER> reader = std::make_unique<STRING_LINE_READER>( aSymbolText, aSource );
return parser.ParseSymbol( map, aFileVersion ); do
{
SCH_SEXPR_PARSER parser( reader.get() );
newSymbol = parser.ParseSymbol( map, aFileVersion );
if( newSymbol )
newSymbols.emplace_back( newSymbol );
reader.reset( new STRING_LINE_READER( *reader ) );
}
while( newSymbol );
return newSymbols;
} }

View File

@ -137,8 +137,9 @@ public:
const wxString& GetError() const override { return m_error; } const wxString& GetError() const override { return m_error; }
static LIB_SYMBOL* ParseLibSymbol( LINE_READER& aReader, static std::vector<LIB_SYMBOL*> ParseLibSymbols( std::string& aSymbolText,
int aVersion = SEXPR_SCHEMATIC_FILE_VERSION ); std::string aSource,
int aFileVersion = SEXPR_SCHEMATIC_FILE_VERSION );
static void FormatLibSymbol( LIB_SYMBOL* aPart, OUTPUTFORMATTER& aFormatter ); static void FormatLibSymbol( LIB_SYMBOL* aPart, OUTPUTFORMATTER& aFormatter );
private: private:

View File

@ -852,43 +852,16 @@ void SYMBOL_EDIT_FRAME::DuplicateSymbol( bool aFromClipboard )
if( aFromClipboard ) if( aFromClipboard )
{ {
wxLogNull doNotLog; // disable logging of failed clipboard actions std::string clipboardData = m_toolManager->GetClipboardUTF8();
auto clipboard = wxTheClipboard;
wxClipboardLocker clipboardLock( clipboard );
if( !clipboardLock
|| !( clipboard->IsSupported( wxDF_TEXT )
|| clipboard->IsSupported( wxDF_UNICODETEXT ) ) )
{
return;
}
wxTextDataObject data;
clipboard->GetData( data );
wxString symbolSource = data.GetText();
std::unique_ptr<STRING_LINE_READER> reader = std::make_unique<STRING_LINE_READER>( TO_UTF8( symbolSource ), wxS( "Clipboard" ) );
LIB_SYMBOL* newSymbol = nullptr;
do
{
try try
{ {
newSymbol = SCH_SEXPR_PLUGIN::ParseLibSymbol( *reader ); newSymbols = SCH_SEXPR_PLUGIN::ParseLibSymbols( clipboardData, "Clipboard" );
} }
catch( IO_ERROR& e ) catch( IO_ERROR& e )
{ {
wxLogMessage( wxS( "Can not paste: %s" ), e.Problem() ); wxLogMessage( wxS( "Can not paste: %s" ), e.Problem() );
break;
} }
if( newSymbol )
newSymbols.emplace_back( newSymbol );
reader.reset( new STRING_LINE_READER( *reader ) );
}
while( newSymbol );
} }
else else
{ {

View File

@ -759,30 +759,31 @@ int SYMBOL_EDITOR_EDIT_TOOL::Copy( const TOOL_EVENT& aEvent )
int SYMBOL_EDITOR_EDIT_TOOL::Paste( const TOOL_EVENT& aEvent ) int SYMBOL_EDITOR_EDIT_TOOL::Paste( const TOOL_EVENT& aEvent )
{ {
LIB_SYMBOL* symbol = m_frame->GetCurSymbol(); LIB_SYMBOL* symbol = m_frame->GetCurSymbol();
LIB_SYMBOL* newPart = nullptr;
if( !symbol || symbol->IsAlias() ) if( !symbol || symbol->IsAlias() )
return 0; return 0;
std::string text_utf8 = m_toolMgr->GetClipboardUTF8(); std::string clipboardData = m_toolMgr->GetClipboardUTF8();
STRING_LINE_READER reader( text_utf8, "Clipboard" );
LIB_SYMBOL* newPart;
try try
{ {
newPart = SCH_SEXPR_PLUGIN::ParseLibSymbol( reader ); std::vector<LIB_SYMBOL*> newParts = SCH_SEXPR_PLUGIN::ParseLibSymbols( clipboardData, "Clipboard" );
if( newParts.empty() || !newParts[0] )
return -1;
newPart = newParts[0];
} }
catch( IO_ERROR& ) catch( IO_ERROR& )
{ {
// If it's not a symbol then paste as text // If it's not a symbol then paste as text
newPart = new LIB_SYMBOL( "dummy_part" ); newPart = new LIB_SYMBOL( "dummy_part" );
LIB_TEXT* newText = new LIB_TEXT( newPart ); LIB_TEXT* newText = new LIB_TEXT( newPart );
newText->SetText( wxString::FromUTF8( text_utf8.c_str() ) ); newText->SetText( clipboardData );
newPart->AddDrawItem( newText ); newPart->AddDrawItem( newText );
} }
if( !newPart )
return -1;
SCH_COMMIT commit( m_toolMgr ); SCH_COMMIT commit( m_toolMgr );
commit.Modify( symbol ); commit.Modify( symbol );