[Aptitude-devel] Review of c6d34c.

[Daniel Burrows]

  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