[Aptitude-devel] Changesets 00288a0dcae2 through 4710b5c94fcc

[Daniel Burrows]

  Hi everyone,

  I'm back from vacation, and I've been trying to catch up on the work
that went on while I was gone.

changeset:   2205:00288a0dcae2
Drop the package actions menu bar menu.
 Secondary actions should be placed in the description pane or info tab.
 The first intention was to have a duplicate of the contextual popup menu in 
 the menu bar as Synaptic or the ncurses version of Aptitude for that matter.
 The tabbed design of the GTK Aptitude however makes this very difficult.
 Determining which packages in which entityview in which tab is supposed to get
 in and out of focus precisely would involve adding quite a lot of code.
====

  Right now we have several almost-redundant ways of doing the same
thing.  The context menu and Package menu are identical to each other,
and the big fat buttons in the description area do a subtly different
thing (when several packages are selected, the menus act on all of them
but the buttons only act on the one whose description is displayed).
Leaving technical issues aside, is this too much redundancy in the UI,
or is shoving the actions in the user's face more important?

  Would it perhaps be better to have a second "package" toolbar?  I
think I decided against this because I felt like it would inevitably be
too cryptic: the great thing about the buttons is that they say exactly
what they do.

changeset:   2205:00288a0dcae2
description:
Implemented a basic duplicate of the contextual popup menu in the menubar.
 This is a reversal from my previous commit.
====

  In addition to reversing the previous commit, this introduces a bunch
of new code:

====
+  void EntityTreeView::on_cursor_changed()
+  {
+    // TODO: we might plan to do some more elaborate filtering
+    //       based on which tab is being active. If not, we may
+    //       as well revert to a simple signal instead of this
+    //       virtual function.
+    signal_selection_change();
+  }
+
====

  This seems like a completely unused and undocumented function, maybe
left over from some intermediate version of the source tree.  Can it
just be pruned?

  Also, the above comment is wrong: on_cursor_changed isn't a virtual
function.

====
   Gtk::Menu *
   EntityView::get_menu(const std::set<PackagesAction> &actions,
-                      const sigc::slot1<void, PackagesAction> &callback) co
nst
+                      const sigc::slot1<void, PackagesAction> &callback,
+                      Gtk::Menu * rval) const
   {
-    Gtk::Menu *rval(manage(new Gtk::Menu));
+    if (rval == 0)
+    {
+      rval = manage(new Gtk::Menu);
+    }
====

  I don't like this idiom: it adds an unnecessary complication to the
interface of get_menu().  I've changed get_menu() to fill_menu(),
which takes a menu as a parameter and inserts actions into it.

====
+  void EntityView::package_menu_handler()
+  {
+    Glib::RefPtr<Gtk::TreeModel> model = get_model();
+    Glib::RefPtr<Gtk::TreeView::Selection> selected = tree->get_selection();
+    if(selected)
+      {
+        std::set<PackagesAction> actions;
+        std::set<pkgCache::PkgIterator> packages;
+
+        Gtk::TreeSelection::ListHandle_Path selected_rows = selected->get_selected_rows();
+        for (Gtk::TreeSelection::ListHandle_Path::iterator path = selected_rows.begin();
+             path != selected_rows.end(); ++path)
+          {
+            Gtk::TreeModel::iterator iter = model->get_iter(*path);
+            cwidget::util::ref_ptr<Entity> ent = (*iter)[cols.EntObject];
+            ent->add_actions(actions);
+            ent->add_packages(packages);
+          }
+
+        std::cout << "package menu handler : ";
+        for (std::set<pkgCache::PkgIterator>::iterator it = packages.begin();
+        it != packages.end(); ++it)
+        {
+          std::cout << " " << it->Name();
+        }
+
+        std::cout << std::endl;
+
+        if(!actions.empty())
+          {
+            get_menu(actions, sigc::mem_fun(this, &EntityView::apply_action_to_selected), pMainWindow->get_menu_package());
           }
       }
   }
====

  This looks good, except that it unnecessarily builds a set of the
packages it's acting on and then writes them to standard output.  I
presume that this is left-over debugging code.  We could use that set
to rebuild the menu when the package states in question change -- but I
think we'd be better off always rebuilding the menu whenever any package
states change (less book-keeping and should be reasonably inexpensive).
I've committed that change (renaming the handler for more clarity at the
same time), so the Package menu updates correctly now.

  The Package menu still doesn't apply to the currently active tab.  I
think that in order to handle this, we'll have to allow tabs to know
whether they're currently selected.  I'll implement a flag on each Tab,
along with signals when a tab is selected or deselected (with a
guarantee that deselection is sent before selection).

  I suspect that we should also move fill_menu() to the main window
class or to gui.cc.  It's quite generic anyway, and we might want to
generate the Package menu from some other source.  Working on that right
now.

  I also noticed a lot of this:

(aptitude:7476): glibmm-CRITICAL **: Glib::ObjectBase* Glib::wrap_create_new_wrapper(GObject*): assertion `wrap_func_table != 0' failed

(aptitude:7476): glibmm-WARNING **: Failed to wrap object of type 'GtkAccelLabel'. Hint: this error is commonly caused by failing to call a library init() function.

(aptitude:7476): Gtk-CRITICAL **: gtk_bin_remove: assertion `GTK_IS_WIDGET (child)' failed

  I'm not sure where it's coming from, but it should be tracked down fixed.

  Daniel