Just a couple minor things. Small enough that I'd be OK with
merging as-is and committing a fix patch on top of this.
> --- a/src/qt/tabs_manager.cc
> +++ b/src/qt/tabs_manager.cc
> @@ -1,4 +1,4 @@
> -/** \file tabs_manager_impl.cc */
> +/** \file tabs_manager.cc */
> //
> // Copyright (C) 2010 Piotr Galiszewski
> //
> @@ -20,6 +20,8 @@
> // Local includes
> #include "tabs_manager.h"
>
> +#include "package.h"
> +
> #include "widgets/tab_widget.h"
> #include "widgets/changes_preview_tab.h"
> #include "widgets/package_info_tab.h"
> @@ -64,10 +66,8 @@ namespace aptitude
>
> boost::unordered_map<tab_widget *, boost::unordered_set<packages_tab *>
> > active_packages_tabs;
>
> - /** \brief List of all opened package_info tabs.
> - * \todo shouldn't this be unordered_map?
> - */
> - boost::unordered_set<package_info_tab *> package_info_tabs;
> + /** \brief List of all opened package_info tabs. */
Document what the keys are (package names).
> + boost::unordered_map<std::string, package_info_tab *>
> package_info_tabs;
>
> /** \brief List of all registered tab_widgets. */
> boost::unordered_set<tab_widget *> tab_widgets;
> @@ -133,7 +133,7 @@ namespace aptitude
> * If a package_info_tab is already displayed for this package, it
> * is reused; otherwise, a new tab is created and displayed.
> */
> - void open_package_info_tab(QWidget *tab_context);
> + void open_package_info_tab(const package_ptr pkg, QWidget
> *tab_context);
Pass pkg by const reference.
>
> /** \brief Display the perform_changes_tab, creating it if it does not
> exist.
> *
> @@ -220,11 +220,31 @@ namespace aptitude
> create_or_display<changes_preview_tab>(changes_preview,
> window->get_tab_widget(), _("Preview"));
> }
>
> - void tabs_manager::tabs_manager_impl::open_package_info_tab(QWidget
> *tab_context)
> + void tabs_manager::tabs_manager_impl::open_package_info_tab(const
> package_ptr pkg, QWidget *tab_context)
Pass pkg by const reference (not noting other occurrences of this).
> {
> main_window *window = qobject_cast<main_window
> *>(tab_context->window());
> if(window == NULL)
> return;
> +
> + const boost::unordered_map<std::string, package_info_tab
> *>::const_iterator
> + found = package_info_tabs.find(pkg->get_name());
> +
> + if(found != package_info_tabs.end())
> + {
> + tab * const searched_tab = found->second;
> + activate_tab(searched_tab);
> + return;
> + }
> +
> + tab_widget *widget = window->get_tab_widget();
> +
> + connect_tab_widget_signals(widget);
> +
> + package_info_tab *new_tab = new package_info_tab(pkg);
> + widget->addTab(new_tab, "Package " +
> QString::fromStdString(pkg->get_name()));
> + widget->setCurrentWidget(new_tab);
Maybe instead defer to activate_tab()?
Daniel
_______________________________________________
Aptitude-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/aptitude-devel
