gui: Changes based on review feedback

- Use status enum instead of strings for status comparisons.
- Remove "Retrieving instances" message which makes "About" happy when there
  are no instances.
- Fix clang copy elision error.

src/client/cli/main.cpp

@@ -32,7 +32,7 @@ int main(int argc, char* argv[])
     auto term = mp::Terminal::make_terminal();
 
     mp::ClientConfig config{mp::client::get_server_address(), mp::RpcConnectionType::ssl,
-                            std::move(mp::client::get_cert_provider()), term.get()};
+                            mp::client::get_cert_provider(), term.get()};
     mp::Client client{config};
 
     return client.run(QCoreApplication::arguments());

src/client/gui/client_gui.h

@@ -24,7 +24,6 @@
 #include <multipass/rpc/multipass.grpc.pb.h>
 #include <multipass/rpc_connection_type.h>
 
-#include <map>
 #include <memory>
 #include <sstream>
 #include <unordered_map>

src/client/gui/gui_cmd.cpp

@@ -49,15 +49,13 @@ mp::ReturnCode cmd::GuiCmd::run(mp::ArgParser* parser)
 
 void cmd::GuiCmd::create_actions()
 {
-    retrieving_action = tray_icon_menu.addAction("Retrieving instances...");
     about_separator = tray_icon_menu.addSeparator();
     quit_action = tray_icon_menu.addAction("Quit");
 }
 
 void cmd::GuiCmd::update_menu()
 {
     std::vector<std::string> instances_to_remove;
-    tray_icon_menu.removeAction(retrieving_action);
 
     auto reply = list_future.result();
 
@@ -80,22 +78,23 @@ void cmd::GuiCmd::update_menu()
     for (const auto& instance : reply.instances())
     {
         auto name = instance.name();
-        auto state = QString::fromStdString(mp::format::status_string_for(instance.instance_status()));
+        auto state = instance.instance_status();
+        auto state_string = QString::fromStdString(mp::format::status_string_for(state));
 
         auto it = instances_entries.find(name);
 
         if (it != instances_entries.end())
         {
             auto instance_state = it->second.state;
-            if (state == "DELETED")
+            if (state.status() == InstanceStatus::DELETED)
             {
                 instances_entries.erase(name);
             }
-            else if (instance_state != state)
+            else if (instance_state.status() != state.status())
             {
                 auto& instance_menu = instances_entries.at(name).menu;
 
-                instance_menu->setTitle(QString("%1 (%2)").arg(QString::fromStdString(name)).arg(state));
+                instance_menu->setTitle(QString("%1 (%2)").arg(QString::fromStdString(name)).arg(state_string));
                 instance_menu->clear();
                 set_menu_actions_for(name, state);
                 instances_entries[name].state = state;
@@ -104,10 +103,10 @@ void cmd::GuiCmd::update_menu()
             continue;
         }
 
-        if (state != "DELETED")
+        if (state.status() != InstanceStatus::DELETED)
         {
             instances_entries[name].menu =
-                std::make_unique<QMenu>(QString("%1 (%2)").arg(QString::fromStdString(name)).arg(state));
+                std::make_unique<QMenu>(QString("%1 (%2)").arg(QString::fromStdString(name)).arg(state_string));
             set_menu_actions_for(name, state);
             instances_entries[name].state = state;
         }
@@ -149,7 +148,7 @@ void cmd::GuiCmd::create_menu()
 
     QObject::connect(&list_watcher, &QFutureWatcher<ListReply>::finished, this, &GuiCmd::update_menu);
 
-    QObject::connect(&menu_update_timer, &QTimer::timeout, [this]() { initiate_menu_layout(); });
+    QObject::connect(&menu_update_timer, &QTimer::timeout, this, [this] { initiate_menu_layout(); });
 
     // Use a singleShot here to make sure the event loop is running before the quit() runs
     QObject::connect(quit_action, &QAction::triggered, [this] {
@@ -158,7 +157,7 @@ void cmd::GuiCmd::create_menu()
     });
 
     QObject::connect(&version_watcher, &QFutureWatcher<VersionReply>::finished, this, &GuiCmd::update_about_menu);
-    QObject::connect(&about_update_timer, &QTimer::timeout, [this]() { initiate_about_menu_layout(); });
+    QObject::connect(&about_update_timer, &QTimer::timeout, this, [this] { initiate_about_menu_layout(); });
     QObject::connect(&update_action, &QAction::triggered,
                      [this](bool checked) { QDesktopServices::openUrl(QUrl(update_action.whatsThis())); });
 
@@ -187,9 +186,6 @@ void cmd::GuiCmd::initiate_menu_layout()
         tray_icon_menu.removeAction(&failure_action);
     }
 
-    if (instances_entries.empty())
-        tray_icon_menu.insertAction(about_separator, retrieving_action);
-
     if (!list_future.isRunning())
     {
         list_future = QtConcurrent::run(this, &GuiCmd::retrieve_all_instances);
@@ -218,7 +214,6 @@ mp::ListReply cmd::GuiCmd::retrieve_all_instances()
     };
 
     auto on_failure = [this](grpc::Status& status) {
-        tray_icon_menu.removeAction(retrieving_action);
         tray_icon_menu.insertAction(about_separator, &failure_action);
 
         return standard_failure_handler_for(name(), cerr, status);
@@ -230,7 +225,7 @@ mp::ListReply cmd::GuiCmd::retrieve_all_instances()
     return list_reply;
 }
 
-void cmd::GuiCmd::set_menu_actions_for(const std::string& instance_name, const QString& state)
+void cmd::GuiCmd::set_menu_actions_for(const std::string& instance_name, const InstanceStatus& state)
 {
     auto& instance_menu = instances_entries.at(instance_name).menu;
 
@@ -239,13 +234,8 @@ void cmd::GuiCmd::set_menu_actions_for(const std::string& instance_name, const Q
         mp::cli::platform::open_multipass_shell(QString::fromStdString(instance_name));
     });
 
-    if (state == "RUNNING")
+    if (state.status() == InstanceStatus::RUNNING)
     {
-        instance_menu->addAction("Suspend");
-        QObject::connect(instance_menu->actions().back(), &QAction::triggered, [this, instance_name] {
-            future_synchronizer.addFuture(QtConcurrent::run(this, &GuiCmd::suspend_instance_for, instance_name));
-        });
-
         instance_menu->addAction("Stop");
         QObject::connect(instance_menu->actions().back(), &QAction::triggered, [this, instance_name] {
             future_synchronizer.addFuture(QtConcurrent::run(this, &GuiCmd::stop_instance_for, instance_name));

src/client/gui/gui_cmd.h

@@ -72,15 +72,14 @@ class GuiCmd final : public QObject, public Command
     void initiate_menu_layout();
     void initiate_about_menu_layout();
     ListReply retrieve_all_instances();
-    void set_menu_actions_for(const std::string& instance_name, const QString& state);
+    void set_menu_actions_for(const std::string& instance_name, const InstanceStatus& state);
     void suspend_instance_for(const std::string& instance_name);
     void stop_instance_for(const std::string& instance_name);
     VersionReply retrieve_version_and_update_info();
 
     QSystemTrayIcon tray_icon;
     QMenu tray_icon_menu;
 
-    QAction* retrieving_action;
     QAction* about_separator;
     QAction* quit_action;
     QAction update_action{"Update available"};
@@ -93,7 +92,7 @@ class GuiCmd final : public QObject, public Command
 
     struct InstanceEntry
     {
-        QString state;
+        InstanceStatus state;
         std::unique_ptr<QMenu> menu;
     };
     std::unordered_map<std::string, InstanceEntry> instances_entries;

src/client/gui/main.cpp

@@ -29,7 +29,7 @@ int main(int argc, char* argv[])
     app.setApplicationName("multipass-gui");
 
     mp::ClientConfig config{mp::client::get_server_address(), mp::RpcConnectionType::ssl,
-                            std::move(mp::client::get_cert_provider())};
+                            mp::client::get_cert_provider()};
     mp::ClientGui client{config};
 
     return client.run(app.arguments());