From b422d1d1a4f95fa4ec37d01f6a49949c37216f17 Mon Sep 17 00:00:00 2001
From: Michael Carlberg <c@rlberg.se>
Date: Fri, 23 Dec 2016 20:43:52 +0100
Subject: [PATCH] refactor(modules): Input handling

---
 include/components/controller.hpp      |  8 ++---
 include/events/signal.hpp              |  5 ++-
 include/events/signal_fwd.hpp          |  1 -
 include/modules/bspwm.hpp              |  2 +-
 include/modules/date.hpp               |  2 +-
 include/modules/i3.hpp                 |  2 +-
 include/modules/menu.hpp               |  2 +-
 include/modules/meta/input_handler.hpp |  6 ++--
 include/modules/mpd.hpp                |  2 +-
 include/modules/volume.hpp             |  2 +-
 include/modules/xbacklight.hpp         |  2 +-
 include/modules/xkeyboard.hpp          |  2 +-
 include/modules/xworkspaces.hpp        |  2 +-
 src/components/controller.cpp          | 45 ++++++++++----------------
 src/modules/bspwm.cpp                  |  4 +--
 src/modules/date.cpp                   |  5 +--
 src/modules/i3.cpp                     | 11 +++----
 src/modules/menu.cpp                   |  4 +--
 src/modules/mpd.cpp                    |  4 +--
 src/modules/volume.cpp                 |  4 +--
 src/modules/xbacklight.cpp             |  4 +--
 src/modules/xkeyboard.cpp              |  4 +--
 src/modules/xworkspaces.cpp            | 25 ++++++--------
 23 files changed, 55 insertions(+), 93 deletions(-)

diff --git a/include/components/controller.hpp b/include/components/controller.hpp
index 94c67888..d3e63cd0 100644
--- a/include/components/controller.hpp
+++ b/include/components/controller.hpp
@@ -26,6 +26,7 @@ class signal_emitter;
 
 namespace modules {
   struct module_interface;
+  class input_handler;
 }
 
 using module_t = unique_ptr<modules::module_interface>;
@@ -42,7 +43,7 @@ namespace sig_ui = signals::ui;
 namespace sig_ipc = signals::ipc;
 
 class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, sig_ev::process_broadcast, sig_ev::process_update,
-                       sig_ev::process_input, sig_ev::process_quit, sig_ev::process_check, sig_ipc::process_action,
+                       sig_ev::process_quit, sig_ev::process_check, sig_ipc::process_action,
                        sig_ipc::process_command, sig_ipc::process_hook, sig_ui::button_press> {
  public:
   using make_type = unique_ptr<controller>;
@@ -55,7 +56,7 @@ class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, sig_ev::proc
   bool run(bool writeback = false);
 
   bool enqueue(event&& evt);
-  bool enqueue(string&& input_data);
+  bool enqueue(string&& input);
 
  protected:
   void read_events();
@@ -64,7 +65,6 @@ class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, sig_ev::proc
 
   bool on(const sig_ev::process_broadcast& evt);
   bool on(const sig_ev::process_update& evt);
-  bool on(const sig_ev::process_input& evt);
   bool on(const sig_ev::process_quit& evt);
   bool on(const sig_ev::process_check& evt);
   bool on(const sig_ui::button_press& evt);
@@ -106,7 +106,7 @@ class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, sig_ev::proc
   /**
    * @brief Module input handlers
    */
-  vector<signal_receiver_interface*> m_inputhandlers;
+  vector<modules::input_handler*> m_inputhandlers;
 
   /**
    * @brief Maximum number of subsequent events to swallow
diff --git a/include/events/signal.hpp b/include/events/signal.hpp
index 3f3e758f..abe25af0 100644
--- a/include/events/signal.hpp
+++ b/include/events/signal.hpp
@@ -94,9 +94,8 @@ namespace signals {
   namespace eventqueue {
     DEFINE_VALUE_SIGNAL(1, process_quit, event);
     DEFINE_VALUE_SIGNAL(2, process_update, event);
-    DEFINE_VALUE_SIGNAL(3, process_input, string);
-    DEFINE_SIGNAL(4, process_check);
-    DEFINE_SIGNAL(5, process_broadcast);
+    DEFINE_SIGNAL(3, process_check);
+    DEFINE_SIGNAL(4, process_broadcast);
   }
 
   namespace ipc {
diff --git a/include/events/signal_fwd.hpp b/include/events/signal_fwd.hpp
index c438501b..98f71b63 100644
--- a/include/events/signal_fwd.hpp
+++ b/include/events/signal_fwd.hpp
@@ -8,7 +8,6 @@ namespace signals {
   namespace eventqueue {
     struct process_quit;
     struct process_update;
-    struct process_input;
     struct process_check;
     struct process_broadcast;
   }
diff --git a/include/modules/bspwm.hpp b/include/modules/bspwm.hpp
index b0177b9b..80710524 100644
--- a/include/modules/bspwm.hpp
+++ b/include/modules/bspwm.hpp
@@ -47,7 +47,7 @@ namespace modules {
     bool build(builder* builder, const string& tag) const;
 
    protected:
-    bool on(const input_event_t& evt);
+    bool input(string&& cmd);
 
    private:
     static constexpr auto DEFAULT_ICON = "ws-icon-default";
diff --git a/include/modules/date.hpp b/include/modules/date.hpp
index 1dcf62a0..f20c076f 100644
--- a/include/modules/date.hpp
+++ b/include/modules/date.hpp
@@ -14,7 +14,7 @@ namespace modules {
     bool build(builder* builder, const string& tag) const;
 
    protected:
-    bool on(const input_event_t& evt);
+    bool input(string&& cmd);
 
    private:
     static constexpr auto TAG_LABEL = "<label>";
diff --git a/include/modules/i3.hpp b/include/modules/i3.hpp
index f0b5a2a6..bfb296e6 100644
--- a/include/modules/i3.hpp
+++ b/include/modules/i3.hpp
@@ -42,7 +42,7 @@ namespace modules {
     bool build(builder* builder, const string& tag) const;
 
    protected:
-    bool on(const input_event_t& evt);
+    bool input(string&& cmd);
 
    private:
     static constexpr const char* DEFAULT_TAGS{"<label-state> <label-mode>"};
diff --git a/include/modules/menu.hpp b/include/modules/menu.hpp
index ad7cc6d5..7f62a3a6 100644
--- a/include/modules/menu.hpp
+++ b/include/modules/menu.hpp
@@ -24,7 +24,7 @@ namespace modules {
     void update() {}
 
    protected:
-    bool on(const input_event_t& evt);
+    bool input(string&& cmd);
 
    private:
     static constexpr auto TAG_LABEL_TOGGLE = "<label-toggle>";
diff --git a/include/modules/meta/input_handler.hpp b/include/modules/meta/input_handler.hpp
index 86af6918..3ce847f3 100644
--- a/include/modules/meta/input_handler.hpp
+++ b/include/modules/meta/input_handler.hpp
@@ -1,16 +1,14 @@
 #pragma once
 
 #include "common.hpp"
-#include "events/signal.hpp"
-#include "events/signal_receiver.hpp"
 
 POLYBAR_NS
 
 namespace modules {
-  using input_event_t = signals::eventqueue::process_input;
-  class input_handler : public signal_receiver<0, input_event_t> {
+  class input_handler {
    public:
     virtual ~input_handler() {}
+    virtual bool input(string&& cmd) = 0;
   };
 }
 
diff --git a/include/modules/mpd.hpp b/include/modules/mpd.hpp
index 487f2e90..6fe93338 100644
--- a/include/modules/mpd.hpp
+++ b/include/modules/mpd.hpp
@@ -27,7 +27,7 @@ namespace modules {
     bool build(builder* builder, const string& tag) const;
 
    protected:
-    bool on(const input_event_t& evt);
+    bool input(string&& cmd);
 
    private:
     static constexpr const char* FORMAT_ONLINE{"format-online"};
diff --git a/include/modules/volume.hpp b/include/modules/volume.hpp
index fb4196f7..21dc7cdd 100644
--- a/include/modules/volume.hpp
+++ b/include/modules/volume.hpp
@@ -31,7 +31,7 @@ namespace modules {
     bool build(builder* builder, const string& tag) const;
 
    protected:
-    bool on(const input_event_t& evt);
+    bool input(string&& cmd);
 
    private:
     static constexpr auto FORMAT_VOLUME = "format-volume";
diff --git a/include/modules/xbacklight.hpp b/include/modules/xbacklight.hpp
index 8ee7766a..11db3e94 100644
--- a/include/modules/xbacklight.hpp
+++ b/include/modules/xbacklight.hpp
@@ -37,7 +37,7 @@ namespace modules {
 
    protected:
     void handle(const evt::randr_notify& evt);
-    bool on(const input_event_t& evt);
+    bool input(string&& cmd);
 
    private:
     static constexpr const char* TAG_LABEL{"<label>"};
diff --git a/include/modules/xkeyboard.hpp b/include/modules/xkeyboard.hpp
index 6ef343dc..1bd772ef 100644
--- a/include/modules/xkeyboard.hpp
+++ b/include/modules/xkeyboard.hpp
@@ -36,7 +36,7 @@ namespace modules {
     void handle(const evt::xkb_state_notify& evt);
     void handle(const evt::xkb_indicator_state_notify& evt);
 
-    bool on(const input_event_t&);
+    bool input(string&& cmd);
 
    private:
     static constexpr const char* TAG_LABEL_LAYOUT{"<label-layout>"};
diff --git a/include/modules/xworkspaces.hpp b/include/modules/xworkspaces.hpp
index e7df5fa2..607ed444 100644
--- a/include/modules/xworkspaces.hpp
+++ b/include/modules/xworkspaces.hpp
@@ -63,7 +63,7 @@ namespace modules {
     void handle(const evt::property_notify& evt);
     void rebuild_desktops();
     void set_current_desktop();
-    bool on(const input_event_t&);
+    bool input(string&& cmd);
 
    private:
     static constexpr const char* DEFAULT_ICON{"icon-default"};
diff --git a/src/components/controller.cpp b/src/components/controller.cpp
index ce73e608..fea2c20c 100644
--- a/src/components/controller.cpp
+++ b/src/components/controller.cpp
@@ -363,17 +363,31 @@ void controller::process_eventqueue() {
  */
 void controller::process_inputdata() {
   if (!m_inputdata.empty()) {
-    auto evt = sig_ev::process_input{string{m_inputdata}};
+    string cmd = m_inputdata;
     m_lastinput = chrono::time_point_cast<decltype(m_swallow_input)>(chrono::system_clock::now());
     m_inputdata.clear();
 
     for (auto&& handler : m_inputhandlers) {
-      if (handler->on(evt)) {
+      if (handler->input(string{cmd})) {
         return;
       }
     }
 
-    m_sig.emit(evt);
+    try {
+      m_log.warn("Uncaught input event, forwarding to shell... (input: %s)", cmd);
+
+      if (m_command) {
+        m_log.warn("Terminating previous shell command");
+        m_command->terminate();
+      }
+
+      m_log.info("Executing shell command: %s", cmd);
+      m_command = command_util::make_command(move(cmd));
+      m_command->exec();
+      m_command.reset();
+    } catch (const application_error& err) {
+      m_log.err("controller: Error while forwarding input to shell -> %s", err.what());
+    }
   }
 }
 
@@ -472,31 +486,6 @@ bool controller::on(const sig_ev::process_update& evt) {
   return true;
 }
 
-/**
- * Process eventqueue input event
- */
-bool controller::on(const sig_ev::process_input& evt) {
-  try {
-    string input{*evt()};
-
-    m_log.warn("Uncaught input event, forwarding to shell... (input: %s)", input);
-
-    if (m_command) {
-      m_log.warn("Terminating previous shell command");
-      m_command->terminate();
-    }
-
-    m_log.info("Executing shell command: %s", input);
-    m_command = command_util::make_command(move(input));
-    m_command->exec();
-    m_command.reset();
-  } catch (const application_error& err) {
-    m_log.err("controller: Error while forwarding input to shell -> %s", err.what());
-  }
-
-  return true;
-}
-
 /**
  * Process eventqueue quit event
  */
diff --git a/src/modules/bspwm.cpp b/src/modules/bspwm.cpp
index 688d6755..53aced5d 100644
--- a/src/modules/bspwm.cpp
+++ b/src/modules/bspwm.cpp
@@ -417,9 +417,7 @@ namespace modules {
     return false;
   }
 
-  bool bspwm_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool bspwm_module::input(string&& cmd) {
     if (cmd.find(EVENT_PREFIX) != 0) {
       return false;
     }
diff --git a/src/modules/date.cpp b/src/modules/date.cpp
index ea4401f2..a4382590 100644
--- a/src/modules/date.cpp
+++ b/src/modules/date.cpp
@@ -84,13 +84,10 @@ namespace modules {
     return true;
   }
 
-  bool date_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool date_module::input(string&& cmd) {
     if (cmd != EVENT_TOGGLE) {
       return false;
     }
-
     m_toggled = !m_toggled;
     wakeup();
     return true;
diff --git a/src/modules/i3.cpp b/src/modules/i3.cpp
index 90a943c3..e45c6322 100644
--- a/src/modules/i3.cpp
+++ b/src/modules/i3.cpp
@@ -191,9 +191,7 @@ namespace modules {
     return true;
   }
 
-  bool i3_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool i3_module::input(string&& cmd) {
     if (cmd.find(EVENT_PREFIX) != 0) {
       return false;
     }
@@ -205,11 +203,10 @@ namespace modules {
       const connection_t conn{};
 
       if (cmd.compare(0, strlen(EVENT_CLICK), EVENT_CLICK) == 0) {
-        const string workspace_num{cmd.substr(strlen(EVENT_CLICK))};
-
-        if (focused_workspace(conn)->num != atoi(workspace_num.c_str())) {
+        cmd.erase(0, strlen(EVENT_CLICK));
+        if (focused_workspace(conn)->num != atoi(cmd.c_str())) {
           m_log.info("%s: Sending workspace focus command to ipc handler", name());
-          conn.send_command("workspace number " + workspace_num);
+          conn.send_command("workspace number " + cmd);
         }
       } else if (cmd.compare(0, strlen(EVENT_SCROLL_UP), EVENT_SCROLL_UP) == 0) {
         scrolldir = m_revscroll ? "prev" : "next";
diff --git a/src/modules/menu.cpp b/src/modules/menu.cpp
index d1228c64..c8284dd5 100644
--- a/src/modules/menu.cpp
+++ b/src/modules/menu.cpp
@@ -80,9 +80,7 @@ namespace modules {
     return true;
   }
 
-  bool menu_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool menu_module::input(string&& cmd) {
     if (cmd.compare(0, 4, "menu") != 0) {
       return false;
     }
diff --git a/src/modules/mpd.cpp b/src/modules/mpd.cpp
index 1a3724be..cd59d191 100644
--- a/src/modules/mpd.cpp
+++ b/src/modules/mpd.cpp
@@ -316,9 +316,7 @@ namespace modules {
     return true;
   }
 
-  bool mpd_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool mpd_module::input(string&& cmd) {
     if (cmd.compare(0, 3, "mpd") != 0) {
       return false;
     }
diff --git a/src/modules/volume.cpp b/src/modules/volume.cpp
index 15884e35..a83aca68 100644
--- a/src/modules/volume.cpp
+++ b/src/modules/volume.cpp
@@ -209,9 +209,7 @@ namespace modules {
     return true;
   }
 
-  bool volume_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool volume_module::input(string&& cmd) {
     if (cmd.compare(0, 3, EVENT_PREFIX) != 0) {
       return false;
     }
diff --git a/src/modules/xbacklight.cpp b/src/modules/xbacklight.cpp
index cec84c72..1f2b926e 100644
--- a/src/modules/xbacklight.cpp
+++ b/src/modules/xbacklight.cpp
@@ -150,9 +150,7 @@ namespace modules {
   /**
    * Process scroll events by changing backlight value
    */
-  bool xbacklight_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool xbacklight_module::input(string&& cmd) {
     int value_mod = 0;
 
     if (cmd == EVENT_SCROLLUP) {
diff --git a/src/modules/xkeyboard.cpp b/src/modules/xkeyboard.cpp
index 1a02a348..e709f130 100644
--- a/src/modules/xkeyboard.cpp
+++ b/src/modules/xkeyboard.cpp
@@ -104,9 +104,7 @@ namespace modules {
   /**
    * Handle input command
    */
-  bool xkeyboard_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-
+  bool xkeyboard_module::input(string&& cmd) {
     if (cmd.compare(0, strlen(EVENT_SWITCH), EVENT_SWITCH) != 0) {
       return false;
     }
diff --git a/src/modules/xworkspaces.cpp b/src/modules/xworkspaces.cpp
index f0ffcc65..7ae70f39 100644
--- a/src/modules/xworkspaces.cpp
+++ b/src/modules/xworkspaces.cpp
@@ -208,13 +208,12 @@ namespace modules {
     return false;
   }
 
-  bool xworkspaces_module::on(const input_event_t& evt) {
-    string cmd{*evt.data()};
-    if (cmd.find(EVENT_PREFIX) != 0) {
+  bool xworkspaces_module::input(string&& cmd) {
+    size_t len{strlen(EVENT_PREFIX)};
+    if (cmd.compare(0, len, EVENT_PREFIX) != 0) {
       return false;
     }
-
-    cmd.erase(0, strlen(EVENT_PREFIX));
+    cmd.erase(0, len);
 
     uint32_t new_desktop{0};
     uint32_t min_desktop{0};
@@ -233,26 +232,22 @@ namespace modules {
       }
     }
 
-    if (cmd.compare(0, strlen(EVENT_CLICK), EVENT_CLICK) == 0) {
-      new_desktop = atoi(cmd.substr(strlen(EVENT_CLICK)).c_str());
-
-    } else if (cmd.compare(0, strlen(EVENT_SCROLL_UP), EVENT_SCROLL_UP) == 0) {
+    if ((len = strlen(EVENT_CLICK)) && cmd.compare(0, len, EVENT_CLICK) == 0) {
+      new_desktop = std::strtoul(cmd.substr(len).c_str(), nullptr, 10);
+    } else if ((len = strlen(EVENT_SCROLL_UP)) && cmd.compare(0, len, EVENT_SCROLL_UP) == 0) {
       new_desktop = math_util::min<uint32_t>(max_desktop, current_desktop + 1);
-
       if (new_desktop == current_desktop) {
         new_desktop = min_desktop;
       }
-
-    } else if (cmd.compare(0, strlen(EVENT_SCROLL_DOWN), EVENT_SCROLL_DOWN) == 0) {
-      new_desktop = math_util::max<uint32_t>(min_desktop, current_desktop - 1);
-
+    } else if ((len = strlen(EVENT_SCROLL_DOWN)) && cmd.compare(0, len, EVENT_SCROLL_DOWN) == 0) {
+      new_desktop = math_util::max<uint32_t>(min_desktop, std::max(0U, current_desktop - 1));
       if (new_desktop == current_desktop) {
         new_desktop = max_desktop;
       }
     }
 
     if (new_desktop != current_desktop) {
-      m_log.info("%s: Requesting change to desktop #%lu", name(), new_desktop);
+      m_log.info("%s: Requesting change to desktop #%u", name(), new_desktop);
       ewmh_util::change_current_desktop(m_ewmh.get(), new_desktop);
       m_connection.flush();
     } else {