GNOME.org
 

[glom/gtktoolpallete] FlowTable: Simplify the remove() code.

commit 7e0da7b202b86521547b4074cb37e1c10b714555
Author: Murray Cumming <murrayc murrayc com>
Date:   Wed Dec 30 12:45:42 2009 +0100

    FlowTable: Simplify the remove() code.
    
    * glom/utility_widgets/flowtable.[h|cc]: Remove the 2 remove() methods,
    because the underlying Gtk::Container::remove() is enough.
    on_remove(): Remove the item if both widgets have been removed.
    * glom/utility_widgets/test_flowtable.cc: Adapt now that
    remove(first, second) is gone.

 ChangeLog                              |   16 +++++
 glom/mode_data/flowtablewithfields.cc  |   96 +++++++++++++++++++++++---------
 glom/mode_data/flowtablewithfields.h   |   17 ++++-
 glom/utility_widgets/flowtable.cc      |   97 +++++++++-----------------------
 glom/utility_widgets/flowtable.h       |    6 --
 glom/utility_widgets/test_flowtable.cc |    7 ++-
 6 files changed, 130 insertions(+), 109 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 99666ea..4452792 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2009-12-30  Murray Cumming  <murrayc murrayc-desktop>
+
+	* glom/mode_data/flowtablewithfields.[h|cc]: Remember the child widgets 
+	in a special list and delete them simply in remove_all(). Destructor: 
+	Call remove_all().
+
+2009-12-30  Murray Cumming  <murrayc murrayc com>>
+
+	FlowTable: Simplify the remove() code.
+
+	* glom/utility_widgets/flowtable.[h|cc]: Remove the 2 remove() methods, 
+	because the underlying Gtk::Container::remove() is enough.
+	on_remove(): Remove the item if both widgets have been removed.
+	* glom/utility_widgets/test_flowtable.cc: Adapt now that 
+	remove(first, second) is gone.
+
 2009-12-30  Murray Cumming  <murrayc murrayc com>
 
 	Fix the build with fatal warnings.
diff --git a/glom/mode_data/flowtablewithfields.cc b/glom/mode_data/flowtablewithfields.cc
index 91ed7d6..2955286 100644
--- a/glom/mode_data/flowtablewithfields.cc
+++ b/glom/mode_data/flowtablewithfields.cc
@@ -69,17 +69,9 @@ FlowTableWithFields::FlowTableWithFields(const Glib::ustring& table_name)
 
 FlowTableWithFields::~FlowTableWithFields()
 {
-  //Remove views. The widgets are deleted automatically because they are managed.
-  for(type_listFields::iterator iter = m_listFields.begin(); iter != m_listFields.end(); ++iter)
-  {
-    View_Composite_Glom* pViewFirst = dynamic_cast<View_Composite_Glom*>(iter->m_first);
-    if(pViewFirst)
-      remove_view(pViewFirst);
-
-   View_Composite_Glom* pViewSecond = dynamic_cast<View_Composite_Glom*>(iter->m_second);
-    if(pViewSecond)
-      remove_view(pViewSecond);
-  }
+  forget_views();
+  //We don't need to call delete_all_child_widgets() because managed widgets 
+  //will be deleted anyway, and maybe already were.
 }
 
 void FlowTableWithFields::set_table(const Glib::ustring& table_name)
@@ -240,6 +232,7 @@ void FlowTableWithFields::add_layout_group_at_position(const sharedptr<LayoutGro
     }
     
     add(*frame, true /* expand */);
+    m_child_widgets.push_back(frame); //Remember it, to delete it later.
 
     m_sub_flow_tables.push_back(flow_table);
     flow_table->set_layout_item(group, m_table_name);
@@ -344,6 +337,7 @@ void FlowTableWithFields::add_layout_portal_at_position(const sharedptr<LayoutIt
   if(portal_box)
   {
     add(*portal_box, true /* expand */);
+    m_child_widgets.push_back(portal_box); //Remember it, to delete it later.
     add_layoutwidgetbase(portal_box, add_before);
   }
   else
@@ -459,8 +453,10 @@ void FlowTableWithFields::add_layout_notebook_at_position(const sharedptr<Layout
 
   if(widget)
     insert_before(*notebook_widget, *widget, true /* expand */);
-  else
+  else {
     add(*notebook_widget, true /* expand */);
+    m_child_widgets.push_back(notebook_widget); //Remember it, to delete it later.
+  }
 }
 
 /*
@@ -574,7 +570,11 @@ void FlowTableWithFields::add_field_at_position(const sharedptr<LayoutItem_Field
   if(widget)
     insert_before(*eventbox, *(info.m_second), *widget, expand_second);
   else
+  {
     add(*eventbox, *(info.m_second), expand_second);
+    m_child_widgets.push_back(eventbox); //Remember it, to delete it later.
+    m_child_widgets.push_back(info.m_second); //Remember it, to delete it later.
+  }
 
   info.m_second->signal_edited().connect( sigc::bind(sigc::mem_fun(*this, &FlowTableWithFields::on_entry_edited), layoutitem_field)  ); //TODO:  Is it a good idea to bind the LayoutItem? sigc::bind() probably stores a copy at this point.
 
@@ -611,8 +611,10 @@ void FlowTableWithFields::add_button_at_position(const sharedptr<LayoutItem_Butt
 
   if(widget)
     insert_before (*button, *widget, false /* expand */);
-  else
+  else {
     add(*button, false /* expand */);
+    m_child_widgets.push_back(button); //Remember it, to delete it later.
+  }
 }
 
 void FlowTableWithFields::add_textobject_at_position(const sharedptr<LayoutItem_Text>& layoutitem_text, const Glib::ustring& table_name , const type_list_layoutwidgets::iterator& add_before)
@@ -642,7 +644,10 @@ void FlowTableWithFields::add_textobject_at_position(const sharedptr<LayoutItem_
     if(widget)
       insert_before(*alignment_label, *widget, false /* expand */);
     else
+    {
       add(*alignment_label, false /* expand */);
+      m_child_widgets.push_back(alignment_label); //Remember it, to delete it later.
+    }
   }
   else
   {
@@ -663,7 +668,11 @@ void FlowTableWithFields::add_textobject_at_position(const sharedptr<LayoutItem_
     if(widget)
       insert_before (*alignment_title, *alignment_label, *widget, false /* expand */);
     else
+    {
       add(*alignment_title, *alignment_label, false /* expand */);
+      m_child_widgets.push_back(alignment_title); //Remember it, to delete it later.
+      m_child_widgets.push_back(alignment_label); //Remember it, to delete it later.
+    }
   }
 }
 
@@ -681,7 +690,10 @@ void FlowTableWithFields::add_placeholder_at_position(const sharedptr<LayoutItem
   if(widget)
     insert_before(m_placeholder_alignment, *widget, false /* expand */);
   else
+  {
     add(m_placeholder_alignment, false);
+    //m_child_widgets.push_back(m_placeholder_alignment); //Remember it, to delete it later.
+  }
 }
 
 void FlowTableWithFields::add_imageobject_at_position(const sharedptr<LayoutItem_Image>& layoutitem_image, const Glib::ustring& table_name , const type_list_layoutwidgets::iterator& add_before)
@@ -707,7 +719,10 @@ void FlowTableWithFields::add_imageobject_at_position(const sharedptr<LayoutItem
     if(widget)
       insert_before(*image, *widget, true /* expand */);
     else
+    {
       add(*image, true /* expand */);
+      m_child_widgets.push_back(image); //Remember it, to delete it later.
+    }
   }
   else
   {
@@ -726,7 +741,11 @@ void FlowTableWithFields::add_imageobject_at_position(const sharedptr<LayoutItem
     if(widget)
       insert_before(*alignment_title, *image, *widget, true /* expand */);
     else
+    {
       add(*alignment_title, *image, true /* expand */);
+      m_child_widgets.push_back(alignment_title); //Remember it, to delete it later.
+      m_child_widgets.push_back(image); //Remember it, to delete it later.
+    }
   }
 }
 
@@ -974,22 +993,16 @@ void FlowTableWithFields::change_group(const Glib::ustring& /* id */, const Glib
   //TODO.
 }
 
-void FlowTableWithFields::remove_all()
+void FlowTableWithFields::forget_views()
 {
-  m_listFields.clear();
-
   for(type_sub_flow_tables::iterator iter = m_sub_flow_tables.begin(); iter != m_sub_flow_tables.end(); ++iter)
   {
     FlowTableWithFields* pSub = *iter;
     if(pSub)
     {
       remove_view(*iter);
-      remove(*pSub);
-
-      delete pSub;
     }
   }
- 
   m_sub_flow_tables.clear();
 
 
@@ -997,26 +1010,57 @@ void FlowTableWithFields::remove_all()
   {
     Box_Data_Portal* pPortal = *iter;
     remove_view(pPortal);
-    remove(*pPortal);
-    delete pPortal;
   }
   m_portals.clear();
 
   m_list_layoutwidgets.clear();
 
-  //Remove views. The widgets are deleted automatically because they are managed.
+  //Remove views:
   for(type_listFields::iterator iter = m_listFields.begin(); iter != m_listFields.end(); ++iter)
   {
-    View_Composite_Glom* pViewFirst = dynamic_cast<View_Composite_Glom*>(iter->m_first);
+    Gtk::Label* label = iter->m_first;
+    View_Composite_Glom* pViewFirst = dynamic_cast<View_Composite_Glom*>(label);
     if(pViewFirst)
       remove_view(pViewFirst);
 
-   View_Composite_Glom* pViewSecond = dynamic_cast<View_Composite_Glom*>(iter->m_second);
+    DataWidget* datawidget = iter->m_second;
+    View_Composite_Glom* pViewSecond = dynamic_cast<View_Composite_Glom*>(datawidget);
     if(pViewSecond)
       remove_view(pViewSecond);
   }
+  m_listFields.clear();
+
+
+  for(type_portals::iterator iter = m_portals.begin(); iter != m_portals.end(); ++iter)
+  {
+    Box_Data_Portal* pPortal = *iter;
+    remove_view(pPortal);
+  }
+  m_portals.clear();
+}
 
-  FlowTable::remove_all();
+void FlowTableWithFields::delete_all_child_widgets()
+{
+  //Remove and delete the widgets.
+  //Any managed child widgets will be deleted too:
+  //We don't just use get_children() because there could be other children 
+  //in the base class that don't belong to us.
+  for(type_list_widgets::iterator iter = m_child_widgets.begin(); iter != m_child_widgets.end(); ++iter)
+  {
+    Gtk::Widget* widget = *iter;
+    remove(*widget);
+    delete widget;
+  }
+  m_child_widgets.clear();
+}
+
+void FlowTableWithFields::remove_all()
+{
+  //Forget other information,
+  //and actually delete the widgets (as documented for this method):
+  //making sure to call remove_view() on child views:
+  forget_views();
+  delete_all_child_widgets();
 }
 
 FlowTableWithFields::type_signal_field_edited FlowTableWithFields::signal_field_edited()
diff --git a/glom/mode_data/flowtablewithfields.h b/glom/mode_data/flowtablewithfields.h
index 51842db..c93c841 100644
--- a/glom/mode_data/flowtablewithfields.h
+++ b/glom/mode_data/flowtablewithfields.h
@@ -95,14 +95,13 @@ public:
    */
   void set_other_field_value(const sharedptr<const LayoutItem_Field>& layout_field, const Gnome::Gda::Value& value);
 
-  typedef std::list<Gtk::Widget*> type_list_widgets;
-  typedef std::list<const Gtk::Widget*> type_list_const_widgets;
-
   virtual void change_group(const Glib::ustring& id, const Glib::ustring& new_group);
 
   virtual void set_design_mode(bool value = true);
 
-  virtual void remove_all();
+  /** Remove (and delete) any items.
+   */
+  void remove_all();
 
   /** Get the layout structure, which might have changed in the child widgets since 
    * the whole widget structure was built.
@@ -144,6 +143,9 @@ public:
   
 private:
 
+  typedef std::list<Gtk::Widget*> type_list_widgets;
+  typedef std::list<const Gtk::Widget*> type_list_const_widgets;
+
   // If include_item is set, then the output list will contain field's widget,
   // otherwise not.
   type_list_widgets get_field(const sharedptr<const LayoutItem_Field>& field, bool include_item);
@@ -263,6 +265,13 @@ private:
   virtual void on_menu_delete_activate(); // override this to add a dialog box
   virtual bool on_button_press_event(GdkEventButton *event);
 #endif // !GLOM_ENABLE_CLIENT_ONLY
+
+  void forget_views();
+  void delete_all_child_widgets();
+
+  //Remember the direct child widgets that we create,
+  //so we can delete them later: 
+  type_list_widgets m_child_widgets;
 };
 
 } //namespace Glom
diff --git a/glom/utility_widgets/flowtable.cc b/glom/utility_widgets/flowtable.cc
index b6f7ad8..8003552 100644
--- a/glom/utility_widgets/flowtable.cc
+++ b/glom/utility_widgets/flowtable.cc
@@ -861,11 +861,23 @@ void FlowTable::on_remove(Gtk::Widget* child)
   {
     const bool visible = child_is_visible(child);
 
-  //g_warning("FlowTable::on_remove");
-    for(type_vecChildren::iterator iter = m_children.begin(); iter != m_children.end(); ++iter)
+    //g_warning("FlowTable::on_remove");
+    type_vecChildren::iterator iter = m_children.begin();
+    while(iter != m_children.end())
     {
       FlowTableItem& item = *iter;
 
+      //Forget the item if the child is the only widget in the item:
+      bool forget_item = false;
+      if((iter->m_first == child) && (iter->m_second == 0))
+      {
+        forget_item = true;
+      }
+      else if((iter->m_second == child) && (iter->m_first == 0))
+      {
+        forget_item = true;
+      }
+
       if(item.m_first == child)
       {
             //g_warning("FlowTable::on_remove unparenting first");
@@ -885,6 +897,12 @@ void FlowTable::on_remove(Gtk::Widget* child)
         if(visible)
           queue_resize();
       }
+
+      //Forget the item if all its widgets have been removed:
+      if(forget_item)
+        iter = m_children.erase(iter); //Returns the one after the removed one.
+      else
+        ++iter;
     }
   }
 
@@ -899,11 +917,11 @@ void FlowTable::forall_vfunc(gboolean /* include_internals */, GtkCallback callb
     FlowTableItem item = *iter;
 
     Gtk::Widget* first = item.m_first;
-    if(first)
+    if(first && first->gobj())
       callback(first->gobj(), callback_data);
 
     Gtk::Widget* second = item.m_second;
-    if(second)
+    if(second && second->gobj())
       callback(second->gobj(), callback_data);
   }
 }
@@ -946,80 +964,17 @@ bool FlowTable::child_is_visible(const Gtk::Widget* widget) const
   #endif
 }
 
-void FlowTable::remove(Gtk::Widget& first)
-{
-  std::cout << "DEBUG: FlowTable::remove() 1" << std::endl;
-  //Gtk::Container::remove() does this too. We need to do it here too:
-  if(first.is_managed_())
-    first.reference();
-
-  gtk_widget_unparent(first.gobj());
-
-  for(type_vecChildren::iterator iter = m_children.begin(); iter != m_children.end(); ++iter)
-  {
-    if((iter->m_first == &first) && (iter->m_second == 0))
-    {
-      g_warning("FlowTable::remove(): removing %10X", (guint)&first);
-
-      m_children.erase(iter);
-      break;
-    }
-  }
-
- std::cout << "DEBUG: FlowTable::remove() 2" << std::endl;  
-}
-
-void FlowTable::remove(Gtk::Widget& first, Gtk::Widget& second)
-{
-  std::cout << "DEBUG: FlowTable::remove(2) 1" << std::endl;
-  //Gtk::Container::remove() does this too. We need to do it here too:
-  if(first.is_managed_())
-    first.reference();
-
-  gtk_widget_unparent(first.gobj());
-
-  for(type_vecChildren::iterator iter = m_children.begin(); iter != m_children.end(); ++iter)
-  {
-    if((iter->m_first == &first) && (iter->m_second == &second))
-    {
-      g_warning("FlowTable::remove(2): removing %10X", (guint)&first);
-
-      m_children.erase(iter);
-      break;
-    }
-  }
-
- std::cout << "DEBUG: FlowTable::remove(2) 2" << std::endl;  
-}
-
 void FlowTable::remove_all()
 {
-
-  for(type_vecChildren::iterator iter = m_children.begin(); iter != m_children.end(); ++iter)
+  while(!m_children.empty())
   {
+    type_vecChildren::iterator iter = m_children.begin();
     if(iter->m_first)
-    {
-      Gtk::Widget* widget = iter->m_first;
-
-      if(widget->is_managed_())
-        widget->reference();
-
-      gtk_widget_unparent(GTK_WIDGET(iter->m_first->gobj()));
-    }
+      remove(*(iter->m_first));
 
     if(iter->m_second)
-    {
-      Gtk::Widget* widget = iter->m_second;
-
-      if(widget->is_managed_())
-        widget->reference();
-
-      gtk_widget_unparent(GTK_WIDGET(iter->m_second->gobj()));
-    }
-
+      remove(*(iter->m_second));
   }
-
-  m_children.clear(); 
 }
 
 void FlowTable::on_realize()
diff --git a/glom/utility_widgets/flowtable.h b/glom/utility_widgets/flowtable.h
index c7914d8..17be659 100644
--- a/glom/utility_widgets/flowtable.h
+++ b/glom/utility_widgets/flowtable.h
@@ -40,12 +40,6 @@ public:
   void insert_before(Gtk::Widget& first, Gtk::Widget& second, Gtk::Widget& before, bool expand_second);
   void insert_before(Gtk::Widget& first, Gtk::Widget& before, bool expand);
 
-  /// Remove @a first if it was added without a second.
-  virtual void remove(Gtk::Widget& first); //override
-
-  /// Remove @a first and @a second if they were added together.
-  void remove(Gtk::Widget& first, Gtk::Widget& second);
-
   void set_columns_count(guint value);
 
   /** Sets the padding to put between the columns of widgets.
diff --git a/glom/utility_widgets/test_flowtable.cc b/glom/utility_widgets/test_flowtable.cc
index ea209e0..e81604f 100644
--- a/glom/utility_widgets/test_flowtable.cc
+++ b/glom/utility_widgets/test_flowtable.cc
@@ -84,11 +84,14 @@ main(int argc, char* argv[])
   flowtable.set_design_mode();
   flowtable.show();
 
-  flowtable.remove(*button13, *button14);
+  flowtable.remove(*button13);
+  flowtable.remove(*button14);
   delete button13;
   delete button14;
 
-  //flowtable.remove(*button15);
+  // Gtk::Containers implemented in C++ can't do auto-removal-on-child-destruction 
+  // because that would require a Glib::wrap() on the underlying gobj of a being-deleted instance. 
+  flowtable.remove(*button15);
   delete button15;
 
 //  Glom::DragWindow drag_window;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]