decouple wxGridTableBase from FP_LIB_TABLE, this was poor information hiding

common/fp_lib_table.cpp

@@ -68,7 +68,7 @@ void FP_LIB_TABLE::Parse( FP_LIB_TABLE_LEXER* in ) throw( IO_ERROR, PARSE_ERROR
 
         in->NeedSYMBOLorNUMBER();
 
-        ROW     row( this );
+        ROW     row;
 
         row.SetNickName( in->FromUTF8() );
 
@@ -112,9 +112,9 @@ void FP_LIB_TABLE::Parse( FP_LIB_TABLE_LEXER* in ) throw( IO_ERROR, PARSE_ERROR
         in->NeedRIGHT();            // terminate the (lib..)
 
         // all nickNames within this table fragment must be unique, so we do not
-        // use doReplace in InsertRow().  However a fallBack table can have a
+        // use doReplace in InsertRow().  (However a fallBack table can have a
         // conflicting nickName and ours will supercede that one since in
-        // FindLib() we search this table before any fall back.
+        // FindLib() we search this table before any fall back.)
         if( !InsertRow( row ) )
         {
             wxString msg = wxString::Format(

include/fp_lib_table.h

@@ -31,7 +31,6 @@
 #include <vector>
 #include <map>
 
-#include <wx/grid.h>
 #include <fp_lib_id.h>
 
 
@@ -80,7 +79,7 @@ class FP_LIB_TABLE_LEXER;
  *
  * @author Wayne Stambaugh
  */
-class FP_LIB_TABLE : public wxGridTableBase
+class FP_LIB_TABLE
 {
     friend class DIALOG_FP_LIB_TABLE;
 
@@ -135,11 +134,6 @@ public:
             return options;
         }
 
-        ~ROW()
-        {
-            // delete lib;
-        }
-
         /**
          * Function Format
          * serializes this object as utf8 text to an OUTPUTFORMATTER, and tries to
@@ -151,11 +145,6 @@ public:
         void Format( OUTPUTFORMATTER* out, int nestLevel ) const
             throw( IO_ERROR );
 
-        ROW( FP_LIB_TABLE* aOwner ) :
-            owner( aOwner )
-            // lib( 0 )
-        {}
-
         /**
          * Function SetNickName
          * changes the logical name of this library, useful for an editor.
@@ -194,7 +183,6 @@ public:
         }
 
     private:
-        FP_LIB_TABLE*   owner;
         wxString        nickName;
         wxString        type;
         wxString        uri;
@@ -279,106 +267,6 @@ public:
      */
     std::vector<wxString> GetLogicalLibs();
 
-    //-----<wxGridTableBase overloads>-------------------------------------------
-
-    int         GetNumberRows () { return rows.size(); }
-    int         GetNumberCols () { return 4; }
-
-    wxString    GetValue( int aRow, int aCol )
-    {
-        if( unsigned( aRow ) < rows.size() )
-        {
-            const ROW&  r  = rows[aRow];
-
-            switch( aCol )
-            {
-            case 0:     return r.GetNickName();
-            case 1:     return r.GetType();
-            case 2:     return r.GetFullURI();
-            case 3:     return r.GetOptions();
-            default:
-                ;       // fall thru to wxEmptyString
-            }
-        }
-
-        return wxEmptyString;
-    }
-
-    void    SetValue( int aRow, int aCol, const wxString &aValue )
-    {
-        if( aCol == 0 )
-        {
-            // when the nickname is changed, there's careful work to do, including
-            // ensuring uniqueness of the nickname.
-        }
-        else if( unsigned( aRow ) < rows.size() )
-        {
-            ROW&  r  = rows[aRow];
-
-            switch( aCol )
-            {
-            case 1:     r.SetType( aValue  );       break;
-            case 2:     r.SetFullURI( aValue );     break;
-            case 3:     r.SetOptions( aValue );     break;
-            }
-        }
-    }
-
-    bool IsEmptyCell( int aRow, int aCol )
-    {
-        if( unsigned( aRow ) < rows.size() )
-            return false;
-        return true;
-    }
-
-    bool InsertRows( size_t aPos = 0, size_t aNumRows = 1 )
-    {
-        if( aPos < rows.size() )
-        {
-            rows.insert( rows.begin() + aPos, aNumRows, ROW( this ) );
-            return true;
-        }
-        return false;
-    }
-
-    bool AppendRows( size_t aNumRows = 1 )
-    {
-        while( aNumRows-- )
-            rows.push_back( ROW( this ) );
-        return true;
-    }
-
-    bool DeleteRows( size_t aPos, size_t aNumRows )
-    {
-        if( aPos + aNumRows <= rows.size() )
-        {
-            ROWS_ITER start = rows.begin() + aPos;
-            rows.erase( start, start + aNumRows );
-            return true;
-        }
-        return false;
-    }
-
-    void Clear()
-    {
-        rows.clear();
-    }
-
-    wxString GetColLabelValue( int aCol )
-    {
-        switch( aCol )
-        {
-        case 0:     return _( "Nickname" );
-        case 1:     return _( "Plugin" );
-        case 2:     return _( "Library Path" );
-        case 3:     return _( "Options" );
-        default:    return wxEmptyString;
-        }
-    }
-
-    //-----</wxGridTableBase overloads>------------------------------------------
-
-
     //----<read accessors>----------------------------------------------------
     // the returning of a const wxString* tells if not found, but might be too
     // promiscuous?
@@ -427,6 +315,7 @@ public:
     void Test();
 #endif
 
+
 protected:  // only a table editor can use these
 
     /**
@@ -448,7 +337,33 @@ protected:  // only a table editor can use these
      */
     ROW* FindRow( const wxString& aNickName ) const;
 
-private:
+    void reindex()
+    {
+        nickIndex.clear();
+
+        for( ROWS_CITER it = rows.begin();  it != rows.end();  ++it )
+            nickIndex.insert( INDEX_VALUE( it->nickName, it - rows.begin() ) );
+    }
+
+    typedef std::vector<ROW>            ROWS;
+    typedef ROWS::iterator              ROWS_ITER;
+    typedef ROWS::const_iterator        ROWS_CITER;
+
+    ROWS           rows;
+
+    /// this is a non-owning index into the ROWS table
+    typedef std::map<wxString,int>      INDEX;              // "int" is std::vector array index
+    typedef INDEX::iterator             INDEX_ITER;
+    typedef INDEX::const_iterator       INDEX_CITER;
+    typedef INDEX::value_type           INDEX_VALUE;
+
+    /// this particular key is the nickName within each row.
+    INDEX           nickIndex;
+
+    FP_LIB_TABLE*  fallBack;
+};
+
+
 
 #if 0   // lets see what we need.
     /**
@@ -479,23 +394,5 @@ private:
     void loadLib( ROW* aRow ) throw( IO_ERROR );
 #endif
 
-    typedef std::vector<ROW>            ROWS;
-    typedef ROWS::iterator              ROWS_ITER;
-    typedef ROWS::const_iterator        ROWS_CITER;
-
-    ROWS           rows;
-
-    /// this is a non-owning index into the ROWS table
-    typedef std::map<wxString,int>      INDEX;              // "int" is std::vector array index
-    typedef INDEX::iterator             INDEX_ITER;
-    typedef INDEX::const_iterator       INDEX_CITER;
-    typedef INDEX::value_type           INDEX_VALUE;
-
-
-    /// the particular key is the nickName within each row.
-    INDEX           nickIndex;
-
-    FP_LIB_TABLE*  fallBack;
-};
 
 #endif  // _FP_LIB_TABLE_H_

pcbnew/dialogs/dialog_fp_lib_table.cpp

@@ -27,6 +27,132 @@
 #include <fctsys.h>
 #include <dialog_fp_lib_table_base.h>
 #include <fp_lib_table.h>
+#include <wx/grid.h>
+#include <wx/grid.h>
+
+
+class FP_TBL_MODEL : public wxGridTableBase, public FP_LIB_TABLE
+{
+public:
+
+    /**
+     * Constructor FP_TBL_MODEL
+     * builds a wxGridTableBase (table model) by wrapping an FP_LIB_TABLE.
+     * @a aFallBackTable.  Loading of this table fragment is done by using Parse().
+     *
+     * @param aFallBackTable is another FP_LIB_TABLE which is searched only when
+     *                       a record is not found in this table.  No ownership is
+     *                       taken of aFallBackTable.
+     */
+    FP_TBL_MODEL( const FP_LIB_TABLE& aTableToEdit ) :
+        FP_LIB_TABLE( aTableToEdit )    // copy constructor
+    {
+    }
+
+    ~FP_TBL_MODEL()
+    {
+        D(printf("%s\n", __func__ );)
+    }
+
+    //-----<wxGridTableBase overloads>-------------------------------------------
+
+    int         GetNumberRows () { return rows.size(); }
+    int         GetNumberCols () { return 4; }
+
+    wxString    GetValue( int aRow, int aCol )
+    {
+        if( unsigned( aRow ) < rows.size() )
+        {
+            const ROW&  r  = rows[aRow];
+
+            switch( aCol )
+            {
+            case 0:     return r.GetNickName();
+            case 1:     return r.GetFullURI();
+            case 2:     return r.GetType();
+            case 3:     return r.GetOptions();
+            default:
+                ;       // fall thru to wxEmptyString
+            }
+        }
+
+        return wxEmptyString;
+    }
+
+    void    SetValue( int aRow, int aCol, const wxString &aValue )
+    {
+        if( unsigned( aRow ) < rows.size() )
+        {
+            ROW&  r  = rows[aRow];
+
+            switch( aCol )
+            {
+            case 0:     r.SetNickName( aValue );    break;
+            case 1:     r.SetType( aValue  );       break;
+            case 2:     r.SetFullURI( aValue );     break;
+            case 3:     r.SetOptions( aValue );     break;
+            }
+        }
+    }
+
+    bool IsEmptyCell( int aRow, int aCol )
+    {
+        if( unsigned( aRow ) < rows.size() )
+            return false;
+        return true;
+    }
+
+    bool InsertRows( size_t aPos = 0, size_t aNumRows = 1 )
+    {
+        if( aPos < rows.size() )
+        {
+            rows.insert( rows.begin() + aPos, aNumRows, ROW() );
+            return true;
+        }
+        return false;
+    }
+
+    bool AppendRows( size_t aNumRows = 1 )
+    {
+        while( aNumRows-- )
+            rows.push_back( ROW() );
+        return true;
+    }
+
+    bool DeleteRows( size_t aPos, size_t aNumRows )
+    {
+        if( aPos + aNumRows <= rows.size() )
+        {
+            ROWS_ITER start = rows.begin() + aPos;
+            rows.erase( start, start + aNumRows );
+            return true;
+        }
+        return false;
+    }
+
+    void Clear()
+    {
+        rows.clear();
+        nickIndex.clear();
+    }
+
+    wxString GetColLabelValue( int aCol )
+    {
+        switch( aCol )
+        {
+        case 0:     return _( "Nickname" );
+        case 1:     return _( "Library Path" );
+        case 2:     return _( "Plugin" );
+        case 3:     return _( "Options" );
+        default:    return wxEmptyString;
+        }
+    }
+
+    //-----</wxGridTableBase overloads>------------------------------------------
+
+};
+
+
 
 /**
  * Class DIALOG_FP_LIB_TABLE
@@ -66,16 +192,16 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
 
     void onCancelButtonClick( wxCommandEvent& event )
     {
-        m_global->rows  = m_orig_global;
-        m_project->rows = m_orig_project;
-
-        // @todo reindex, or add member function for wholesale row replacement
-
         EndModal( wxID_CANCEL );
     }
 
     void onOKButtonClick( wxCommandEvent& event )
     {
+        *m_global  = m_global_model;
+        *m_project = m_project_model;
+
+        // @todo reindex, or add member function for wholesale row replacement
+
         EndModal( wxID_OK );
     }
 
@@ -87,9 +213,9 @@ class DIALOG_FP_LIB_TABLE : public DIALOG_FP_LIB_TABLE_BASE
     FP_LIB_TABLE*       m_global;
     FP_LIB_TABLE*       m_project;
 
-    // local copies are saved and restored if Cancel button.
-    FP_LIB_TABLE::ROWS  m_orig_global;
-    FP_LIB_TABLE::ROWS  m_orig_project;
+    // local copies which are edited, but aborted if Cancel button.
+    FP_TBL_MODEL        m_global_model;
+    FP_TBL_MODEL        m_project_model;
 
     wxGrid*             m_cur_grid;     ///< changed based on tab choice
 
@@ -99,8 +225,8 @@ public:
         DIALOG_FP_LIB_TABLE_BASE( aParent ),
         m_global( aGlobal ),
         m_project( aProject ),
-        m_orig_global( aGlobal->rows ),
-        m_orig_project( aProject->rows )
+        m_global_model( *aGlobal ),
+        m_project_model( *aProject )
     {
         /*
         GetSizer()->SetSizeHints( this );
@@ -111,28 +237,27 @@ public:
 
 #if 1 && defined(DEBUG)
         // put some dummy data into table(s)
-        FP_LIB_TABLE::ROW   row( m_global );
+        FP_LIB_TABLE::ROW   row;
 
         row.SetNickName( wxT( "passives" ) );
         row.SetType( wxT( "kicad" ) );
         row.SetFullURI( wxT( "%G/passives" ) );
         row.SetOptions( wxT( "speed=fast,purpose=testing" ) );
-        m_global->InsertRow( row );
+        m_global_model.InsertRow( row );
 
         row.SetNickName( wxT( "micros" ) );
         row.SetType( wxT( "legacy" ) );
         row.SetFullURI( wxT( "%P/micros" ) );
         row.SetOptions( wxT( "speed=fast,purpose=testing" ) );
-        m_global->InsertRow( row );
+        m_global_model.InsertRow( row );
 
-        row.owner = m_project;
         row.SetFullURI( wxT( "%P/chips" ) );
-        m_project->InsertRow( row );
+        m_project_model.InsertRow( row );
 
 #endif
 
-        m_global_grid->SetTable( m_global );
-        m_project_grid->SetTable( m_project );
+        m_global_grid->SetTable( (wxGridTableBase*) &m_global_model );
+        m_project_grid->SetTable( (wxGridTableBase*) &m_project_model );
 
         //m_global_grid->AutoSize();
         m_global_grid->AutoSizeColumns( false );
@@ -143,6 +268,16 @@ public:
         //m_path_subs_grid->AutoSize();
         m_path_subs_grid->AutoSizeColumns( false );
     }
+
+    ~DIALOG_FP_LIB_TABLE()
+    {
+        // Destroy the gui stuff first, with a goal of destroying the two wxGrids now,
+        // since the ~wxGrid() wants the wxGridTableBase to still be non-destroyed.
+        // Without this call, the wxGridTableBase objects are destroyed first
+        // (i.e. destructor called) and there is a segfault since wxGridTableBase's vtable
+        // is then no longer valid.
+        DestroyChildren();
+    }
 };
 
 

pcbnew/dialogs/dialog_fp_lib_table_base.cpp

@@ -28,8 +28,8 @@ DIALOG_FP_LIB_TABLE_BASE::DIALOG_FP_LIB_TABLE_BASE( wxWindow* parent, wxWindowID
 	m_global_panel = new wxPanel( m_auinotebook, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL );
 	m_global_panel->SetToolTip( _("Module libraries which  are visible for all projects") );
 	
-	wxBoxSizer* m_global_box_sizer;
-	m_global_box_sizer = new wxBoxSizer( wxVERTICAL );
+	wxBoxSizer* m_global_sizer;
+	m_global_sizer = new wxBoxSizer( wxVERTICAL );
 	
 	m_global_grid = new wxGrid( m_global_panel, wxID_ANY, wxDefaultPosition, wxDefaultSize, 0 );
 	
@@ -56,12 +56,12 @@ DIALOG_FP_LIB_TABLE_BASE::DIALOG_FP_LIB_TABLE_BASE( wxWindow* parent, wxWindowID
 	
 	// Cell Defaults
 	m_global_grid->SetDefaultCellAlignment( wxALIGN_LEFT, wxALIGN_TOP );
-	m_global_box_sizer->Add( m_global_grid, 1, wxALL|wxEXPAND, 5 );
+	m_global_sizer->Add( m_global_grid, 1, wxALL|wxEXPAND, 5 );
 	
 	
-	m_global_panel->SetSizer( m_global_box_sizer );
+	m_global_panel->SetSizer( m_global_sizer );
 	m_global_panel->Layout();
-	m_global_box_sizer->Fit( m_global_panel );
+	m_global_sizer->Fit( m_global_panel );
 	m_auinotebook->AddPage( m_global_panel, _("Global Libraries"), true, wxNullBitmap );
 	m_project_panel = new wxPanel( m_auinotebook, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL );
 	wxBoxSizer* m_project_sizer;
@@ -92,7 +92,7 @@ DIALOG_FP_LIB_TABLE_BASE::DIALOG_FP_LIB_TABLE_BASE( wxWindow* parent, wxWindowID
 	
 	// Cell Defaults
 	m_project_grid->SetDefaultCellAlignment( wxALIGN_LEFT, wxALIGN_TOP );
-	m_project_sizer->Add( m_project_grid, 0, wxALL, 5 );
+	m_project_sizer->Add( m_project_grid, 1, wxALL|wxEXPAND, 5 );
 	
 	
 	m_project_panel->SetSizer( m_project_sizer );

pcbnew/dialogs/dialog_fp_lib_table_base.fbp

@@ -431,7 +431,7 @@
                                                     <event name="OnUpdateUI"></event>
                                                     <object class="wxBoxSizer" expanded="1">
                                                         <property name="minimum_size"></property>
-                                                        <property name="name">m_global_box_sizer</property>
+                                                        <property name="name">m_global_sizer</property>
                                                         <property name="orient">wxVERTICAL</property>
                                                         <property name="permission">none</property>
                                                         <object class="sizeritem" expanded="0">
@@ -665,8 +665,8 @@
                                                         <property name="permission">none</property>
                                                         <object class="sizeritem" expanded="0">
                                                             <property name="border">5</property>
-                                                            <property name="flag">wxALL</property>
-                                                            <property name="proportion">0</property>
+                                                            <property name="flag">wxALL|wxEXPAND</property>
+                                                            <property name="proportion">1</property>
                                                             <object class="wxGrid" expanded="0">
                                                                 <property name="BottomDockable">1</property>
                                                                 <property name="LeftDockable">1</property>