[Aptitude-devel] 002-qt-stubs review

[Daniel Burrows]

On Wed, Jul 14, 2010 at 02:09:15AM +0200, Piotr Galiszewski <piotr at galiszewski.pl> was heard to say:
> 2010/7/13 Daniel Burrows <dburrows at google.com>:
> >  If Qt has no way of abstracting over slots, that would make this
> > difficult.  In that case maybe you could just make a public signal for
> > now?
> >
> 
> There is another problem. Qt moc requires that all declaration of
> signals and slots have to be placed in header file.

  That's kind of awkward...

> Due to that, completely hiding implementation will not work and there
> have to be another header. I can create tabs_manager_impl.h header,
> but I do not know if it is acceptable solution. Another solutions are
> not using normal class as it was written before, or something different
> than Qt signals and slots mechanism. Maybe I am wrong, but my google
> skill is to low to find better solution :/

  I don't have a problem with using the sigc++ mechanisms more broadly,
but I had the impression from you and Sune that this was strongly
dispreferred.

  If it's just a signal, could you leave it in the header despite the
rest of the class being abstract?  I guess that means that other code
could emit the signal, but other than that it should be OK for now...

> Second problem is that I do not know for now, how to force one
> packages_tab per window. Probably I will delay it for some later patch

  Assuming you identify tab_widgets with windows, what about just a map
storing the packages tabs that are active on each widget?

    boost::unordered_map<tab_widget *, boost::unordered_set<packages_tab *> >
      active_packages_tabs;

    ...

    void register_packages_tab(tab_widget *tabs, packages_tab *pkg_tab)
    {
      active_packages_tabs[tabs].insert(pkg_tab);

      update_packages_tabs_closability(tabs);
    }

    void unregister_packages_tab(tab_widget *tabs, packages_tab *pkg_tab)
    {
      active_package_tabs[tabs].erase(pkg_tab);

      update_packages_tabs_closability(tabs);
    }

    void update_packages_tabs_closability(tab_widget *tabs)
    {
      const boost::unordered_set<packages_tab *> &packages_tabs =
          active_packages_tabs[tabs];

      if(packages_tabs.size() == 1)
        packages_tabs.begin()->set_closable(false);
      else
        {
          // TODO: only do this on the transition between 1 and 2, to
          // avoid excessive numbers of set_closable() calls with more
          // tabs.
          for(boost::unordered_set<packages_tab *>::const_iterator it =
                packages_tabs.begin(); it != packages_tabs.end(); ++it)
            it->set_closable(true);
        }
    }

  (this would be a really good candidate for unit-testing, with a
   little refactoring to hide the widgets behind abstract interfaces
   so that the unit test didn't have to use GUI objects directly ...
   if such a thing is possible with the way Qt works, anyway)

  Daniel