actions: Move parsing to utility file

include/utils/actions.hpp

@@ -7,6 +7,19 @@ POLYBAR_NS
 
 namespace actions_util {
   string get_action_string(const modules::input_handler& handler, string action, string data);
+
+  /**
+   * Parses an action string of the form "#name.action[.data]".
+   *
+   * Only call this function with an action string that begins with '#'.
+   *
+   * \returns a triple (name, action, data)
+   *          If no data exists, the third string will be empty.
+   *          This means "#name.action." and "#name.action" will be produce the
+   *          same result.
+   * \throws runtime_error If the action string is malformed
+   */
+  std::tuple<string, string, string> parse_action_string(string action);
 }  // namespace actions_util
 
 POLYBAR_NS_END

src/components/controller.cpp

@@ -13,6 +13,7 @@
 #include "events/signal_emitter.hpp"
 #include "modules/meta/event_handler.hpp"
 #include "modules/meta/factory.hpp"
+#include "utils/actions.hpp"
 #include "utils/command.hpp"
 #include "utils/factory.hpp"
 #include "utils/inotify.hpp"
@@ -409,46 +410,36 @@ void controller::process_inputdata() {
    * module.
    */
   if (cmd.front() == '#') {
-    // Find the second delimiter '.'
-    auto end_pos = cmd.find('.', 1);
+    try {
+      auto res = actions_util::parse_action_string(cmd);
 
-    if (end_pos == string::npos) {
-      m_log.err("Invalid action string (input: %s)", cmd);
-      return;
-    }
+      auto handler_name = std::get<0>(res);
+      auto action = std::get<1>(res);
+      auto data = std::get<2>(res);
 
-    auto handler_name = cmd.substr(1, end_pos - 1);
-    auto action = cmd.substr(end_pos + 1);
-    string data;
+      m_log.info(
+          "Forwarding data to input handlers (name: '%s', action: '%s', data: '%s') ", handler_name, action, data);
 
-    // Find the '.' character that separates the data from the action name
-    auto data_sep_pos = action.find('.', 0);
+      int num_delivered = 0;
 
-    // The action contains data
-    if (data_sep_pos != string::npos) {
-      data = action.substr(data_sep_pos + 1);
-      action.erase(data_sep_pos);
-    }
+      // 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))) {
+            m_log.err("The '%s' module does not support the '%s' action.", handler_name, action);
+          }
 
-    m_log.info("Forwarding data to input handlers (name: '%s', action: '%s', data: '%s') ", handler_name, action, data);
-
-    int num_delivered = 0;
-
-    // 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))) {
-          m_log.err("The '%s' module does not support the '%s' action.", handler_name, action);
+          num_delivered++;
         }
-
-        num_delivered++;
       }
-    }
 
-    if (num_delivered == 0) {
-      m_log.err("There exists no input handler with name '%s' (input: %s)", handler_name, cmd);
-    } else {
-      m_log.info("Delivered input to %d input handler%s", num_delivered, num_delivered > 1 ? "s" : "");
+      if (num_delivered == 0) {
+        m_log.err("There exists no input handler with name '%s' (input: %s)", handler_name, cmd);
+      } else {
+        m_log.info("Delivered input to %d input handler%s", num_delivered, num_delivered > 1 ? "s" : "");
+      }
+    } catch (runtime_error& e) {
+      m_log.err("Invalid action string (reason: %s)", e.what());
     }
 
     return;

src/utils/actions.cpp

@@ -1,5 +1,8 @@
 #include "utils/actions.hpp"
 
+#include <cassert>
+#include <stdexcept>
+
 #include "common.hpp"
 
 POLYBAR_NS
@@ -13,6 +16,39 @@ namespace actions_util {
 
     return str;
   }
+
+  std::tuple<string, string, string> parse_action_string(string action_str) {
+    assert(action_str.front() == '#');
+
+    action_str.erase(0, 1);
+
+    auto action_sep = action_str.find('.');
+
+    if (action_sep == string::npos) {
+      throw std::runtime_error("Missing separator between name and action");
+    }
+
+    auto handler_name = action_str.substr(0, action_sep);
+
+    if (handler_name.empty()) {
+      throw std::runtime_error("The handler name must not be empty");
+    }
+
+    auto action = action_str.substr(action_sep + 1);
+    auto data_sep = action.find('.');
+    string data;
+
+    if (data_sep != string::npos) {
+      data = action.substr(data_sep + 1);
+      action.erase(data_sep);
+    }
+
+    if (action.empty()) {
+      throw std::runtime_error("The action name must not be empty");
+    }
+
+    return std::tuple<string, string, string>{handler_name, action, data};
+  }
 }  // namespace actions_util
 
 POLYBAR_NS_END

tests/CMakeLists.txt

@@ -47,6 +47,7 @@ function(add_unit_test source_file)
   add_dependencies(all_unit_tests ${name})
 endfunction()
 
+add_unit_test(utils/actions)
 add_unit_test(utils/color)
 add_unit_test(utils/command)
 add_unit_test(utils/math unit_tests)

tests/unit_tests/utils/actions.cpp

@@ -0,0 +1,51 @@
+#include "utils/actions.hpp"
+
+#include "common/test.hpp"
+
+using namespace polybar;
+using namespace actions_util;
+
+template<typename T1, typename T2, typename T3>
+using triple = std::tuple<T1, T2, T3>;
+
+class ParseActionStringTest : public ::testing::TestWithParam<pair<string, triple<string, string, string>>> {};
+
+vector<pair<string, triple<string, string, string>>> parse_action_string_list = {
+  {"#foo.bar", {"foo", "bar", ""}},
+  {"#foo.bar.", {"foo", "bar", ""}},
+  {"#foo.bar.data", {"foo", "bar", "data"}},
+  {"#foo.bar.data.data2", {"foo", "bar", "data.data2"}},
+  {"#a.b.c", {"a", "b", "c"}},
+  {"#a.b.", {"a", "b", ""}},
+  {"#a.b", {"a", "b", ""}},
+};
+
+TEST_P(ParseActionStringTest, correctness) {
+  auto action_string = GetParam().first;
+  auto exp = GetParam().second;
+
+  auto res = parse_action_string(action_string);
+
+  EXPECT_EQ(res, exp);
+}
+
+INSTANTIATE_TEST_SUITE_P(Inst, ParseActionStringTest, ::testing::ValuesIn(parse_action_string_list));
+
+class ParseActionStringThrowTest : public ::testing::TestWithParam<string> {};
+
+vector<string> parse_action_string_throw_list = {
+  "#",
+  "#.",
+  "#..",
+  "#handler..",
+  "#.action.",
+  "#.action.data",
+  "#..data",
+  "#.data",
+};
+
+INSTANTIATE_TEST_SUITE_P(Inst, ParseActionStringThrowTest, ::testing::ValuesIn(parse_action_string_throw_list));
+
+TEST_P(ParseActionStringThrowTest, correctness) {
+  EXPECT_THROW(parse_action_string(GetParam()), std::runtime_error);
+}