Incorporate reviews

include/components/controller.hpp

@@ -75,7 +75,7 @@ class controller
  private:
   size_t setup_modules(alignment align);
 
-  bool try_forward_legacy_action(const string cmd);
+  bool try_forward_legacy_action(const string& cmd);
 
   connection& m_connection;
   signal_emitter& m_sig;

include/modules/alsa.hpp

@@ -36,7 +36,7 @@ namespace modules {
     static constexpr auto EVENT_TOGGLE = "toggle";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr auto FORMAT_VOLUME = "format-volume";

include/modules/backlight.hpp

@@ -32,7 +32,7 @@ namespace modules {
     static constexpr const char* EVENT_DEC = "dec";
 
    protected:
-    bool input(string&& cmd, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr auto TAG_LABEL = "<label>";

include/modules/bspwm.hpp

@@ -54,7 +54,7 @@ namespace modules {
     static constexpr auto EVENT_PREV = "prev";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     bool handle_status(string& data);

include/modules/date.hpp

@@ -21,7 +21,7 @@ namespace modules {
     static constexpr auto EVENT_TOGGLE = "toggle";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr auto TAG_LABEL = "<label>";

include/modules/i3.hpp

@@ -58,7 +58,7 @@ namespace modules {
     static constexpr auto EVENT_PREV = "prev";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static string make_workspace_command(const string& workspace);

include/modules/menu.hpp

@@ -29,7 +29,7 @@ namespace modules {
     static constexpr auto EVENT_EXEC = "exec";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr auto TAG_LABEL_TOGGLE = "<label-toggle>";

include/modules/meta/base.hpp

@@ -137,7 +137,7 @@ namespace modules {
     void teardown();
     string contents();
 
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
     string input_handler_name() const;
 
     static constexpr auto TYPE = "";

include/modules/meta/base.inl

@@ -104,7 +104,7 @@ namespace modules {
   }
 
   template <typename Impl>
-  bool module<Impl>::input(string&&, string&&) {
+  bool module<Impl>::input(const string&, const string&) {
     // By default a module doesn't support inputs
     return false;
   }

include/modules/meta/input_handler.hpp

@@ -16,7 +16,7 @@ namespace modules {
      *
      * \returns true if the action is supported and false otherwise
      */
-    virtual bool input(string&& action, string&& data) = 0;
+    virtual bool input(const string& action, const string& data) = 0;
 
     /**
      * The name of this input handler

include/modules/mpd.hpp

@@ -38,7 +38,7 @@ namespace modules {
     static constexpr const char* EVENT_SEEK = "seek";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr const char* FORMAT_ONLINE{"format-online"};

include/modules/pulseaudio.hpp

@@ -29,7 +29,7 @@ namespace modules {
     static constexpr auto EVENT_TOGGLE = "toggle";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr auto FORMAT_VOLUME = "format-volume";

include/modules/systray.hpp

@@ -24,7 +24,7 @@ namespace modules {
     static constexpr auto EVENT_TOGGLE = "toggle";
 
    protected:
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
 

include/modules/unsupported.hpp

@@ -36,7 +36,7 @@ namespace modules {
     string input_handler_name() const {                                                 \
       return "";                                                                        \
     }                                                                                   \
-    bool input(string&&, string&&) {                                                    \
+    bool input(const string&, const string&) {                                          \
       return false;                                                                     \
     }                                                                                   \
   }

include/modules/xbacklight.hpp

@@ -37,7 +37,7 @@ namespace modules {
 
    protected:
     void handle(const evt::randr_notify& evt);
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr const char* TAG_LABEL{"<label>"};

include/modules/xkeyboard.hpp

@@ -38,7 +38,7 @@ namespace modules {
     void handle(const evt::xkb_state_notify& evt);
     void handle(const evt::xkb_indicator_state_notify& evt);
 
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static constexpr const char* TAG_LABEL_LAYOUT{"<label-layout>"};

include/modules/xworkspaces.hpp

@@ -72,7 +72,7 @@ namespace modules {
     void rebuild_desktop_states();
     void set_desktop_urgent(xcb_window_t window);
 
-    bool input(string&& action, string&& data);
+    bool input(const string& action, const string& data);
 
    private:
     static vector<string> get_desktop_names();

src/components/controller.cpp

@@ -397,7 +397,7 @@ void controller::process_eventqueue() {
  * \returns true iff the given command matches a legacy action string and was
  *          successfully forwarded to a module
  */
-bool controller::try_forward_legacy_action(const string cmd) {
+bool controller::try_forward_legacy_action(const string& cmd) {
   /*
    * Maps legacy action names to a module type and the new action name in that module.
    *
@@ -412,7 +412,7 @@ bool controller::try_forward_legacy_action(const string cmd) {
 // clang-format off
 #define A_MAP(old, module_name, event) {old, {string(module_name::TYPE), string(module_name::event)}}
 
-  const std::map<string, std::pair<string, const string>> legacy_actions{
+  static const std::map<string, std::pair<string, const string>> legacy_actions{
     A_MAP("datetoggle", date_module, EVENT_TOGGLE),
 #if ENABLE_ALSA
     A_MAP("volup", alsa_module, EVENT_INC),
@@ -487,7 +487,7 @@ bool controller::try_forward_legacy_action(const string cmd) {
           }
           m_log.info(
               "Forwarding legacy action '%s' to module '%s' as '%s' with data '%s'", cmd, handler_name, action, data);
-          if (!handler_ptr->input(std::forward<string>(action), std::forward<string>(data))) {
+          if (!handler_ptr->input(action, data)) {
             m_log.err("Failed to forward deprecated action to %s module", type);
             // Forward to shell if the module cannot accept the action to not break existing behavior.
             return false;
@@ -514,8 +514,8 @@ void controller::process_inputdata() {
     return;
   }
 
-  const string cmd = m_inputdata;
-  m_inputdata.clear();
+  const string cmd = std::move(m_inputdata);
+  m_inputdata = string{};
 
   m_log.trace("controller: Processing inputdata: %s", cmd);
 
@@ -543,7 +543,7 @@ void controller::process_inputdata() {
     // Forwards the action to all input handlers that match the name
     for (auto&& handler : m_inputhandlers) {
       if (handler->input_handler_name() == handler_name) {
-        if (!handler->input(std::forward<string>(action), std::forward<string>(data))) {
+        if (!handler->input(action, data)) {
           m_log.err("The '%s' module does not support the '%s' action.", handler_name, action);
         }
 

src/modules/alsa.cpp

@@ -218,7 +218,7 @@ namespace modules {
     return true;
   }
 
-  bool alsa_module::input(string&& action , string&&) {
+  bool alsa_module::input(const string& action, const string&) {
     if (!m_handle_events) {
       return false;
     } else if (!m_mixer[mixer::MASTER]) {

src/modules/backlight.cpp

@@ -113,12 +113,12 @@ namespace modules {
     return true;
   }
 
-  bool backlight_module::input(string&& cmd, string&&) {
+  bool backlight_module::input(const string& action, const string&) {
     double value_mod{0.0};
 
-    if (cmd == EVENT_INC) {
+    if (action == EVENT_INC) {
       value_mod = 5.0;
-    } else if (cmd == EVENT_DEC) {
+    } else if (action == EVENT_DEC) {
       value_mod = -5.0;
     } else {
       return false;

src/modules/bspwm.cpp

@@ -445,7 +445,7 @@ namespace modules {
     return false;
   }
 
-  bool bspwm_module::input(string&& action, string&& data) {
+  bool bspwm_module::input(const string& action, const string& data) {
     auto send_command = [this](string payload_cmd, string log_info) {
       try {
         auto ipc = bspwm_util::make_connection();

src/modules/date.cpp

@@ -83,7 +83,7 @@ namespace modules {
     return true;
   }
 
-  bool date_module::input(string&& action, string&&) {
+  bool date_module::input(const string& action, const string&) {
     if (action != EVENT_TOGGLE) {
       return false;
     }

src/modules/i3.cpp

@@ -218,7 +218,7 @@ namespace modules {
     return true;
   }
 
-  bool i3_module::input(string&& action, string&& data) {
+  bool i3_module::input(const string& action, const string& data) {
     try {
       const i3_util::connection_t conn{};
 

src/modules/menu.cpp

@@ -99,7 +99,7 @@ namespace modules {
     return true;
   }
 
-  bool menu_module::input(string&& action, string&& data) {
+  bool menu_module::input(const string& action, const string& data) {
     if (action == EVENT_EXEC) {
       auto sep = data.find("-");
 

src/modules/mpd.cpp

@@ -353,7 +353,7 @@ namespace modules {
     return true;
   }
 
-  bool mpd_module::input(string&& action, string&& data) {
+  bool mpd_module::input(const string& action, const string& data) {
     m_log.info("%s: event: %s", name(), action);
 
     try {

src/modules/pulseaudio.cpp

@@ -142,7 +142,7 @@ namespace modules {
     return true;
   }
 
-  bool pulseaudio_module::input(string&& action, string&&) {
+  bool pulseaudio_module::input(const string& action, const string&) {
     if (!m_handle_events) {
       return false;
     }

src/modules/systray.cpp

@@ -53,7 +53,7 @@ namespace modules {
   /**
    * Handle input event
    */
-  bool systray_module::input(string&& action, string&&) {
+  bool systray_module::input(const string& action, const string&) {
     if (action.find(EVENT_TOGGLE) != 0) {
       return false;
     }

src/modules/xbacklight.cpp

@@ -147,7 +147,7 @@ namespace modules {
   /**
    * Process scroll events by changing backlight value
    */
-  bool xbacklight_module::input(string&& action, string&&) {
+  bool xbacklight_module::input(const string& action, const string&) {
     double value_mod{0.0};
 
     if (action == EVENT_INC) {

src/modules/xkeyboard.cpp

@@ -207,7 +207,7 @@ namespace modules {
   /**
    * Handle input command
    */
-  bool xkeyboard_module::input(string&& action, string&&) {
+  bool xkeyboard_module::input(const string& action, const string&) {
     if (action != EVENT_SWITCH) {
       return false;
     }

src/modules/xworkspaces.cpp

@@ -368,7 +368,7 @@ namespace modules {
   /**
    * Handle user input event
    */
-  bool xworkspaces_module::input(string&& action, string&& data) {
+  bool xworkspaces_module::input(const string& action, const string& data) {
     std::lock_guard<std::mutex> lock(m_workspace_mutex);
 
     vector<unsigned int> indexes;