From 56cb940a12a71c3cae0e1e91f056fbc1eb994faf Mon Sep 17 00:00:00 2001 From: Andrzej Rybczak Date: Wed, 7 Dec 2016 19:02:47 +0100 Subject: [PATCH] actions: use shared_ptr to store actions --- src/actions.cpp | 20 ++++++++++++-------- src/actions.h | 4 +++- src/bindings.cpp | 33 ++++++++++++++++++++------------- src/bindings.h | 9 +++++---- src/help.cpp | 2 +- src/macro_utilities.cpp | 6 +++--- src/macro_utilities.h | 6 +++--- src/ncmpcpp.cpp | 2 -- 8 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/actions.cpp b/src/actions.cpp index f68b83df..c3e088f9 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -76,7 +76,7 @@ namespace ph = std::placeholders; namespace { -std::vector> AvailableActions; +std::vector> AvailableActions; void populateActions(); @@ -281,22 +281,26 @@ BaseAction &get(Actions::Type at) { if (AvailableActions.empty()) populateActions(); - BaseAction *action = AvailableActions.at(static_cast(at)).get(); - // action should be always present if action type in queried - assert(action != nullptr); - return *action; + return *AvailableActions.at(static_cast(at)); } -BaseAction *get(const std::string &name) +std::shared_ptr get_(Actions::Type at) { - BaseAction *result = nullptr; + if (AvailableActions.empty()) + populateActions(); + return AvailableActions.at(static_cast(at)); +} + +std::shared_ptr get_(const std::string &name) +{ + std::shared_ptr result; if (AvailableActions.empty()) populateActions(); for (const auto &action : AvailableActions) { if (action->name() == name) { - result = action.get(); + result = action; break; } } diff --git a/src/actions.h b/src/actions.h index bde3cefb..300e8ee4 100644 --- a/src/actions.h +++ b/src/actions.h @@ -216,7 +216,9 @@ private: }; BaseAction &get(Type at); -BaseAction *get(const std::string &name); + +std::shared_ptr get_(Type at); +std::shared_ptr get_(const std::string &name); struct Dummy: BaseAction { diff --git a/src/bindings.cpp b/src/bindings.cpp index b1765fa1..a0d1f1cd 100644 --- a/src/bindings.cpp +++ b/src/bindings.cpp @@ -134,13 +134,13 @@ NC::Key::Type stringToKey(const std::string &s) } template -Actions::BaseAction *parseActionLine(const std::string &line, F error) +std::shared_ptr parseActionLine(const std::string &line, F error) { - Actions::BaseAction *result = 0; + std::shared_ptr result; size_t i = 0; for (; i < line.size() && !isspace(line[i]); ++i) { } if (i == line.size()) // only action name - result = Actions::get(line); + result = Actions::get_(line); else // there is something else { std::string action_name = line.substr(0, i); @@ -149,9 +149,11 @@ Actions::BaseAction *parseActionLine(const std::string &line, F error) // push single character into input queue std::string arg = getEnclosedString(line, '"', '"', 0); NC::Key::Type k = stringToSpecialKey(arg); - auto queue = std::vector{ k }; if (k != NC::Key::None) - result = new Actions::PushCharacters(&Global::wFooter, std::move(queue)); + result = std::static_pointer_cast( + std::make_shared( + &Global::wFooter, + std::vector{k})); else error() << "invalid character passed to push_character: '" << arg << "'\n"; } @@ -161,11 +163,13 @@ Actions::BaseAction *parseActionLine(const std::string &line, F error) std::string arg = getEnclosedString(line, '"', '"', 0); if (!arg.empty()) { - std::vector queue(arg.begin(), arg.end()); // if char is signed, erase 1s from char -> int conversion for (auto it = arg.begin(); it != arg.end(); ++it) *it &= 0xff; - result = new Actions::PushCharacters(&Global::wFooter, std::move(queue)); + result = std::static_pointer_cast( + std::make_shared( + &Global::wFooter, + std::vector{arg.begin(), arg.end()})); } else error() << "empty argument passed to push_characters\n"; @@ -176,7 +180,8 @@ Actions::BaseAction *parseActionLine(const std::string &line, F error) std::string arg = getEnclosedString(line, '"', '"', 0); ScreenType screen_type = stringToScreenType(arg); if (screen_type != ScreenType::Unknown) - result = new Actions::RequireScreen(screen_type); + result = std::static_pointer_cast( + std::make_shared(screen_type)); else error() << "unknown screen passed to require_screen: '" << arg << "'\n"; } @@ -184,9 +189,10 @@ Actions::BaseAction *parseActionLine(const std::string &line, F error) { // require that given action is runnable std::string arg = getEnclosedString(line, '"', '"', 0); - auto action = Actions::get(arg); + auto action = Actions::get_(arg); if (action) - result = new Actions::RequireRunnable(action); + result = std::static_pointer_cast( + std::make_shared(action)); else error() << "unknown action passed to require_runnable: '" << arg << "'\n"; } @@ -194,7 +200,8 @@ Actions::BaseAction *parseActionLine(const std::string &line, F error) { std::string command = getEnclosedString(line, '"', '"', 0); if (!command.empty()) - result = new Actions::RunExternalCommand(std::move(command)); + result = std::static_pointer_cast( + std::make_shared(std::move(command))); else error() << "empty command passed to run_external_command\n"; } @@ -472,11 +479,11 @@ void BindingsConfiguration::generateDefaults() if (notBound(k = stringToKey("up"))) bind(k, Actions::Type::ScrollUp); if (notBound(k = stringToKey("shift-up"))) - bind(k, Binding::ActionChain({ &Actions::get(Actions::Type::SelectItem), &Actions::get(Actions::Type::ScrollUp) })); + bind(k, Binding::ActionChain({Actions::get_(Actions::Type::SelectItem), Actions::get_(Actions::Type::ScrollUp)})); if (notBound(k = stringToKey("down"))) bind(k, Actions::Type::ScrollDown); if (notBound(k = stringToKey("shift-down"))) - bind(k, Binding::ActionChain({ &Actions::get(Actions::Type::SelectItem), &Actions::get(Actions::Type::ScrollDown) })); + bind(k, Binding::ActionChain({Actions::get_(Actions::Type::SelectItem), Actions::get_(Actions::Type::ScrollDown)})); if (notBound(k = stringToKey("["))) bind(k, Actions::Type::ScrollUpAlbum); if (notBound(k = stringToKey("]"))) diff --git a/src/bindings.h b/src/bindings.h index 3badd9ca..987311e1 100644 --- a/src/bindings.h +++ b/src/bindings.h @@ -33,7 +33,7 @@ std::wstring keyToWString(const NC::Key::Type key); /// Represents either single action or chain of actions bound to a certain key struct Binding { - typedef std::vector ActionChain; + typedef std::vector> ActionChain; template Binding(ArgT &&actions_) @@ -41,7 +41,7 @@ struct Binding assert(!m_actions.empty()); } Binding(Actions::Type at) - : Binding(ActionChain({&Actions::get(at)})) { } + : Binding(ActionChain({Actions::get_(at)})) { } bool execute() const { return std::all_of(m_actions.begin(), m_actions.end(), @@ -53,9 +53,10 @@ struct Binding return m_actions.size() == 1; } - Actions::BaseAction *action() const { + Actions::BaseAction &action() const { assert(isSingle()); - return m_actions[0]; + assert(m_actions[0] != nullptr); + return *m_actions[0]; } const ActionChain &actions() const { diff --git a/src/help.cpp b/src/help.cpp index e7d9fafd..51e0a6f2 100644 --- a/src/help.cpp +++ b/src/help.cpp @@ -60,7 +60,7 @@ std::string display_keys(const Actions::Type at) { for (auto j = it->second.begin(); j != it->second.end(); ++j) { - if (j->isSingle() && j->action()->type() == at) + if (j->isSingle() && j->action().type() == at) { skey = keyToWString(it->first); if (!skey.empty()) diff --git a/src/macro_utilities.cpp b/src/macro_utilities.cpp index bf89fafd..5cc63720 100644 --- a/src/macro_utilities.cpp +++ b/src/macro_utilities.cpp @@ -46,9 +46,9 @@ void PushCharacters::run() (*m_window)->pushChar(*it); } -RequireRunnable::RequireRunnable(BaseAction *action) +RequireRunnable::RequireRunnable(std::shared_ptr action) : BaseAction(Type::MacroUtility, "require_runnable") - , m_action(action) + , m_action(std::move(action)) { assert(m_action != nullptr); m_name += " \""; @@ -75,7 +75,7 @@ bool RequireScreen::canBeRun() return Global::myScreen->type() == m_screen_type; } -RunExternalCommand::RunExternalCommand(std::string command) +RunExternalCommand::RunExternalCommand(std::string &&command) : BaseAction(Type::MacroUtility, "run_external_command") , m_command(std::move(command)) { diff --git a/src/macro_utilities.h b/src/macro_utilities.h index 7b1082bb..5ff0742b 100644 --- a/src/macro_utilities.h +++ b/src/macro_utilities.h @@ -40,13 +40,13 @@ private: struct RequireRunnable: BaseAction { - RequireRunnable(BaseAction *action); + RequireRunnable(std::shared_ptr action); private: virtual bool canBeRun() override; virtual void run() override { } - BaseAction *m_action; + std::shared_ptr m_action; }; struct RequireScreen: BaseAction @@ -62,7 +62,7 @@ private: struct RunExternalCommand: BaseAction { - RunExternalCommand(std::string command); + RunExternalCommand(std::string &&command); private: virtual void run() override; diff --git a/src/ncmpcpp.cpp b/src/ncmpcpp.cpp index d98b91f1..5acea3af 100644 --- a/src/ncmpcpp.cpp +++ b/src/ncmpcpp.cpp @@ -207,8 +207,6 @@ int main(int argc, char **argv) if (!key_pressed) continue; - //Statusbar::print(ToString(keyToWString(input))); - // The reason we want to update timer here is that if the timer is updated // in Status::trace, then Key::read usually blocks for 500ms and if key is // pressed 400ms after Key::read was called, we end up with Timer that is