FP_LIB_TABLE::ROW::SetFullURI() does environment variable substitution up front.

We can now remove a few calls to FP_LIB_TABLE::ExpandSubstitutions( const wxString& aString )
since its being done in above function.

DIALOG_FP_LIB_TABLE now handles table to copy cut copy paste from global to
project and reverse.  Fixed a problem with cursor position management when
switching between tables.

common/fp_lib_table.cpp

@@ -51,6 +51,13 @@ void FP_LIB_TABLE::ROW::SetType( const wxString& aType )
 }
 
 
+void FP_LIB_TABLE::ROW::SetFullURI( const wxString& aFullURI )
+{
+    uri_user = aFullURI;
+    uri_expanded = FP_LIB_TABLE::ExpandSubstitutions( aFullURI );
+}
+
+
 FP_LIB_TABLE::FP_LIB_TABLE( FP_LIB_TABLE* aFallBackTable ) :
     fallBack( aFallBackTable )
 {
@@ -407,7 +414,7 @@ const FP_LIB_TABLE::ROW* FP_LIB_TABLE::FindRowByURI( const wxString& aURI )
 
         for( unsigned i = 0;  i < cur->rows.size();  i++ )
         {
-            wxString uri = ExpandSubstitutions( cur->rows[i].GetFullURI() );
+            wxString uri = cur->rows[i].GetFullURI( true );
 
             if( wxFileName::GetPathSeparator() == wxChar( '\\' ) && uri.Find( wxChar( '/' ) ) >= 0 )
                 uri.Replace( wxT( "/" ), wxT( "\\" ) );
@@ -461,16 +468,17 @@ const FP_LIB_TABLE::ROW* FP_LIB_TABLE::FindRow( const wxString& aLibraryNickName
         THROW_IO_ERROR( msg );
     }
 
-    /*  enable this when FP_LIB_TABLE::Footprint*() functions are put into use.
+#if 0   // enable this as soon as FP_LIB_TABLE::FindRow() is not being used outside
+        // this class, and FP_LIB_TABLE::Footprint*() functions are put into use.
     if( !row->plugin )
         row->setPlugin( IO_MGR::PluginFind( row->type ) );
-    */
+#endif
 
     return row;
 }
 
 
-const wxString FP_LIB_TABLE::ExpandSubstitutions( const wxString aString )
+const wxString FP_LIB_TABLE::ExpandSubstitutions( const wxString& aString )
 {
     // We reserve the right to do this another way, by providing our own member
     // function.
@@ -604,7 +612,7 @@ bool FP_LIB_TABLE::ConvertFromLegacy( NETLIST& aNetList, const wxArrayString& aL
 
                 for( unsigned i = 0;  i < cur->rows.size();  i++ )
                 {
-                    wxString uri = ExpandSubstitutions( cur->rows[i].GetFullURI() );
+                    wxString uri = cur->rows[i].GetFullURI( true );
 
                     if( wxFileName::GetPathSeparator() == wxChar( '\\' )
                       && uri.Find( wxChar( '/' ) ) >= 0 )
@@ -616,6 +624,7 @@ bool FP_LIB_TABLE::ConvertFromLegacy( NETLIST& aNetList, const wxArrayString& aL
                         break;
                     }
                 }
+
             } while( ( cur = cur->fallBack ) != 0 && libNickname.IsEmpty() );
 
             if( libNickname.IsEmpty() )

include/fp_lib_table.h

@@ -112,17 +112,18 @@ public:
         ROW( const wxString& aNick, const wxString& aURI, const wxString& aType,
              const wxString& aOptions, const wxString& aDescr = wxEmptyString ) :
             nickName( aNick ),
-            uri( aURI ),
-            options( aOptions ),
             description( aDescr ),
             properties( 0 )
         {
+            SetOptions( aOptions ),
+            SetFullURI( aURI );
             SetType( aType );
         }
 
         ROW( const ROW& a ) :
             nickName( a.nickName ),
-            uri( a.uri ),
+            uri_user( a.uri_user ),
+            uri_expanded( a.uri_expanded ),
             type( a.type ),
             options( a.options ),
             description( a.description ),
@@ -139,22 +140,23 @@ public:
 
         ROW& operator=( const ROW& r )
         {
-            nickName = r.nickName;
+            nickName     = r.nickName;
-            uri      = r.uri;
+            uri_user     = r.uri_user;
-            type     = r.type;
+            uri_expanded = r.uri_expanded;
-            options  = r.options;
+            type         = r.type;
+            options      = r.options;
+            description  = r.description;
+            properties   = r.properties ? new PROPERTIES( *r.properties ) : NULL;
 
-            description = r.description;
+            // do not copy the PLUGIN, it is lazily created.
-            properties  = r.properties ? new PROPERTIES( *r.properties ) : NULL;
+            // setPlugin( NULL );
-
-            setPlugin( NULL );  // do not copy the PLUGIN, it is lazily created.
 
             return *this;
         }
 
         bool operator==( const ROW& r ) const
         {
-            return  nickName==r.nickName && uri==r.uri && type==r.type && options==r.options;
+            return  nickName==r.nickName && uri_user==r.uri_user && type==r.type && options==r.options;
         }
 
         bool operator!=( const ROW& r ) const   { return !( *this == r ); }
@@ -187,23 +189,24 @@ public:
 
         /**
          * Function GetFullURI
-         * returns the full location specifying URI for the LIB.
+         * returns the full location specifying URI for the LIB, either in original
+         * UI form or in environment variable expanded form.
          *
-         * @param doEnvVarSubs tells this function to do the substitution, else not.
+         * @param aSubstituted Tells if caller wanted the substituted form, else not.
          */
-        const wxString GetFullURI( bool doEnvVarSubs = false ) const
+        const wxString& GetFullURI( bool aSubstituted = false ) const
         {
-            if( doEnvVarSubs )
+            if( aSubstituted )
-                return FP_LIB_TABLE::ExpandSubstitutions( uri );
+                return uri_expanded;
             else
-                return uri;
+                return uri_user;
         }
 
         /**
          * Function SetFullURI
          * changes the full URI for the library.
          */
-        void SetFullURI( const wxString& aFullURI ) { uri = aFullURI; }
+        void SetFullURI( const wxString& aFullURI );
 
         /**
          * Function GetOptions
@@ -274,7 +277,8 @@ public:
         }
 
         wxString        nickName;
-        wxString        uri;
+        wxString        uri_user;           ///< what user entered from UI or loaded from disk
+        wxString        uri_expanded;       ///< from ExpandSubstitutions()
         LIB_T           type;
         wxString        options;
         wxString        description;
@@ -524,7 +528,7 @@ public:
      * This enables (fp_lib_table)s to have platform dependent environment
      * variables in them, allowing for a uniform table across platforms.
      */
-    static const wxString ExpandSubstitutions( const wxString aString );
+    static const wxString ExpandSubstitutions( const wxString& aString );
 
     /**
      * Function LoadGlobalTable

pcbnew/dialogs/dialog_fp_lib_table.cpp

@@ -251,6 +251,16 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
     int selRowCount;
     int selColCount;
 
+    int getCursorRow() const
+    {
+        return m_cur_grid->GetGridCursorRow();
+    }
+
+    int getCursorCol() const
+    {
+        return m_cur_grid->GetGridCursorCol();
+    }
+
     /// Gets the selected area into a sensible rectangle of sel{Row,Col}{Start,Count} above.
     void getSelectedArea()
     {
@@ -328,128 +338,13 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
             menu.Enable( MYID_PASTE, false );
         }
 
-        // if there is no current cell cursor, disable paste.
-        else if( m_cur_row == -1 || m_cur_col == -1 )
-            menu.Enable( MYID_PASTE, false );
-
         PopupMenu( &menu );
 
         // passOnFocus();
     }
 
-    void cutcopy( bool doCut )
+    void cutcopy( bool doCut );
-    {
+    void paste();
-        // this format is compatible with most spreadsheets
-        if( wxTheClipboard->Open() )
-        {
-            wxGridTableBase*    tbl = m_cur_grid->GetTable();
-            wxString            txt;
-
-            for( int row = selRowStart;  row < selRowStart + selRowCount;  ++row )
-            {
-                for( int col = selColStart;  col < selColStart + selColCount; ++col )
-                {
-                    txt += tbl->GetValue( row, col );
-
-                    if( col < selColStart + selColCount - 1 )   // that was not last column
-                        txt += COL_SEP;
-
-                    if( doCut )
-                        tbl->SetValue( row, col, wxEmptyString );
-                }
-                txt += ROW_SEP;
-            }
-
-            wxTheClipboard->SetData( new wxTextDataObject( txt ) );
-            wxTheClipboard->Close();
-
-            m_cur_grid->AutoSizeColumns();
-            m_cur_grid->ForceRefresh();
-        }
-    }
-
-    void paste()
-    {
-        // assume format came from a spreadsheet or us.
-        if( wxTheClipboard->Open() )
-        {
-            if( wxTheClipboard->IsSupported( wxDF_TEXT ) )
-            {
-                wxTextDataObject    data;
-                FP_TBL_MODEL*       tbl = (FP_TBL_MODEL*) m_cur_grid->GetTable();
-
-                wxTheClipboard->GetData( data );
-
-                wxString    cb_text = data.GetText();
-                size_t      ndx = cb_text.find( wxT( "(fp_lib_table" ) );
-
-                if( ndx != std::string::npos )
-                {
-                    // paste the ROWs of s-expression (fp_lib_table), starting
-                    // at column 0 regardless of current cursor column.
-
-                    STRING_LINE_READER  slr( TO_UTF8( cb_text ), wxT( "Clipboard" ) );
-                    FP_LIB_TABLE_LEXER  lexer( &slr );
-                    FP_LIB_TABLE        tmp_tbl;
-                    bool                parsed = true;
-
-                    try
-                    {
-                        tmp_tbl.Parse( &lexer );
-                    }
-                    catch( PARSE_ERROR& pe )
-                    {
-                        // @todo tell what line and offset
-                        parsed = false;
-                    }
-
-                    if( parsed )
-                    {
-                        // if clipboard rows would extend past end of current table size...
-                        if( int( tmp_tbl.rows.size() ) > tbl->GetNumberRows() - m_cur_row )
-                        {
-                            int newRowsNeeded = tmp_tbl.rows.size() - ( tbl->GetNumberRows() - m_cur_row );
-                            tbl->AppendRows( newRowsNeeded );
-                        }
-
-                        for( int i = 0;  i < (int) tmp_tbl.rows.size();  ++i )
-                        {
-                            tbl->rows[m_cur_row+i] = tmp_tbl.rows[i];
-                        }
-                    }
-                    m_cur_grid->AutoSizeColumns();
-                }
-                else
-                {
-                    wxStringTokenizer   rows( cb_text, ROW_SEP, wxTOKEN_RET_EMPTY );
-
-                    // if clipboard rows would extend past end of current table size...
-                    if( int( rows.CountTokens() ) > tbl->GetNumberRows() - m_cur_row )
-                    {
-                        int newRowsNeeded = rows.CountTokens() - ( tbl->GetNumberRows() - m_cur_row );
-                        tbl->AppendRows( newRowsNeeded );
-                    }
-
-                    for( int row = m_cur_row;  rows.HasMoreTokens();  ++row )
-                    {
-                        wxString rowTxt = rows.GetNextToken();
-
-                        wxStringTokenizer   cols( rowTxt, COL_SEP, wxTOKEN_RET_EMPTY );
-
-                        for( int col = m_cur_col; cols.HasMoreTokens();  ++col )
-                        {
-                            wxString cellTxt = cols.GetNextToken();
-                            tbl->SetValue( row, col, cellTxt );
-                        }
-                    }
-                    m_cur_grid->AutoSizeColumns();
-                }
-            }
-
-            wxTheClipboard->Close();
-            m_cur_grid->ForceRefresh();
-        }
-    }
 
     // the user clicked on a popup menu choice:
     void onPopupSelection( wxCommandEvent& event )
@@ -473,6 +368,9 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
         case MYID_SELECT:
             m_cur_grid->SelectAll();
             break;
+
+        default:
+            ;
         }
     }
 
@@ -617,16 +515,16 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
 
     void deleteRowHandler( wxMouseEvent& event )
     {
-        int curRow = m_cur_grid->GetGridCursorRow();
+        int curRow = getCursorRow();
         m_cur_grid->DeleteRows( curRow );
     }
 
     void moveUpHandler( wxMouseEvent& event )
     {
-        int curRow = m_cur_grid->GetGridCursorRow();
+        int curRow = getCursorRow();
         if( curRow >= 1 )
         {
-            int curCol = m_cur_grid->GetGridCursorCol();
+            int curCol = getCursorCol();
 
             FP_TBL_MODEL* tbl = (FP_TBL_MODEL*) m_cur_grid->GetTable();
 
@@ -655,10 +553,10 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
     {
         FP_TBL_MODEL* tbl = (FP_TBL_MODEL*) m_cur_grid->GetTable();
 
-        int curRow = m_cur_grid->GetGridCursorRow();
+        int curRow = getCursorRow();
         if( unsigned( curRow + 1 ) < tbl->rows.size() )
         {
-            int curCol  = m_cur_grid->GetGridCursorCol();
+            int curCol  = getCursorCol();
 
             ROW move_me = tbl->rows[curRow];
 
@@ -734,17 +632,6 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
         rightClickCellPopupMenu();
     }
 
-    void onGridCmdSelectCell( wxGridEvent& event )
-    {
-        m_cur_row = event.GetRow();
-        m_cur_col = event.GetCol();
-
-        D(printf("change cursor(%d,%d)\n", m_cur_row, m_cur_col );)
-
-        // somebody else wants this
-        event.Skip();
-    }
-
     /// Populate the readonly environment variable table with names and values
     /// by examining all the full_uri columns.
     void populateEnvironReadOnlyTable()
@@ -822,20 +709,13 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
 
     wxGrid*             m_cur_grid;     ///< changed based on tab choice
 
-    // wxGrid makes it difficult to know if the cursor is yet visible,
-    // use this to solve that, initial values are -1
-    int                 m_cur_row;      ///< cursor position
-    int                 m_cur_col;
-
 public:
     DIALOG_FP_LIB_TABLE( wxFrame* aParent, FP_LIB_TABLE* aGlobal, FP_LIB_TABLE* aProject ) :
         DIALOG_FP_LIB_TABLE_BASE( aParent ),
         m_global( aGlobal ),
         m_project( aProject ),
         m_global_model( *aGlobal ),
-        m_project_model( *aProject ),
+        m_project_model( *aProject )
-        m_cur_row( 0 ),
-        m_cur_col( 0 )
     {
         m_global_grid->SetTable( (wxGridTableBase*) &m_global_model );
         m_project_grid->SetTable( (wxGridTableBase*) &m_project_model );
@@ -899,6 +779,127 @@ public:
 };
 
 
+void DIALOG_FP_LIB_TABLE::cutcopy( bool doCut )
+{
+    // this format is compatible with most spreadsheets
+    if( wxTheClipboard->Open() )
+    {
+        wxGridTableBase*    tbl = m_cur_grid->GetTable();
+        wxString            txt;
+
+        for( int row = selRowStart;  row < selRowStart + selRowCount;  ++row )
+        {
+            for( int col = selColStart;  col < selColStart + selColCount; ++col )
+            {
+                txt += tbl->GetValue( row, col );
+
+                if( col < selColStart + selColCount - 1 )   // that was not last column
+                    txt += COL_SEP;
+
+                if( doCut )
+                    tbl->SetValue( row, col, wxEmptyString );
+            }
+            txt += ROW_SEP;
+        }
+
+        wxTheClipboard->SetData( new wxTextDataObject( txt ) );
+        wxTheClipboard->Close();
+
+        m_cur_grid->AutoSizeColumns();
+        m_cur_grid->ForceRefresh();
+    }
+}
+
+
+void DIALOG_FP_LIB_TABLE::paste()
+{
+    // assume format came from a spreadsheet or us.
+    if( wxTheClipboard->Open() )
+    {
+        if( wxTheClipboard->IsSupported( wxDF_TEXT ) )
+        {
+            wxTextDataObject    data;
+            FP_TBL_MODEL*       tbl = (FP_TBL_MODEL*) m_cur_grid->GetTable();
+
+            wxTheClipboard->GetData( data );
+
+            wxString    cb_text = data.GetText();
+            size_t      ndx = cb_text.find( wxT( "(fp_lib_table" ) );
+
+            if( ndx != std::string::npos )
+            {
+                // paste the ROWs of s-expression (fp_lib_table), starting
+                // at column 0 regardless of current cursor column.
+
+                STRING_LINE_READER  slr( TO_UTF8( cb_text ), wxT( "Clipboard" ) );
+                FP_LIB_TABLE_LEXER  lexer( &slr );
+                FP_LIB_TABLE        tmp_tbl;
+                bool                parsed = true;
+
+                try
+                {
+                    tmp_tbl.Parse( &lexer );
+                }
+                catch( PARSE_ERROR& pe )
+                {
+                    // @todo tell what line and offset
+                    parsed = false;
+                }
+
+                if( parsed )
+                {
+                    const int cur_row = getCursorRow();
+
+                    // if clipboard rows would extend past end of current table size...
+                    if( int( tmp_tbl.rows.size() ) > tbl->GetNumberRows() - cur_row )
+                    {
+                        int newRowsNeeded = tmp_tbl.rows.size() - ( tbl->GetNumberRows() - cur_row );
+                        tbl->AppendRows( newRowsNeeded );
+                    }
+
+                    for( int i = 0;  i < (int) tmp_tbl.rows.size();  ++i )
+                    {
+                        tbl->rows[cur_row+i] = tmp_tbl.rows[i];
+                    }
+                }
+                m_cur_grid->AutoSizeColumns();
+            }
+            else
+            {
+                const int           cur_row = getCursorRow();
+                const int           cur_col = getCursorCol();
+                wxStringTokenizer   rows( cb_text, ROW_SEP, wxTOKEN_RET_EMPTY );
+
+                // if clipboard rows would extend past end of current table size...
+                if( int( rows.CountTokens() ) > tbl->GetNumberRows() - cur_row )
+                {
+                    int newRowsNeeded = rows.CountTokens() - ( tbl->GetNumberRows() - cur_row );
+
+                    tbl->AppendRows( newRowsNeeded );
+                }
+
+                for( int row = cur_row;  rows.HasMoreTokens();  ++row )
+                {
+                    wxString rowTxt = rows.GetNextToken();
+
+                    wxStringTokenizer   cols( rowTxt, COL_SEP, wxTOKEN_RET_EMPTY );
+
+                    for( int col = cur_col; cols.HasMoreTokens();  ++col )
+                    {
+                        wxString cellTxt = cols.GetNextToken();
+                        tbl->SetValue( row, col, cellTxt );
+                    }
+                }
+                m_cur_grid->AutoSizeColumns();
+            }
+        }
+
+        wxTheClipboard->Close();
+        m_cur_grid->ForceRefresh();
+    }
+}
+
+
 int InvokePcbLibTableEditor( wxFrame* aParent, FP_LIB_TABLE* aGlobal, FP_LIB_TABLE* aProject )
 {
     DIALOG_FP_LIB_TABLE dlg( aParent, aGlobal, aProject );

pcbnew/io_mgr.cpp

@@ -221,7 +221,7 @@ BOARD* IO_MGR::Load( PCB_FILE_T aFileType, const wxString& aFileName,
                      BOARD* aAppendToMe, const PROPERTIES* aProperties )
 {
     // release the PLUGIN even if an exception is thrown.
-    PLUGIN::RELEASER pi = PluginFind( aFileType );
+    PLUGIN::RELEASER pi( PluginFind( aFileType ) );
 
     if( (PLUGIN*) pi )  // test pi->plugin
     {
@@ -235,7 +235,7 @@ BOARD* IO_MGR::Load( PCB_FILE_T aFileType, const wxString& aFileName,
 void IO_MGR::Save( PCB_FILE_T aFileType, const wxString& aFileName, BOARD* aBoard, const PROPERTIES* aProperties )
 {
     // release the PLUGIN even if an exception is thrown.
-    PLUGIN::RELEASER pi = PluginFind( aFileType );
+    PLUGIN::RELEASER pi( PluginFind( aFileType ) );
 
     if( (PLUGIN*) pi )  // test pi->plugin
     {

pcbnew/io_mgr.h

@@ -405,7 +405,11 @@ public:
         API functions which take one.
     */
 
-    virtual ~PLUGIN() {};
+    virtual ~PLUGIN()
+    {
+        //printf( "~%s", __func__ );
+    };
+
 
     /**
      * Class RELEASER
@@ -419,6 +423,9 @@ public:
         // private assignment operator so it's illegal
         RELEASER& operator=( RELEASER& aOther ) { return *this; }
 
+        // private copy constructor so it's illegal
+        RELEASER( const RELEASER& aOther ) {}
+
     public:
         RELEASER( PLUGIN* aPlugin = NULL ) :
             plugin( aPlugin )