Make download_status_display part of the command-line code.

It's really just a hack to break the command-line implementation up in a
way that's more amenable to unit-testing; it doesn't belong in the generic
view interface.

7 files changed, 146 insertions, 52 deletions

diff --git a/src/cmdline/cmdline_download_progress_display.cc b/src/cmdline/cmdline_download_progress_display.cc
index acd83147..15febc6f 100644
--- a/src/cmdline/cmdline_download_progress_display.cc
+++ b/src/cmdline/cmdline_download_progress_display.cc
@@ -55,18 +55,18 @@ namespace aptitude
       {
         bool display_messages;
         shared_ptr<transient_message> message;
-        shared_ptr<views::download_status_display> status_display;
+        shared_ptr<download_status_display> status_display;
         shared_ptr<terminal_input> term_input;
 
         download_progress(bool _display_messages,
                           const shared_ptr<transient_message> &_message,
-                          const shared_ptr<views::download_status_display> &_status_display,
+                          const shared_ptr<download_status_display> &_status_display,
                           const shared_ptr<terminal_input> &_term_input);
 
         friend shared_ptr<download_progress>
         make_shared<download_progress>(const bool &,
                                        const shared_ptr<transient_message> &,
-                                       const shared_ptr<views::download_status_display> &,
+                                       const shared_ptr<download_status_display> &,
                                        const shared_ptr<terminal_input> &);
 
       public:
@@ -74,23 +74,19 @@ namespace aptitude
 
         void file_started(const std::string &description,
                           const boost::optional<unsigned long> &id,
-                          const boost::optional<unsigned long> &file_size,
-                          const status &current_status);
+                          const boost::optional<unsigned long> &file_size);
 
         void file_already_downloaded(const std::string &description,
                                      const boost::optional<unsigned long> &id,
-                                     const boost::optional<unsigned long> &file_size,
-                                     const status &current_status);
+                                     const boost::optional<unsigned long> &file_size);
 
         void error(bool ignored,
                    const std::string &error,
                    const std::string &description,
-                   const boost::optional<unsigned long> &id,
-                   const status &current_status);
+                   const boost::optional<unsigned long> &id);
 
         void file_finished(const std::string &description,
-                           const boost::optional<unsigned long> &id,
-                           const status &current_status);
+                           const boost::optional<unsigned long> &id);
 
         void done(double fetched_bytes,
                   unsigned long elapsed_time,
@@ -107,7 +103,7 @@ namespace aptitude
 
       download_progress::download_progress(bool _display_messages,
                                            const shared_ptr<transient_message> &_message,
-                                           const shared_ptr<views::download_status_display> &_status_display,
+                                           const shared_ptr<download_status_display> &_status_display,
                                            const shared_ptr<terminal_input> &_term_input)
         : display_messages(_display_messages),
           message(_message),
@@ -126,8 +122,7 @@ namespace aptitude
 
       void download_progress::file_started(const std::string &description,
                                            const boost::optional<unsigned long> &id,
-                                           const boost::optional<unsigned long> &file_size,
-                                           const status &current_status)
+                                           const boost::optional<unsigned long> &file_size)
       {
         if(display_messages)
           {
@@ -146,13 +141,11 @@ namespace aptitude
 
             message->display_and_advance(transcode(join(entries, " ")));
           }
-        status_display->display_status(current_status);
       }
 
       void download_progress::file_already_downloaded(const std::string &description,
                                                       const boost::optional<unsigned long> &id,
-                                                      const boost::optional<unsigned long> &file_size,
-                                                      const status &current_status)
+                                                      const boost::optional<unsigned long> &file_size)
       {
         if(display_messages)
           {
@@ -171,14 +164,12 @@ namespace aptitude
 
             message->display_and_advance(transcode(join(entries, " ")));
           }
-        status_display->display_status(current_status);
       }
 
       void download_progress::error(bool ignored,
                                     const std::string &error,
                                     const std::string &description,
-                                    const boost::optional<unsigned long> &id,
-                                    const status &current_status)
+                                    const boost::optional<unsigned long> &id)
       {
         if(display_messages)
           {
@@ -201,15 +192,11 @@ namespace aptitude
             if(!ignored && !error.empty())
               message->display_and_advance(transcode("  " + error));
           }
-
-        status_display->display_status(current_status);
       }
 
       void download_progress::file_finished(const std::string &description,
-                                            const boost::optional<unsigned long> &id,
-                                            const status &current_status)
+                                            const boost::optional<unsigned long> &id)
       {
-        status_display->display_status(current_status);
       }
 
       void download_progress::done(double fetched_bytes,
@@ -263,7 +250,7 @@ namespace aptitude
       {
       }
 
-      class dummy_status_display : public views::download_status_display
+      class dummy_status_display : public download_status_display
       {
         dummy_status_display();
 
@@ -282,28 +269,28 @@ namespace aptitude
       {
       }
 
-      class download_status_display : public views::download_status_display
+      class download_status_display_impl : public download_status_display
       {
         shared_ptr<transient_message> message;
         shared_ptr<terminal_locale> term_locale;
         shared_ptr<terminal_metrics> term_metrics;
 
-        download_status_display(const shared_ptr<transient_message> &_message,
-                                const shared_ptr<terminal_locale> &_term_locale,
-                                const shared_ptr<terminal_metrics> &_term_metrics);
+        download_status_display_impl(const shared_ptr<transient_message> &_message,
+                                     const shared_ptr<terminal_locale> &_term_locale,
+                                     const shared_ptr<terminal_metrics> &_term_metrics);
 
-        friend shared_ptr<download_status_display>
-        make_shared<download_status_display>(const shared_ptr<transient_message> &,
-                                             const shared_ptr<terminal_locale> &,
-                                             const shared_ptr<terminal_metrics> &);
+        friend shared_ptr<download_status_display_impl>
+        make_shared<download_status_display_impl>(const shared_ptr<transient_message> &,
+                                                  const shared_ptr<terminal_locale> &,
+                                                  const shared_ptr<terminal_metrics> &);
 
       public:
         void display_status(const download_progress::status &status);
       };
 
-      download_status_display::download_status_display(const shared_ptr<transient_message> &_message,
-                                                       const shared_ptr<terminal_locale> &_term_locale,
-                                                       const shared_ptr<terminal_metrics> &_term_metrics)
+      download_status_display_impl::download_status_display_impl(const shared_ptr<transient_message> &_message,
+                                                                 const shared_ptr<terminal_locale> &_term_locale,
+                                                                 const shared_ptr<terminal_metrics> &_term_metrics)
         : message(_message),
           term_locale(_term_locale),
           term_metrics(_term_metrics)
@@ -429,7 +416,7 @@ namespace aptitude
           }
       }
 
-      void download_status_display::display_status(const download_progress::status &status)
+      void download_status_display_impl::display_status(const download_progress::status &status)
       {
         typedef views::download_progress::status::worker_status worker_status;
         const double download_rate = status.get_download_rate();
@@ -535,9 +522,13 @@ namespace aptitude
       }
     }
 
+    download_status_display::~download_status_display()
+    {
+    }
+
     shared_ptr<views::download_progress>
     create_download_progress_display(const boost::shared_ptr<transient_message> &message,
-                                     const boost::shared_ptr<views::download_status_display> &status_display,
+                                     const boost::shared_ptr<download_status_display> &status_display,
                                      const boost::shared_ptr<terminal_input> &term_input,
                                      bool display_messages)
     {
@@ -547,7 +538,7 @@ namespace aptitude
                                             term_input);
     }
 
-    shared_ptr<views::download_status_display>
+    shared_ptr<download_status_display>
     create_cmdline_download_status_display(const shared_ptr<transient_message> &message,
                                            const shared_ptr<terminal_locale> &term_locale,
                                            const shared_ptr<terminal_metrics> &term_metrics,
@@ -556,9 +547,9 @@ namespace aptitude
       if(hide_status)
         return make_shared<dummy_status_display>();
       else
-        return make_shared<download_status_display>(message,
-                                                    term_locale,
-                                                    term_metrics);
+        return make_shared<download_status_display_impl>(message,
+                                                         term_locale,
+                                                         term_metrics);
     }
   }
 }
diff --git a/src/cmdline/cmdline_download_progress_display.h b/src/cmdline/cmdline_download_progress_display.h
index 4eaf848e..d1f9b46c 100644
--- a/src/cmdline/cmdline_download_progress_display.h
+++ b/src/cmdline/cmdline_download_progress_display.h
@@ -20,17 +20,14 @@
 #ifndef CMDLINE_DOWNLOAD_PROGRESS_DISPLAY
 #define CMDLINE_DOWNLOAD_PROGRESS_DISPLAY
 
+// Local includes:
+#include <generic/views/download_progress.h>
+
 // System includes:
 #include <boost/shared_ptr.hpp>
 
 namespace aptitude
 {
-  namespace views
-  {
-    class download_progress;
-    class download_status_display;
-  }
-
   namespace cmdline
   {
     class terminal_input;
@@ -38,6 +35,23 @@ namespace aptitude
     class terminal_metrics;
     class transient_message;
 
+    /** \brief Used to display the current status of a command-line download.
+     *
+     *  This class exists for purely pragmatic reasons.
+     *
+     *  Splitting it out reduces the number of cases I need to check
+     *  in the unit tests (display_messages and hide_status can be
+     *  tested separately instead of having to test all four variants
+     *  of them).
+     */
+    class download_status_display
+    {
+    public:
+      virtual ~download_status_display();
+
+      virtual void display_status(const views::download_progress::status &status) = 0;
+    };
+
     /**
      * Create a new command-line download progress display.
      *
@@ -49,7 +63,7 @@ namespace aptitude
      */
     boost::shared_ptr<views::download_progress>
     create_download_progress_display(const boost::shared_ptr<transient_message> &message,
-                                     const boost::shared_ptr<views::download_status_display> &status_display,
+                                     const boost::shared_ptr<download_status_display> &status_display,
                                      const boost::shared_ptr<terminal_input> &term_input,
                                      bool display_messages);
 
@@ -68,7 +82,7 @@ namespace aptitude
      *        it makes testing more effective; the callers of this code aren't
      *        currently testable.
      */
-    boost::shared_ptr<views::download_status_display>
+    boost::shared_ptr<download_status_display>
     create_cmdline_download_status_display(const boost::shared_ptr<transient_message> &message,
                                            const boost::shared_ptr<terminal_locale> &term_locale,
                                            const boost::shared_ptr<terminal_metrics> &term_metrics,
diff --git a/src/cmdline/cmdline_progress.cc b/src/cmdline/cmdline_progress.cc
index 9aa08cea..27368acc 100644
--- a/src/cmdline/cmdline_progress.cc
+++ b/src/cmdline/cmdline_progress.cc
@@ -38,6 +38,7 @@
 #include <sigc++/functors/mem_fun.h>
 #include <sigc++/functors/ptr_fun.h>
 
+using aptitude::cmdline::download_status_display;
 using aptitude::controllers::acquire_download_progress;
 using aptitude::controllers::create_acquire_download_progress;
 using boost::shared_ptr;
@@ -61,7 +62,7 @@ namespace aptitude
       const shared_ptr<transient_message> message =
         create_transient_message(term_locale, term_metrics, term_output);
 
-      const shared_ptr<views::download_status_display> download_status =
+      const shared_ptr<download_status_display> download_status =
         create_cmdline_download_status_display(message,
                                                term_locale,
                                                term_metrics,
diff --git a/src/cmdline/mocks/Makefile.am b/src/cmdline/mocks/Makefile.am
index 2721d411..60291a93 100644
--- a/src/cmdline/mocks/Makefile.am
+++ b/src/cmdline/mocks/Makefile.am
@@ -5,6 +5,8 @@ noinst_LIBRARIES = libcmdline-mocks.a
 libcmdline_mocks_a_SOURCES = \
 	cmdline_progress_display.h \
 	cmdline_progress_throttle.h \
+	download_status_display.cc \
+	download_status_display.h \
 	teletype.cc \
 	teletype.h \
 	terminal.cc \
diff --git a/src/cmdline/mocks/SConscript b/src/cmdline/mocks/SConscript
index 942de7be..ae8662ef 100644
--- a/src/cmdline/mocks/SConscript
+++ b/src/cmdline/mocks/SConscript
@@ -7,6 +7,8 @@ srcs = map(File, [
 programs_env.Dist(
     'cmdline_progress_display.h',
     'cmdline_progress_throttle.h',
+    'download_status_display.cc',
+    'download_status_display.h',
     'teletype.cc',
     'teletype.h',
     'terminal.cc',
diff --git a/src/cmdline/mocks/download_status_display.cc b/src/cmdline/mocks/download_status_display.cc
new file mode 100644
index 00000000..39c74eb4
--- /dev/null
+++ b/src/cmdline/mocks/download_status_display.cc
@@ -0,0 +1,34 @@
+/** \file download_status_display.cc */
+
+// Copyright (C) 2010 Daniel Burrows
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License as
+// published by the Free Software Foundation; either version 2 of the
+// License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; see the file COPYING.  If not, write to
+// the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+// Boston, MA 02111-1307, USA.
+
+// Local includes:
+#include "download_status_display.h"
+
+namespace aptitude
+{
+  namespace cmdline
+  {
+    namespace mocks
+    {
+      download_status_display::download_status_display()
+      {
+      }
+    }
+  }
+}
diff --git a/src/cmdline/mocks/download_status_display.h b/src/cmdline/mocks/download_status_display.h
new file mode 100644
index 00000000..fbf66845
--- /dev/null
+++ b/src/cmdline/mocks/download_status_display.h
@@ -0,0 +1,50 @@
+/** \file download_status_display.h */ // -*-c++-*-
+
+// Copyright (C) 2010 Daniel Burrows
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License as
+// published by the Free Software Foundation; either version 2 of the
+// License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; see the file COPYING.  If not, write to
+// the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+// Boston, MA 02111-1307, USA.
+
+#ifndef APTITUDE_CMDLINE_MOCKS_DOWNLOAD_STATUS_DISPLAY_H
+#define APTITUDE_CMDLINE_MOCKS_DOWNLOAD_STATUS_DISPLAY_H
+
+// Local includes:
+#include <cmdline/cmdline_download_progress_display.h>
+
+#include <generic/util/mocks/mock_util.h>
+
+namespace aptitude
+{
+  namespace cmdline
+  {
+    namespace mocks
+    {
+      class download_status_display : public cmdline::download_status_display,
+                                      public util::mocks::Mock<download_status_display>
+      {
+        download_status_display();
+        friend boost::shared_ptr<download_status_display>
+        boost::make_shared<download_status_display>();
+
+        MOCK_FRIENDS();
+
+      public:
+        MOCK_METHOD1(display_status, void(const views::download_progress::status &));
+      };
+    }
+  }
+}
+
+#endif // APTITUDE_CMDLINE_MOCKS_DOWNLOAD_STATUS_DISPLAY_H