From cb61525394cda687239fafa8d86fae146f3c81cd Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Thu, 26 Jul 2018 15:38:30 +0100 Subject: [PATCH] Handle separate parsing rules for ID_SCH and ID_PCB. This removes the existing constructors so that all parsing must be explicit and callers are made aware that they need to think about illegal characters, malformed ids, etc. Fixes: lp:1783474 * https://bugs.launchpad.net/kicad/+bug/1783474 --- common/footprint_info.cpp | 17 +++-- common/lib_id.cpp | 70 +++++++------------ cvpcb/display_footprints_frame.cpp | 2 +- cvpcb/readwrite_dlgs.cpp | 5 +- eeschema/class_libentry.cpp | 3 +- eeschema/dialogs/dialog_choose_component.cpp | 8 +-- .../dialog_edit_component_in_schematic.cpp | 10 ++- .../dialogs/dialog_edit_components_libid.cpp | 11 +-- eeschema/dialogs/dialog_rescue_each.cpp | 13 ++-- eeschema/files-io.cpp | 3 +- eeschema/getpart.cpp | 2 +- eeschema/project_rescue.cpp | 25 ++++--- eeschema/sch_legacy_plugin.cpp | 2 +- eeschema/selpart.cpp | 2 +- include/lib_id.h | 36 +++++----- ...og_edit_footprint_for_BoardEditor_base.cpp | 14 ++-- ...og_edit_footprint_for_BoardEditor_base.fbp | 6 +- ...alog_edit_footprint_for_fp_editor_base.cpp | 12 ++-- ...alog_edit_footprint_for_fp_editor_base.fbp | 4 +- pcbnew/dialogs/dialog_exchange_footprints.cpp | 18 +++-- pcbnew/dialogs/dialog_get_footprint.cpp | 4 +- pcbnew/eagle_plugin.cpp | 4 +- pcbnew/footprint_libraries_utils.cpp | 22 +++--- pcbnew/footprint_viewer_frame.cpp | 22 +++--- pcbnew/github/github_plugin.cpp | 2 +- pcbnew/gpcb_plugin.cpp | 2 +- pcbnew/kicad_netlist_reader.cpp | 2 +- pcbnew/kicad_plugin.cpp | 2 +- pcbnew/legacy_plugin.cpp | 6 +- pcbnew/load_select_footprint.cpp | 10 ++- pcbnew/microwave/microwave_inductor.cpp | 2 +- pcbnew/netlist_reader.cpp | 2 +- pcbnew/pcad2kicadpcb_plugin/pcb_module.cpp | 4 +- pcbnew/pcb_parser.cpp | 2 +- 34 files changed, 167 insertions(+), 182 deletions(-) diff --git a/common/footprint_info.cpp b/common/footprint_info.cpp index 69a70318e9..7ccd0f87de 100644 --- a/common/footprint_info.cpp +++ b/common/footprint_info.cpp @@ -56,17 +56,16 @@ FOOTPRINT_INFO* FOOTPRINT_LIST::GetModuleInfo( const wxString& aFootprintName ) if( aFootprintName.IsEmpty() ) return NULL; + LIB_ID fpid; + + wxCHECK_MSG( fpid.Parse( aFootprintName, LIB_ID::ID_PCB ) < 0, NULL, + wxString::Format( wxT( "\"%s\" is not a valid LIB_ID." ), aFootprintName ) ); + + wxString libNickname = fpid.GetLibNickname(); + wxString footprintName = fpid.GetLibItemName(); + for( auto& fp : m_list ) { - LIB_ID fpid; - - wxCHECK_MSG( fpid.Parse( aFootprintName ) < 0, NULL, - wxString::Format( - wxT( "\"%s\" is not a valid LIB_ID." ), GetChars( aFootprintName ) ) ); - - wxString libNickname = fpid.GetLibNickname(); - wxString footprintName = fpid.GetLibItemName(); - if( libNickname == fp->GetNickname() && footprintName == fp->GetFootprintName() ) return &*fp; } diff --git a/common/lib_id.cpp b/common/lib_id.cpp index 86c9abde31..f014a47480 100644 --- a/common/lib_id.cpp +++ b/common/lib_id.cpp @@ -119,7 +119,7 @@ void LIB_ID::clear() } -int LIB_ID::Parse( const UTF8& aId ) +int LIB_ID::Parse( const UTF8& aId, LIB_ID_TYPE aType, bool aFix ) { clear(); @@ -127,7 +127,7 @@ int LIB_ID::Parse( const UTF8& aId ) const char* rev = EndsWithRev( buffer, buffer+aId.length(), '/' ); size_t revNdx; size_t partNdx; - int offset; + int offset = -1; //============================================== // in a LIB_ID like discret:R3/rev4 @@ -150,9 +150,7 @@ int LIB_ID::Parse( const UTF8& aId ) offset = SetLibNickname( aId.substr( 0, partNdx ) ); if( offset > -1 ) - { return offset; - } ++partNdx; // skip ':' } @@ -165,48 +163,24 @@ int LIB_ID::Parse( const UTF8& aId ) if( partNdx >= revNdx ) return partNdx; // Error: no library item name. + UTF8 fpname = aId.substr( partNdx, revNdx-partNdx ); + // Be sure the item name is valid. // Some chars can be found in legacy files converted files from other EDA tools. - std::string fpname = aId.substr( partNdx, revNdx-partNdx ); - ReplaceIllegalFileNameChars( &fpname, '_' ); - SetLibItemName( UTF8( fpname ) ); + if( aFix ) + fpname = FixIllegalChars( fpname, aType, false ); + else + offset = HasIllegalChars( fpname, aType ); + + if( offset > -1 ) + return offset; + + SetLibItemName( fpname ); return -1; } -LIB_ID::LIB_ID( const UTF8& aId ) -{ - int offset = Parse( aId ); - - if( offset != -1 ) - { - THROW_PARSE_ERROR( _( "Illegal character found in LIB_ID string" ), - wxString::FromUTF8( aId.c_str() ), - aId.c_str(), - 0, - offset ); - } -} - - -LIB_ID::LIB_ID( const wxString& aId ) -{ - UTF8 id = aId; - - int offset = Parse( id ); - - if( offset != -1 ) - { - THROW_PARSE_ERROR( _( "Illegal character found in LIB_ID string" ), - aId, - id.c_str(), - 0, - offset ); - } -} - - LIB_ID::LIB_ID( const wxString& aLibName, const wxString& aLibItemName, const wxString& aRevision ) : nickname( aLibName ), @@ -359,15 +333,19 @@ int LIB_ID::compare( const LIB_ID& aLibId ) const } -bool LIB_ID::HasIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ) +int LIB_ID::HasIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ) { + int offset = 0; + for( auto ch : aLibItemName ) { if( !isLegalChar( ch, aType ) ) - return true; + return offset; + else + ++offset; } - return false; + return -1; } @@ -391,7 +369,8 @@ UTF8 LIB_ID::FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType, bool bool LIB_ID::isLegalChar( unsigned aUniChar, LIB_ID_TYPE aType ) { bool const colon_allowed = ( aType == ID_ALIAS ); - bool const space_allowed = ( aType != ID_SCH ); + bool const space_allowed = ( aType == ID_ALIAS || aType == ID_PCB ); + bool const illegal_filename_chars_allowed = ( aType == ID_SCH || aType == ID_ALIAS ); if( aUniChar < ' ' ) return false; @@ -400,7 +379,10 @@ bool LIB_ID::isLegalChar( unsigned aUniChar, LIB_ID_TYPE aType ) { case '/': case '\\': - return false; + case '<': + case '>': + case '"': + return illegal_filename_chars_allowed; case ':': return colon_allowed; diff --git a/cvpcb/display_footprints_frame.cpp b/cvpcb/display_footprints_frame.cpp index 7f55615a7f..55434be6f6 100644 --- a/cvpcb/display_footprints_frame.cpp +++ b/cvpcb/display_footprints_frame.cpp @@ -429,7 +429,7 @@ MODULE* DISPLAY_FOOTPRINTS_FRAME::Get_Module( const wxString& aFootprintName ) { LIB_ID fpid; - if( fpid.Parse( aFootprintName ) >= 0 ) + if( fpid.Parse( aFootprintName, LIB_ID::ID_PCB ) >= 0 ) { DisplayInfoMessage( this, wxString::Format( _( "Footprint ID \"%s\" is not valid." ), GetChars( aFootprintName ) ) ); diff --git a/cvpcb/readwrite_dlgs.cpp b/cvpcb/readwrite_dlgs.cpp index bb6d0b1c83..76f3af3d32 100644 --- a/cvpcb/readwrite_dlgs.cpp +++ b/cvpcb/readwrite_dlgs.cpp @@ -99,9 +99,8 @@ void CVPCB_MAINFRAME::SetNewPkg( const wxString& aFootprintName, int aIndex ) if( !aFootprintName.IsEmpty() ) { - wxCHECK_RET( fpid.Parse( aFootprintName ) < 0, - wxString::Format( _( "\"%s\" is not a valid LIB_ID." ), - GetChars( aFootprintName ) ) ); + wxCHECK_RET( fpid.Parse( aFootprintName, LIB_ID::ID_PCB ) < 0, + wxString::Format( _( "\"%s\" is not a valid LIB_ID." ), aFootprintName ) ); } component->SetFPID( fpid ); diff --git a/eeschema/class_libentry.cpp b/eeschema/class_libentry.cpp index 84f2936e38..ec0c97bb3b 100644 --- a/eeschema/class_libentry.cpp +++ b/eeschema/class_libentry.cpp @@ -287,8 +287,7 @@ void LIB_PART::SetName( const wxString& aName ) else m_aliases[0]->SetName( aName ); - // LIB_ALIAS validates the name, reuse it instead of validating the name again - wxString validatedName( m_aliases[0]->GetName() ); + wxString validatedName = LIB_ID::FixIllegalChars( aName, LIB_ID::ID_SCH ); m_libId.SetLibItemName( validatedName, false ); LIB_FIELD& valueField = GetValueField(); diff --git a/eeschema/dialogs/dialog_choose_component.cpp b/eeschema/dialogs/dialog_choose_component.cpp index 492a8105ec..5db8045ca1 100644 --- a/eeschema/dialogs/dialog_choose_component.cpp +++ b/eeschema/dialogs/dialog_choose_component.cpp @@ -57,15 +57,15 @@ DIALOG_CHOOSE_COMPONENT::DIALOG_CHOOSE_COMPONENT( SCH_BASE_FRAME* aParent, const bool aShowFootprints ) : DIALOG_SHIM( aParent, wxID_ANY, aTitle, wxDefaultPosition, wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER ), + m_hsplitter( nullptr ), + m_vsplitter( nullptr ), m_fp_sel_ctrl( nullptr ), m_fp_view_ctrl( nullptr ), m_parent( aParent ), m_deMorganConvert( aDeMorganConvert >= 0 ? aDeMorganConvert : 0 ), m_allow_field_edits( aAllowFieldEdits ), m_show_footprints( aShowFootprints ), - m_external_browser_requested( false ), - m_hsplitter( nullptr ), - m_vsplitter( nullptr ) + m_external_browser_requested( false ) { auto sizer = new wxBoxSizer( wxVERTICAL ); wxHtmlWindow* details = nullptr; @@ -325,7 +325,7 @@ void DIALOG_CHOOSE_COMPONENT::ShowFootprint( wxString const& aName ) { LIB_ID lib_id; - if( lib_id.Parse( aName ) == -1 && lib_id.IsValid() ) + if( lib_id.Parse( aName, LIB_ID::ID_PCB ) == -1 && lib_id.IsValid() ) { m_fp_view_ctrl->ClearStatus(); m_fp_view_ctrl->CacheFootprint( lib_id ); diff --git a/eeschema/dialogs/dialog_edit_component_in_schematic.cpp b/eeschema/dialogs/dialog_edit_component_in_schematic.cpp index 9e59857d83..d2653ee087 100644 --- a/eeschema/dialogs/dialog_edit_component_in_schematic.cpp +++ b/eeschema/dialogs/dialog_edit_component_in_schematic.cpp @@ -301,7 +301,7 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnBrowseLibrary( wxCommandEvent& event SCH_BASE_FRAME::HISTORY_LIST dummy; LIB_ID id; - id.Parse( m_libraryNameTextCtrl->GetValue() ); + id.Parse( m_libraryNameTextCtrl->GetValue(), LIB_ID::ID_SCH ); auto sel = GetParent()->SelectComponentFromLibrary( nullptr, dummy, true, 0, 0, false, &id ); @@ -349,7 +349,6 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnEditSpiceModel( wxCommandEvent& event bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::Validate() { wxString msg; - wxString tmp; LIB_ID id; // Commit any pending in-place edits and close the editor @@ -365,9 +364,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::Validate() return false; } - tmp = m_libraryNameTextCtrl->GetValue(); - tmp.Replace( wxT( " " ), wxT( "_" ) ); - id.Parse( tmp ); + id.Parse( m_libraryNameTextCtrl->GetValue(), LIB_ID::ID_SCH ); if( !id.IsValid() ) { @@ -438,7 +435,8 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnOKButtonClick( wxCommandEvent& event STATUS_FLAGS flags = m_cmp->GetFlags(); // Library symbol identifier - LIB_ID id( m_libraryNameTextCtrl->GetValue() ); + LIB_ID id; + id.Parse( m_libraryNameTextCtrl->GetValue(), LIB_ID::ID_SCH, true ); m_cmp->SetLibId( id, Prj().SchSymbolLibTable(), Prj().SchLibs()->GetCacheLibrary() ); // For symbols with multiple shapes (De Morgan representation) Set the selected shape: diff --git a/eeschema/dialogs/dialog_edit_components_libid.cpp b/eeschema/dialogs/dialog_edit_components_libid.cpp index 6069d6785a..1ebd204d33 100644 --- a/eeschema/dialogs/dialog_edit_components_libid.cpp +++ b/eeschema/dialogs/dialog_edit_components_libid.cpp @@ -560,7 +560,7 @@ bool DIALOG_EDIT_COMPONENTS_LIBID::validateLibIds() // a new lib id is found. validate this new value LIB_ID id; - id.Parse( new_libid ); + id.Parse( new_libid, LIB_ID::ID_SCH ); if( !id.IsValid() ) { @@ -629,7 +629,8 @@ void DIALOG_EDIT_COMPONENTS_LIBID::onClickOrphansButton( wxCommandEvent& event ) wxString orphanLibid = m_grid->GetCellValue( m_OrphansRowIndexes[ii], COL_CURR_LIBID ); int grid_row_idx = m_OrphansRowIndexes[ii]; //row index in m_grid for the current item - LIB_ID curr_libid( orphanLibid ); + LIB_ID curr_libid; + curr_libid.Parse( orphanLibid, LIB_ID::ID_SCH, true ); wxString symbName = curr_libid.GetLibItemName(); // number of full LIB_ID candidates (because we search for a symbol name // inside all avaiable libraries, perhaps the same symbol name can be found @@ -637,7 +638,7 @@ void DIALOG_EDIT_COMPONENTS_LIBID::onClickOrphansButton( wxCommandEvent& event ) int libIdCandidateCount = 0; candidateSymbNames.Clear(); - // now try to fin a candidate + // now try to find a candidate for( auto &lib : libs ) { aliasNames.Clear(); @@ -754,7 +755,7 @@ bool DIALOG_EDIT_COMPONENTS_LIBID::TransferDataFromWindow() // a new lib id is found and was already validated. // set this new value LIB_ID id; - id.Parse( new_libid ); + id.Parse( new_libid, LIB_ID::ID_SCH, true ); for( CMP_CANDIDATE& cmp : m_components ) { @@ -791,7 +792,7 @@ void DIALOG_EDIT_COMPONENTS_LIBID::revertChanges() continue; LIB_ID id; - id.Parse( cmp.m_InitialLibId ); + id.Parse( cmp.m_InitialLibId, LIB_ID::ID_SCH, true ); if( cmp.m_Component->GetLibId() != id ) { diff --git a/eeschema/dialogs/dialog_rescue_each.cpp b/eeschema/dialogs/dialog_rescue_each.cpp index cff704a93a..5e9511509e 100644 --- a/eeschema/dialogs/dialog_rescue_each.cpp +++ b/eeschema/dialogs/dialog_rescue_each.cpp @@ -96,33 +96,32 @@ DIALOG_RESCUE_EACH::DIALOG_RESCUE_EACH( SCH_EDIT_FRAME* aParent, RESCUER& aRescu dc.SetFont( font ); - int padding = 30; int width = dc.GetTextExtent( header ).GetWidth(); m_ListOfConflicts->AppendToggleColumn( header, wxDATAVIEW_CELL_ACTIVATABLE, width, wxALIGN_CENTER ); header = _( "Symbol Name" ); - width = dc.GetTextExtent( header ).GetWidth() + padding; + width = dc.GetTextExtent( header ).GetWidth() * 2; m_ListOfConflicts->AppendTextColumn( header, wxDATAVIEW_CELL_INERT, width ); header = _( "Action Taken" ); - width = dc.GetTextExtent( header ).GetWidth() + padding; + width = dc.GetTextExtent( header ).GetWidth() * 10; m_ListOfConflicts->AppendTextColumn( header, wxDATAVIEW_CELL_INERT, width ); header = _( "Reference" ); - width = dc.GetTextExtent( header ).GetWidth() + padding; + width = dc.GetTextExtent( header ).GetWidth() * 2; m_ListOfInstances->AppendTextColumn( header, wxDATAVIEW_CELL_INERT, width ); header = _( "Value" ); - width = dc.GetTextExtent( header ).GetWidth() + padding; + width = dc.GetTextExtent( header ).GetWidth() * 10; m_ListOfInstances->AppendTextColumn( header, wxDATAVIEW_CELL_INERT, width ); m_componentViewOld->SetLayoutDirection( wxLayout_LeftToRight ); m_componentViewNew->SetLayoutDirection( wxLayout_LeftToRight ); Layout(); - SetSizeInDU( 280, 240 ); + SetSizeInDU( 480, 240 ); // Make sure the HTML window is large enough. Some fun size juggling and // fudge factors here but it does seem to work pretty reliably. @@ -133,7 +132,7 @@ DIALOG_RESCUE_EACH::DIALOG_RESCUE_EACH( SCH_EDIT_FRAME* aParent, RESCUER& aRescu m_htmlPrompt->SetSizeHints( 2 * prompt_size.x / 3, approx_info_height ); Layout(); GetSizer()->SetSizeHints( this ); - SetSizeInDU( 280, 240 ); + SetSizeInDU( 480, 240 ); Center(); } diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index bc64d6a694..cd08aa3d89 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -844,7 +844,8 @@ bool SCH_EDIT_FRAME::importFile( const wxString& aFileName, int aFileType ) if( !fpField->GetText().IsEmpty() ) { - LIB_ID fpId( fpField->GetText() ); + LIB_ID fpId; + fpId.Parse( fpField->GetText(), LIB_ID::ID_SCH, true ); fpId.SetLibNickname( newfilename.GetName() ); fpField->SetText( fpId.Format() ); } diff --git a/eeschema/getpart.cpp b/eeschema/getpart.cpp index 9f539be385..b81e3063d6 100644 --- a/eeschema/getpart.cpp +++ b/eeschema/getpart.cpp @@ -85,7 +85,7 @@ SCH_BASE_FRAME::COMPONENT_SELECTION SCH_BASE_FRAME::SelectComponentFromLibBrowse { LIB_ID id; - if( id.Parse( symbol ) == -1 ) + if( id.Parse( symbol, LIB_ID::ID_SCH ) == -1 ) sel.LibId = id; sel.Unit = viewlibFrame->GetUnit(); diff --git a/eeschema/project_rescue.cpp b/eeschema/project_rescue.cpp index c4911d31c7..37fed7c70e 100644 --- a/eeschema/project_rescue.cpp +++ b/eeschema/project_rescue.cpp @@ -258,10 +258,15 @@ void RESCUE_CACHE_CANDIDATE::FindRescues( RESCUER& aRescuer, // 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. - if( ( ( cache_match && lib_match - && !cache_match->PinsConflictWith( *lib_match, true, true, true, true, false ) ) - || (!cache_match && lib_match ) ) && !LIB_ID::HasIllegalChars( part_name, LIB_ID::ID_SCH ) ) - continue; + if( LIB_ID::HasIllegalChars( part_name, LIB_ID::ID_SCH ) == -1 ) + { + if( cache_match && lib_match && + !cache_match->PinsConflictWith( *lib_match, true, true, true, true, false ) ) + continue; + + if( !cache_match && lib_match ) + continue; + } // Check if the symbol has already been rescued. wxString new_name = LIB_ID::FixIllegalChars( part_name, LIB_ID::ID_SCH ); @@ -379,12 +384,14 @@ void RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::FindRescues( continue; // Test whether there is a conflict or if the symbol can only be found in the cache. - if( ( ( cache_match && lib_match - && !cache_match->PinsConflictWith( *lib_match, true, true, true, true, false ) ) - || (!cache_match && lib_match ) ) - && !LIB_ID::HasIllegalChars( part_id.GetLibItemName(), LIB_ID::ID_SCH ) ) + if( LIB_ID::HasIllegalChars( part_id.GetLibItemName(), LIB_ID::ID_SCH ) == -1 ) { - continue; + if( cache_match && lib_match && + !cache_match->PinsConflictWith( *lib_match, true, true, true, true, false ) ) + continue; + + if( !cache_match && lib_match ) + continue; } // Fix illegal LIB_ID name characters. diff --git a/eeschema/sch_legacy_plugin.cpp b/eeschema/sch_legacy_plugin.cpp index 68e746f76e..33314071ae 100644 --- a/eeschema/sch_legacy_plugin.cpp +++ b/eeschema/sch_legacy_plugin.cpp @@ -1431,7 +1431,7 @@ SCH_COMPONENT* SCH_LEGACY_PLUGIN::loadComponent( FILE_LINE_READER& aReader ) // parsing the symbol name with LIB_ID::Parse() would break symbol library links // that contained '/' and ':' characters. if( m_version > 3 ) - libId.Parse( libName ); + libId.Parse( libName, LIB_ID::ID_SCH, true ); else libId.SetLibItemName( libName, false ); diff --git a/eeschema/selpart.cpp b/eeschema/selpart.cpp index b7038c3656..b8ba28948a 100644 --- a/eeschema/selpart.cpp +++ b/eeschema/selpart.cpp @@ -46,7 +46,7 @@ static void DisplayCmpDocAndKeywords( wxString& aSelection, void* aData ) LIB_ID id; - if( id.Parse( aSelection ) != -1 ) + if( id.Parse( aSelection, LIB_ID::ID_SCH ) != -1 ) { aSelection = _( "Invalid symbol library identifier!" ); return; diff --git a/include/lib_id.h b/include/lib_id.h index 308ff524c2..f7b04b4b3a 100644 --- a/include/lib_id.h +++ b/include/lib_id.h @@ -52,25 +52,15 @@ class LIB_ID { public: - LIB_ID() {} - - /** - * Takes \a aId string and parses it. - * - * A typical LIB_ID string consists of a library nickname followed by a library item name. - * e.g.: "smt:R_0805", or - * e.g.: "mylib:R_0805", or - * e.g.: "ttl:7400" - * - * @param aId is a string to be parsed into the LIB_ID object. - */ - LIB_ID( const UTF8& aId ); - - LIB_ID( const wxString& aId ); - ///> Types of library identifiers enum LIB_ID_TYPE { ID_SCH, ID_ALIAS, ID_PCB }; + LIB_ID() {} + + // NOTE: don't define any constructors which call Parse() on their arguments. We want it + // to be obvious to callers that parsing is involved (and that valid IDs are guaranteed in + // the presence of disallowed characters, malformed ids, etc.). + /** * This LIB_ID ctor is a special version which ignores the parsing due to symbol * names allowing '/' as a valid character. This was causing the symbol names to @@ -87,13 +77,19 @@ public: /** * Parse LIB_ID with the information from @a aId. * + * A typical LIB_ID string consists of a library nickname followed by a library item name. + * e.g.: "smt:R_0805", or + * e.g.: "mylib:R_0805", or + * e.g.: "ttl:7400" + * * @param aId is the string to populate the #LIB_ID object. + * @param aType indicates the LIB_ID type for type-specific parsing (such as allowed chars). + * @param aFix indicates invalid chars should be replaced with '_'. * * @return int - minus 1 (i.e. -1) means success, >= 0 indicates the character offset into * aId at which an error was detected. */ - int Parse( const UTF8& aId ); - + int Parse( const UTF8& aId, LIB_ID_TYPE aType, bool aFix = false ); /** * Return the logical library name portion of a LIB_ID. @@ -209,9 +205,9 @@ public: * * @param aLibItemName is the #LIB_ID name to test for illegal characters. * @param aType is the library identifier type - * @return true if \a aLibItemName contain illegal characters otherwise false. + * @return offset of first illegal character otherwise -1. */ - static bool HasIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ); + static int HasIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ); /** * Replace illegal #LIB_ID item name characters with underscores '_'. diff --git a/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.cpp b/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.cpp index bc4d3105b1..bda328c8b6 100644 --- a/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.cpp +++ b/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.cpp @@ -36,13 +36,13 @@ DIALOG_FOOTPRINT_BOARD_EDITOR_BASE::DIALOG_FOOTPRINT_BOARD_EDITOR_BASE( wxWindow m_itemsGrid->SetMargins( 0, 0 ); // Columns - m_itemsGrid->SetColSize( 0, 120 ); - m_itemsGrid->SetColSize( 1, 48 ); + m_itemsGrid->SetColSize( 0, 116 ); + m_itemsGrid->SetColSize( 1, 53 ); m_itemsGrid->SetColSize( 2, 90 ); m_itemsGrid->SetColSize( 3, 90 ); m_itemsGrid->SetColSize( 4, 90 ); - m_itemsGrid->SetColSize( 5, 48 ); - m_itemsGrid->SetColSize( 6, 112 ); + m_itemsGrid->SetColSize( 5, 53 ); + m_itemsGrid->SetColSize( 6, 106 ); m_itemsGrid->SetColSize( 7, 90 ); m_itemsGrid->SetColSize( 8, 90 ); m_itemsGrid->SetColSize( 9, 90 ); @@ -424,14 +424,14 @@ DIALOG_FOOTPRINT_BOARD_EDITOR_BASE::DIALOG_FOOTPRINT_BOARD_EDITOR_BASE( wxWindow m_modelsGrid->SetMargins( 0, 0 ); // Columns - m_modelsGrid->SetColSize( 0, 662 ); - m_modelsGrid->SetColSize( 1, 53 ); + m_modelsGrid->SetColSize( 0, 650 ); + m_modelsGrid->SetColSize( 1, 65 ); m_modelsGrid->EnableDragColMove( false ); m_modelsGrid->EnableDragColSize( false ); m_modelsGrid->SetColLabelSize( 22 ); m_modelsGrid->SetColLabelValue( 0, _("3D Model(s)") ); m_modelsGrid->SetColLabelValue( 1, _("Preview") ); - m_modelsGrid->SetColLabelAlignment( wxALIGN_LEFT, wxALIGN_CENTRE ); + m_modelsGrid->SetColLabelAlignment( wxALIGN_CENTRE, wxALIGN_CENTRE ); // Rows m_modelsGrid->EnableDragRowSize( false ); diff --git a/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.fbp b/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.fbp index 63278fa56d..fc6fdff354 100644 --- a/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.fbp +++ b/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor_base.fbp @@ -303,7 +303,7 @@ "Text Items" "Show" "Width" "Height" "Thickness" "Italic" "Layer" "Orientation" "Keep Upright" "X Offset" "Y Offset" wxALIGN_CENTRE 11 - 120,48,90,90,90,48,112,90,90,90,90 + 116,53,90,90,90,53,106,90,90,90,90 1 0 @@ -4589,12 +4589,12 @@ wxALIGN_TOP 0 1 - wxALIGN_LEFT + wxALIGN_CENTRE 22 "3D Model(s)" "Preview" wxALIGN_CENTRE 2 - 662,53 + 650,65 1 0 diff --git a/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.cpp b/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.cpp index 62e4cd9aa3..3b5ce13d1c 100644 --- a/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.cpp +++ b/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.cpp @@ -36,13 +36,13 @@ DIALOG_FOOTPRINT_FP_EDITOR_BASE::DIALOG_FOOTPRINT_FP_EDITOR_BASE( wxWindow* pare m_itemsGrid->SetMargins( 0, 0 ); // Columns - m_itemsGrid->SetColSize( 0, 120 ); - m_itemsGrid->SetColSize( 1, 48 ); + m_itemsGrid->SetColSize( 0, 116 ); + m_itemsGrid->SetColSize( 1, 53 ); m_itemsGrid->SetColSize( 2, 96 ); m_itemsGrid->SetColSize( 3, 96 ); m_itemsGrid->SetColSize( 4, 96 ); - m_itemsGrid->SetColSize( 5, 48 ); - m_itemsGrid->SetColSize( 6, 112 ); + m_itemsGrid->SetColSize( 5, 53 ); + m_itemsGrid->SetColSize( 6, 106 ); m_itemsGrid->SetColSize( 7, 80 ); m_itemsGrid->SetColSize( 8, 48 ); m_itemsGrid->SetColSize( 9, 96 ); @@ -332,8 +332,8 @@ DIALOG_FOOTPRINT_FP_EDITOR_BASE::DIALOG_FOOTPRINT_FP_EDITOR_BASE( wxWindow* pare m_modelsGrid->SetMargins( 0, 0 ); // Columns - m_modelsGrid->SetColSize( 0, 662 ); - m_modelsGrid->SetColSize( 1, 53 ); + m_modelsGrid->SetColSize( 0, 650 ); + m_modelsGrid->SetColSize( 1, 65 ); m_modelsGrid->EnableDragColMove( false ); m_modelsGrid->EnableDragColSize( false ); m_modelsGrid->SetColLabelSize( 22 ); diff --git a/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.fbp b/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.fbp index 0bab9a4316..7ea663af5d 100644 --- a/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.fbp +++ b/pcbnew/dialogs/dialog_edit_footprint_for_fp_editor_base.fbp @@ -302,7 +302,7 @@ "Text Items" "Show" "Width" "Height" "Thickness" "Italic" "Layer" "Orientation" "Unconstrained" "X Offset" "Y Offset" wxALIGN_CENTRE 11 - 120,48,96,96,96,48,112,80,48,96,96 + 116,53,96,96,96,53,106,80,48,96,96 1 0 @@ -3734,7 +3734,7 @@ "3D Model(s)" "Preview" wxALIGN_CENTRE 2 - 662,53 + 650,65 1 0 diff --git a/pcbnew/dialogs/dialog_exchange_footprints.cpp b/pcbnew/dialogs/dialog_exchange_footprints.cpp index 68d64366a8..e439afddcc 100644 --- a/pcbnew/dialogs/dialog_exchange_footprints.cpp +++ b/pcbnew/dialogs/dialog_exchange_footprints.cpp @@ -187,6 +187,8 @@ void DIALOG_EXCHANGE_FOOTPRINTS::setMatchMode( int aMatchMode ) bool DIALOG_EXCHANGE_FOOTPRINTS::isMatch( MODULE* aModule ) { + LIB_ID specifiedID; + switch( getMatchMode() ) { case ID_MATCH_FP_ALL: @@ -203,7 +205,8 @@ bool DIALOG_EXCHANGE_FOOTPRINTS::isMatch( MODULE* aModule ) else return aModule->GetValue() == m_specifiedValue->GetValue(); case ID_MATCH_FP_ID: - return aModule->GetFPID() == m_specifiedID->GetValue(); + specifiedID.Parse( m_specifiedID->GetValue(), LIB_ID::ID_PCB ); + return aModule->GetFPID() == specifiedID; } return false; // just to quiet compiler warnings.... } @@ -325,7 +328,7 @@ bool DIALOG_EXCHANGE_FOOTPRINTS::processCurrentModule() if( newFPIDStr.IsEmpty() ) return false; - newFPID = LIB_ID( newFPIDStr ); + newFPID.Parse( newFPIDStr, LIB_ID::ID_PCB, true ); } return processModule( m_currentModule, newFPID ); @@ -337,14 +340,19 @@ bool DIALOG_EXCHANGE_FOOTPRINTS::processMatchingModules() MODULE* Module; MODULE* PtBack; bool change = false; - wxString newFPID = m_newID->GetValue(); + LIB_ID newFPID; wxString value; if( !m_parent->GetBoard()->m_Modules ) return false; - if( !m_updateMode && newFPID == wxEmptyString ) - return false; + if( !m_updateMode ) + { + newFPID.Parse( m_newID->GetValue(), LIB_ID::ID_PCB ); + + if( !newFPID.IsValid() ) + return false; + } /* The change is done from the last module because processModule() modifies the last item * in the list. diff --git a/pcbnew/dialogs/dialog_get_footprint.cpp b/pcbnew/dialogs/dialog_get_footprint.cpp index ddff9b0457..7edfa41a0d 100644 --- a/pcbnew/dialogs/dialog_get_footprint.cpp +++ b/pcbnew/dialogs/dialog_get_footprint.cpp @@ -52,7 +52,9 @@ DIALOG_GET_FOOTPRINT::DIALOG_GET_FOOTPRINT( PCB_BASE_FRAME* parent, bool aShowBr for( size_t ii = 0; ii < s_HistoryList.size(); ++ii ) { - LIB_ID fpid( s_HistoryList[ ii ] ); + LIB_ID fpid; + fpid.Parse( s_HistoryList[ ii ], LIB_ID::ID_PCB ); + if( m_frame->CheckFootprint( fpid ) ) m_historyList->Append( s_HistoryList[ ii ] ); } diff --git a/pcbnew/eagle_plugin.cpp b/pcbnew/eagle_plugin.cpp index 89803ceccb..11a6b437ba 100644 --- a/pcbnew/eagle_plugin.cpp +++ b/pcbnew/eagle_plugin.cpp @@ -1346,7 +1346,9 @@ MODULE* EAGLE_PLUGIN::makeModule( wxXmlNode* aPackage, const wxString& aPkgName { std::unique_ptr m( new MODULE( m_board ) ); - m->SetFPID( LIB_ID( UTF8( aPkgName ) ) ); + LIB_ID fpID; + fpID.Parse( aPkgName, LIB_ID::ID_PCB, true ); + m->SetFPID( fpID ); // Get the first package item and iterate wxXmlNode* packageItem = aPackage->GetChildren(); diff --git a/pcbnew/footprint_libraries_utils.cpp b/pcbnew/footprint_libraries_utils.cpp index a2201611c4..4ab0f0dda8 100644 --- a/pcbnew/footprint_libraries_utils.cpp +++ b/pcbnew/footprint_libraries_utils.cpp @@ -550,23 +550,21 @@ bool FOOTPRINT_EDIT_FRAME::DeleteModuleFromCurrentLibrary() if( !Prj().PcbFootprintLibs()->IsFootprintLibWritable( nickname ) ) { - wxString msg = wxString::Format( - _( "Library \"%s\" is read only" ), - GetChars( nickname ) - ); - + wxString msg = wxString::Format( _( "Library \"%s\" is read only" ), nickname ); DisplayError( this, msg ); return false; } - wxString fpid_txt = PCB_BASE_FRAME::SelectFootprint( this, nickname, - wxEmptyString, wxEmptyString, Prj().PcbFootprintLibs() ); + LIB_ID fpid; + wxString fpid_txt = PCB_BASE_FRAME::SelectFootprint( this, nickname, wxEmptyString, + wxEmptyString, Prj().PcbFootprintLibs() ); - if( !fpid_txt ) + fpid.Parse( fpid_txt, LIB_ID::ID_PCB ); + + if( !fpid.IsValid() ) return false; - LIB_ID fpid( fpid_txt ); - wxString fpname = fpid.GetLibItemName(); + wxString fpname = fpid.GetLibItemName(); // Confirmation wxString msg = wxString::Format( FMT_OK_DELETE, fpname.GetData(), nickname.GetData() ); @@ -671,7 +669,7 @@ public: { m_module = aModule; m_savedFPID = aModule->GetFPID(); - m_module->SetFPID( LIB_ID( m_savedFPID.GetLibItemName() ) ); + m_module->SetFPID( LIB_ID( wxEmptyString, m_savedFPID.GetLibItemName() ) ); } ~LIBRARY_NAME_CLEARER() { @@ -855,7 +853,7 @@ MODULE* PCB_BASE_FRAME::CreateNewModule( const wxString& aModuleName ) module->SetLastEditTime(); // Update its name in lib - module->SetFPID( LIB_ID( moduleName ) ); + module->SetFPID( LIB_ID( wxEmptyString, moduleName ) ); wxPoint default_pos; BOARD_DESIGN_SETTINGS& settings = GetDesignSettings(); diff --git a/pcbnew/footprint_viewer_frame.cpp b/pcbnew/footprint_viewer_frame.cpp index 3b66e4e793..1dadf6430c 100644 --- a/pcbnew/footprint_viewer_frame.cpp +++ b/pcbnew/footprint_viewer_frame.cpp @@ -621,22 +621,16 @@ bool FOOTPRINT_VIEWER_FRAME::ShowModal( wxString* aFootprint, wxWindow* aResulta { if( aFootprint && !aFootprint->IsEmpty() ) { - try - { - LIB_ID fpid( *aFootprint ); + LIB_ID fpid; - if( fpid.IsValid() ) - { - setCurNickname( fpid.GetLibNickname() ); - setCurFootprintName( fpid.GetLibItemName() ); - ReCreateFootprintList(); - SelectAndViewFootprint( NEW_PART ); - } - } - catch( ... ) + fpid.Parse( *aFootprint, LIB_ID::ID_PCB, true ); + + if( fpid.IsValid() ) { - // LIB_ID's constructor throws on some invalid footprint IDs. It'd be nicer - // if it just set it to !IsValid(), but it is what it is. + setCurNickname( fpid.GetLibNickname() ); + setCurFootprintName( fpid.GetLibItemName() ); + ReCreateFootprintList(); + SelectAndViewFootprint( NEW_PART ); } } diff --git a/pcbnew/github/github_plugin.cpp b/pcbnew/github/github_plugin.cpp index 1ddaab5ee8..d450895b71 100644 --- a/pcbnew/github/github_plugin.cpp +++ b/pcbnew/github/github_plugin.cpp @@ -232,7 +232,7 @@ MODULE* GITHUB_PLUGIN::FootprintLoad( const wxString& aLibraryPath, // any name found in the pretty file; any name in the pretty file // must be ignored here. Also, the library nickname is unknown in // this context so clear it just in case. - ret->SetFPID( aFootprintName ); + ret->SetFPID( LIB_ID( wxEmptyString, aFootprintName ) ); return ret; } diff --git a/pcbnew/gpcb_plugin.cpp b/pcbnew/gpcb_plugin.cpp index f5b18c7ed0..f3d8dd4875 100644 --- a/pcbnew/gpcb_plugin.cpp +++ b/pcbnew/gpcb_plugin.cpp @@ -276,7 +276,7 @@ void GPCB_FPL_CACHE::Load() MODULE* footprint = parseMODULE( &reader ); // The footprint name is the file name without the extension. - footprint->SetFPID( LIB_ID( fn.GetName() ) ); + footprint->SetFPID( LIB_ID( wxEmptyString, fn.GetName() ) ); m_modules.insert( name, new GPCB_FPL_CACHE_ITEM( footprint, fn.GetName() ) ); } catch( const IO_ERROR& ioe ) diff --git a/pcbnew/kicad_netlist_reader.cpp b/pcbnew/kicad_netlist_reader.cpp index 018cef2cdc..c4fd3747d8 100644 --- a/pcbnew/kicad_netlist_reader.cpp +++ b/pcbnew/kicad_netlist_reader.cpp @@ -368,7 +368,7 @@ void KICAD_NETLIST_PARSER::parseComponent() } } - if( !footprint.IsEmpty() && fpid.Parse( footprint ) >= 0 ) + if( !footprint.IsEmpty() && fpid.Parse( footprint, LIB_ID::ID_PCB, true ) >= 0 ) { wxString error; error.Printf( _( "invalid footprint ID in\nfile: \"%s\"\nline: %d\noffset: %d" ), diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp index 2a5fe37984..93234dfc1e 100644 --- a/pcbnew/kicad_plugin.cpp +++ b/pcbnew/kicad_plugin.cpp @@ -299,7 +299,7 @@ void FP_CACHE::Load() // The footprint name is the file name without the extension. wxString fpName = fullPath.GetName(); - footprint->SetFPID( LIB_ID( fpName ) ); + footprint->SetFPID( LIB_ID( wxEmptyString, fpName ) ); m_modules.insert( fpName, new FP_CACHE_ITEM( footprint, fullPath ) ); m_cache_timestamp += fullPath.GetModificationTime().GetValue().GetValue(); diff --git a/pcbnew/legacy_plugin.cpp b/pcbnew/legacy_plugin.cpp index e29975e520..957d76935a 100644 --- a/pcbnew/legacy_plugin.cpp +++ b/pcbnew/legacy_plugin.cpp @@ -441,7 +441,7 @@ void LEGACY_PLUGIN::loadAllSections( bool doAppend ) ReplaceIllegalFileNameChars( &fpName ); if( !fpName.empty() ) - fpid = LIB_ID( UTF8( fpName ) ); + fpid.Parse( fpName, LIB_ID::ID_PCB, true ); module->SetFPID( fpid ); @@ -3317,7 +3317,7 @@ void LP_CACHE::LoadModules( LINE_READER* aReader ) ReplaceIllegalFileNameChars( &footprintName ); // set the footprint name first thing, so exceptions can use name. - module->SetFPID( LIB_ID( UTF8( footprintName ) ) ); + module->SetFPID( LIB_ID( wxEmptyString, footprintName ) ); m_owner->loadMODULE( module.get() ); @@ -3372,7 +3372,7 @@ void LP_CACHE::LoadModules( LINE_READER* aReader ) { nameOK = true; - m->SetFPID( LIB_ID( UTF8( newName ) ) ); + m->SetFPID( LIB_ID( wxEmptyString, newName ) ); std::pair r = m_modules.insert( newName, m ); wxASSERT_MSG( r.second, wxT( "error doing cache insert using guaranteed unique name" ) ); diff --git a/pcbnew/load_select_footprint.cpp b/pcbnew/load_select_footprint.cpp index 0b591551aa..135773c510 100644 --- a/pcbnew/load_select_footprint.cpp +++ b/pcbnew/load_select_footprint.cpp @@ -245,9 +245,8 @@ MODULE* PCB_BASE_FRAME::LoadModuleFromLibrary( const wxString& aLibrary, bool aU LIB_ID fpid; - wxCHECK_MSG( fpid.Parse( moduleName ) < 0, NULL, - wxString::Format( wxT( "Could not parse LIB_ID string \"%s\"." ), - GetChars( moduleName ) ) ); + wxCHECK_MSG( fpid.Parse( moduleName, LIB_ID::ID_PCB ) < 0, NULL, + wxString::Format( wxT( "Could not parse LIB_ID \"%s\"." ), moduleName ) ); try { @@ -275,9 +274,8 @@ MODULE* PCB_BASE_FRAME::LoadModuleFromLibrary( const wxString& aLibrary, bool aU } else { - wxCHECK_MSG( fpid.Parse( moduleName ) < 0, NULL, - wxString::Format( wxT( "Could not parse LIB_ID string \"%s\"." ), - GetChars( moduleName ) ) ); + wxCHECK_MSG( fpid.Parse( moduleName, LIB_ID::ID_PCB ) < 0, NULL, + wxString::Format( wxT( "Could not parse LIB_ID \"%s\"." ), moduleName ) ); try { diff --git a/pcbnew/microwave/microwave_inductor.cpp b/pcbnew/microwave/microwave_inductor.cpp index 752793d07a..e33011fdf4 100644 --- a/pcbnew/microwave/microwave_inductor.cpp +++ b/pcbnew/microwave/microwave_inductor.cpp @@ -345,7 +345,7 @@ MODULE* MWAVE::CreateMicrowaveInductor( INDUCTOR_PATTERN& inductorPattern, MODULE* module = aPcbFrame->CreateNewModule( msg ); aPcbFrame->AddModuleToBoard( module ); - module->SetFPID( LIB_ID( wxString( "mw_inductor" ) ) ); + module->SetFPID( LIB_ID( wxEmptyString, wxT( "mw_inductor" ) ) ); module->SetAttributes( MOD_VIRTUAL | MOD_CMS ); module->ClearFlags(); module->SetPosition( inductorPattern.m_End ); diff --git a/pcbnew/netlist_reader.cpp b/pcbnew/netlist_reader.cpp index 14fe4f83fe..212e4c9e8c 100644 --- a/pcbnew/netlist_reader.cpp +++ b/pcbnew/netlist_reader.cpp @@ -175,7 +175,7 @@ bool CMP_READER::Load( NETLIST* aNetlist ) { LIB_ID fpid; - if( !footprint.IsEmpty() && fpid.Parse( footprint ) >= 0 ) + if( !footprint.IsEmpty() && fpid.Parse( footprint, LIB_ID::ID_PCB, true ) >= 0 ) { wxString error; error.Printf( _( "invalid footprint ID in\nfile: \"%s\"\nline: %d" ), diff --git a/pcbnew/pcad2kicadpcb_plugin/pcb_module.cpp b/pcbnew/pcad2kicadpcb_plugin/pcb_module.cpp index fc7f3dcd4c..58adc082f9 100644 --- a/pcbnew/pcad2kicadpcb_plugin/pcb_module.cpp +++ b/pcbnew/pcad2kicadpcb_plugin/pcb_module.cpp @@ -520,7 +520,9 @@ void PCB_MODULE::AddToBoard() module->SetTimeStamp( 0 ); module->SetLastEditTime( 0 ); - module->SetFPID( LIB_ID( m_compRef ) ); + LIB_ID fpID; + fpID.Parse( m_compRef, LIB_ID::ID_PCB, true ); + module->SetFPID( fpID ); module->SetAttributes( MOD_DEFAULT | MOD_CMS ); diff --git a/pcbnew/pcb_parser.cpp b/pcbnew/pcb_parser.cpp index 72b0a4dd14..b2f1fed0e2 100644 --- a/pcbnew/pcb_parser.cpp +++ b/pcbnew/pcb_parser.cpp @@ -1808,7 +1808,7 @@ MODULE* PCB_PARSER::parseMODULE_unchecked( wxArrayString* aInitialComments ) name = FromUTF8(); - if( !name.IsEmpty() && fpid.Parse( FromUTF8() ) >= 0 ) + if( !name.IsEmpty() && fpid.Parse( name, LIB_ID::ID_PCB, true ) >= 0 ) { wxString error; error.Printf( _( "Invalid footprint ID in\nfile: \"%s\"\nline: %d\noffset: %d" ),