From 507004df87a97e759ed89668ffc4b035c68f1aad Mon Sep 17 00:00:00 2001 From: patrick96 Date: Sun, 31 May 2020 16:28:31 +0200 Subject: [PATCH] actions: Move parsing to utility file --- include/utils/actions.hpp | 13 +++++++ src/components/controller.cpp | 55 +++++++++++++----------------- src/utils/actions.cpp | 36 +++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/unit_tests/utils/actions.cpp | 51 +++++++++++++++++++++++++++ 5 files changed, 124 insertions(+), 32 deletions(-) create mode 100644 tests/unit_tests/utils/actions.cpp diff --git a/include/utils/actions.hpp b/include/utils/actions.hpp index 5fb6e482..17e6d5b7 100644 --- a/include/utils/actions.hpp +++ b/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 parse_action_string(string action); } // namespace actions_util POLYBAR_NS_END diff --git a/src/components/controller.cpp b/src/components/controller.cpp index 9322b1c9..de529825 100644 --- a/src/components/controller.cpp +++ b/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(action), std::forward(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(action), std::forward(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; diff --git a/src/utils/actions.cpp b/src/utils/actions.cpp index b0d4ecb7..a4ae567b 100644 --- a/src/utils/actions.cpp +++ b/src/utils/actions.cpp @@ -1,5 +1,8 @@ #include "utils/actions.hpp" +#include +#include + #include "common.hpp" POLYBAR_NS @@ -13,6 +16,39 @@ namespace actions_util { return str; } + + std::tuple 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{handler_name, action, data}; + } } // namespace actions_util POLYBAR_NS_END diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index dfad6588..2186c71d 100644 --- a/tests/CMakeLists.txt +++ b/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) diff --git a/tests/unit_tests/utils/actions.cpp b/tests/unit_tests/utils/actions.cpp new file mode 100644 index 00000000..d93fc36f --- /dev/null +++ b/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 +using triple = std::tuple; + +class ParseActionStringTest : public ::testing::TestWithParam>> {}; + +vector>> 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 {}; + +vector 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); +}