From 41ddaf9e231e0bfb57c72841badf4ad0116cf420 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 2 Mar 2010 19:29:51 +0100 Subject: [PATCH] refactor settings classes result: easier declaration of settings, easier construction of custom setting classes, commitcount++ --- src/setting.cc | 4 +- src/setting.hh | 283 +++++++++++++++++++++++++------------------------ src/x11.cc | 33 +++--- src/x11.h | 20 ++-- 4 files changed, 171 insertions(+), 169 deletions(-) diff --git a/src/setting.cc b/src/setting.cc index 55f4f896..4c003bf2 100644 --- a/src/setting.cc +++ b/src/setting.cc @@ -89,7 +89,7 @@ namespace conky { return; } - iter->second->call_lua_setter(&l, init); + iter->second->lua_setter(l, init); l.pushvalue(-2); l.insert(-2); l.rawset(-4); @@ -162,5 +162,5 @@ namespace conky { } /////////// example settings, remove after real settings are available /////// - config_setting> asdf("asdf", range_checking_accessors(42, 47, 45, true)); + range_config_setting asdf("asdf", 42, 47, 45, true); } diff --git a/src/setting.hh b/src/setting.hh index 39d5df93..16dca545 100644 --- a/src/setting.hh +++ b/src/setting.hh @@ -52,8 +52,8 @@ namespace conky { struct lua_traits { static const lua::Type type = lua::TNUMBER; - static std::pair convert(lua::state *l, int index, const std::string &) - { return {l->tointeger(index), true}; } + static std::pair convert(lua::state &l, int index, const std::string &) + { return {l.tointeger(index), true}; } }; // specialization for floating point types @@ -61,8 +61,8 @@ namespace conky { struct lua_traits { static const lua::Type type = lua::TNUMBER; - static std::pair convert(lua::state *l, int index, const std::string &) - { return {l->tonumber(index), true}; } + static std::pair convert(lua::state &l, int index, const std::string &) + { return {l.tonumber(index), true}; } }; // specialization for std::string @@ -70,8 +70,8 @@ namespace conky { struct lua_traits { static const lua::Type type = lua::TSTRING; - static std::pair convert(lua::state *l, int index, const std::string &) - { return {l->tostring(index), true}; } + static std::pair convert(lua::state &l, int index, const std::string &) + { return {l.tostring(index), true}; } }; // specialization for bool @@ -79,8 +79,8 @@ namespace conky { struct lua_traits { static const lua::Type type = lua::TBOOLEAN; - static std::pair convert(lua::state *l, int index, const std::string &) - { return {l->toboolean(index), true}; } + static std::pair convert(lua::state &l, int index, const std::string &) + { return {l.toboolean(index), true}; } }; // specialization for enums @@ -92,9 +92,9 @@ namespace conky { typedef std::initializer_list> Map; static Map map; - static std::pair convert(lua::state *l, int index, const std::string &name) + static std::pair convert(lua::state &l, int index, const std::string &name) { - std::string val = l->tostring(index); + std::string val = l.tostring(index); for(auto i = map.begin(); i != map.end(); ++i) { if(i->first == val) @@ -115,117 +115,30 @@ namespace conky { } }; - - // standard getters and setters for basic types. They try to do The Right Thing(tm) (accept - // only values of correct type and print an error message otherwise). For something more - // elaborate, one can always write a new accessor class - template> - class simple_accessors { - const T default_value; - bool modifiable; - - protected: - std::pair do_convert(lua::state *l, int index, const std::string &name) - { - if(l->isnil(index)) - return {default_value, true}; - - if(l->type(index) != Traits::type) { - NORM_ERR("Invalid value of type '%s' for setting '%s'. " - "Expected value of type '%s'.", l->type_name(l->type(index)), - name.c_str(), l->type_name(Traits::type) ); - return {default_value, false}; - } - - return Traits::convert(l, index, name); - } - - std::pair setter_check(lua::state *l, bool init, const std::string &name) - { - if(!init && !modifiable) { - NORM_ERR("Setting '%s' is not modifiable", name.c_str()); - return {default_value, false}; - } else - return do_convert(l, -2, name); - } - - public: - simple_accessors(T default_value_ = T(), bool modifiable_ = false) - : default_value(default_value_), modifiable(modifiable_) - {} - - T getter(lua::state *l, const std::string &name) - { - lua::stack_sentry s(*l, -1); - auto ret = do_convert(l, -1, name); - l->pop(); - - // setter function should make sure the value is valid - assert(ret.second); - - return ret.first; - } - - void lua_setter(lua::state *l, bool init, const std::string &name) - { - lua::stack_sentry s(*l, -2); - - auto ret = setter_check(l, init, name); - if(ret.second) - l->pop(); - else - l->replace(-2); - ++s; - } - }; - - template> - class range_checking_accessors: private simple_accessors { - typedef simple_accessors Base; - - T min; - T max; - public: - range_checking_accessors(T min_ = std::numeric_limits::min(), - T max_ = std::numeric_limits::max(), - T default_value_ = T(), bool modifiable_ = false) - : Base(default_value_, modifiable_), min(min_), max(max_) - { assert(min_ <= default_value_ && default_value_ <= max_); } - - using Base::getter; - - void lua_setter(lua::state *l, bool init, const std::string &name) - { - lua::stack_sentry s(*l, -2); - - auto ret = Base::setter_check(l, init, name); - if(ret.second) { - if(ret.first < min || ret.first > max) { - NORM_ERR("Value is out of range for setting '%s'", name.c_str()); - // we ignore out-of-range values. an alternative would be to clamp them. do - // we want to do that? - l->replace(-2); - } else - l->pop(); - } else - l->replace(-2); - ++s; - } - }; - namespace priv { class config_setting_base { private: static void process_setting(lua::state &l, bool init); static int config__newindex(lua::state *l); + // copying is a REALLY bad idea + config_setting_base(const config_setting_base &) = delete; + config_setting_base& operator=(const config_setting_base &) = delete; + protected: - virtual void call_lua_setter(lua::state *l, bool init) = 0; + /* + * Set the setting, if the value is sane + * stack on entry: | ... potential_new_value old_value | + * stack on exit: | ... real_new_value | + * real_new_value can be the old value if the new value doesn't make sense + */ + virtual void lua_setter(lua::state &l, bool init) = 0; public: const std::string name; - config_setting_base(const std::string &name_); + explicit config_setting_base(const std::string &name_); + virtual ~config_setting_base() {} /* * Set the setting manually. @@ -242,39 +155,28 @@ namespace conky { extern config_settings_t *config_settings; } - /* - * Declares a setting in the conky.config table. - * Getter function is used to translate the lua value into C++. It recieves the value on the - * lua stack. It should pop it and return the C++ value. In case the value is nil, it should - * return a predefined default value. Translation into basic types is provided with the - * default simple_getter::do_it functions. - * The lua_setter function is called when someone tries to set the value. - * It recieves the new and the old value on the stack (old one is on top). It should return - * the new value for the setting. It doesn't have to be the value the user set, if e.g. the - * value doesn't make sense. The second parameter is true if the assignment occurs during the - * initial parsing of the config file, and false afterwards. Some settings obviously cannot - * be changed (easily?) when conky is running, but some (e.g. x/y position of the window) - * can. - */ - template> - class config_setting: public priv::config_setting_base { + // If you need some very exotic setting, derive it from this class. Otherwise, scroll down. + template + class config_setting_template: public priv::config_setting_base { public: - config_setting(const std::string &name_, const Accessors &accessors_ = Accessors()) - : config_setting_base(name_), accessors(accessors_) + explicit config_setting_template(const std::string &name_) + : config_setting_base(name_) {} + // get the value of the setting as a C++ type T get(lua::state &l); protected: - virtual void call_lua_setter(lua::state *l, bool init) - { accessors.lua_setter(l, init, name); } - - private: - Accessors accessors; + /* + * Convert the value into a C++ type. + * stack on entry: | ... value | + * stack on exit: | ... | + */ + virtual T getter(lua::state &l) = 0; }; - template - T config_setting::get(lua::state &l) + template + T config_setting_template::get(lua::state &l) { lua::stack_sentry s(l); l.checkstack(2); @@ -286,11 +188,118 @@ namespace conky { l.getfield(-1, name.c_str()); l.replace(-2); - return accessors.getter(&l, name); + return getter(l); } + /* + * Declares a setting in the conky.config table. + * Getter function is used to translate the lua value into C++. It recieves the value on the + * lua stack. It should pop it and return the C++ value. In case the value is nil, it should + * return a predefined default value. Translation into basic types works with the help of + * lua_traits template above + * The lua_setter function is called when someone tries to set the value. It recieves the + * new and the old value on the stack (old one is on top). It should return the new value for + * the setting. It doesn't have to be the value the user set, if e.g. the value doesn't make + * sense. The second parameter is true if the assignment occurs during the initial parsing of + * the config file, and false afterwards. Some settings obviously cannot be changed (easily?) + * when conky is running, but some (e.g. x/y position of the window) can. + */ + template> + class simple_config_setting: public config_setting_template { + typedef config_setting_template Base; + + public: + simple_config_setting(const std::string &name_, const T &default_value_ = T(), + bool modifiable_ = false) + : Base(name_), default_value(default_value_), modifiable(modifiable_) + {} + + protected: + const T default_value; + const bool modifiable; + + virtual std::pair do_convert(lua::state &l, int index); + virtual void lua_setter(lua::state &l, bool init); + + virtual T getter(lua::state &l) + { + lua::stack_sentry s(l, -1); + auto ret = do_convert(l, -1); + l.pop(); + + // setter function should make sure the value is valid + assert(ret.second); + + return ret.first; + } + }; + + template + std::pair simple_config_setting::do_convert(lua::state &l, int index) + { + if(l.isnil(index)) + return {default_value, true}; + + if(l.type(index) != Traits::type) { + NORM_ERR("Invalid value of type '%s' for setting '%s'. " + "Expected value of type '%s'.", l.type_name(l.type(index)), + Base::name.c_str(), l.type_name(Traits::type) ); + return {default_value, false}; + } + + return Traits::convert(l, index, Base::name); + } + + template + void simple_config_setting::lua_setter(lua::state &l, bool init) + { + lua::stack_sentry s(l, -2); + + bool ok = true; + if(!init && !modifiable) { + NORM_ERR("Setting '%s' is not modifiable", Base::name.c_str()); + ok = false; + } + + if(ok && do_convert(l, -2).second) + l.pop(); + else + l.replace(-2); + ++s; + } + + // Just like simple_config_setting, except that in only accepts value in the [min, max] range + template> + class range_config_setting: public simple_config_setting { + typedef simple_config_setting Base; + + const T min; + const T max; + public: + range_config_setting(const std::string &name_, + const T &min_ = std::numeric_limits::min(), + const T &max_ = std::numeric_limits::max(), + const T &default_value_ = T(), + bool modifiable_ = false) + : Base(name_, default_value_, modifiable_), min(min_), max(max_) + { assert(min <= Base::default_value && Base::default_value <= max); } + + protected: + virtual std::pair do_convert(lua::state &l, int index) + { + auto ret = Base::do_convert(l, index); + if(ret.second && (ret.first < min || ret.first > max)) { + NORM_ERR("Value is out of range for setting '%s'", Base::name.c_str()); + // we ignore out-of-range values. an alternative would be to clamp them. do we + // want to do that? + ret.second = false; + } + return ret; + } + }; + /////////// example settings, remove after real settings are available /////// - extern config_setting> asdf; + extern range_config_setting asdf; } #endif /* SETTING_HH */ diff --git a/src/x11.cc b/src/x11.cc index 6eb059a8..35d19b3c 100644 --- a/src/x11.cc +++ b/src/x11.cc @@ -935,25 +935,23 @@ conky::lua_traits::Map conky::lua_traits::map = { { "middle_right", MIDDLE_RIGHT }, { "none", NONE } }; -conky::config_setting text_alignment("alignment", - conky::simple_accessors(NONE, false)); +conky::simple_config_setting text_alignment("alignment", NONE, false); -conky::config_setting out_to_x("out_to_x", conky::simple_accessors(false, false)); +conky::simple_config_setting out_to_x("out_to_x", false, false); #ifdef OWN_WINDOW -conky::config_setting own_window("own_window", conky::simple_accessors(false, false)); -conky::config_setting set_transparent("own_window_transparent", - conky::simple_accessors(false, false)); -conky::config_setting own_window_class("own_window_class", - conky::simple_accessors(PACKAGE_NAME, false)); +conky::simple_config_setting own_window("own_window", false, false); +conky::simple_config_setting set_transparent("own_window_transparent", false, false); +conky::simple_config_setting own_window_class("own_window_class", + PACKAGE_NAME, false); namespace { // used to set the default value for own_window_title std::string gethostnamecxx() { update_uname(); return info.uname_s.nodename; } } -conky::config_setting own_window_title("own_window_title", - conky::simple_accessors(PACKAGE_NAME " (" + gethostnamecxx()+")", false)); +conky::simple_config_setting own_window_title("own_window_title", + PACKAGE_NAME " (" + gethostnamecxx()+")", false); template<> conky::lua_traits::Map conky::lua_traits::map = { @@ -963,18 +961,13 @@ conky::lua_traits::Map conky::lua_traits::map = { { "desktop", TYPE_DESKTOP }, { "override", TYPE_OVERRIDE } }; -conky::config_setting own_window_type("own_window_type", - conky::simple_accessors(TYPE_NORMAL, false)); +conky::simple_config_setting own_window_type("own_window_type", TYPE_NORMAL, false); -conky::config_setting background_colour("background_colour", - conky::simple_accessors("black", false)); +conky::simple_config_setting background_colour("background_colour", "black", false); #ifdef BUILD_ARGB -conky::config_setting use_argb_visual("own_window_argb_visual", - conky::simple_accessors(false, false)); -conky::config_setting> - own_window_argb_value("own_window_argb_value", - conky::range_checking_accessors(0, 255, 255, false) - ); +conky::simple_config_setting use_argb_visual("own_window_argb_visual", false, false); +conky::range_config_setting own_window_argb_value("own_window_argb_value", + 0, 255, 255, false); #endif #endif /*OWN_WINDOW*/ diff --git a/src/x11.h b/src/x11.h index c3c83755..9de3a652 100644 --- a/src/x11.h +++ b/src/x11.h @@ -145,24 +145,24 @@ enum alignment { NONE }; -extern conky::config_setting text_alignment; -extern conky::config_setting out_to_x; +extern conky::simple_config_setting text_alignment; +extern conky::simple_config_setting out_to_x; #ifdef OWN_WINDOW -extern conky::config_setting own_window; -extern conky::config_setting set_transparent; -extern conky::config_setting own_window_class; -extern conky::config_setting own_window_title; -extern conky::config_setting own_window_type; +extern conky::simple_config_setting own_window; +extern conky::simple_config_setting set_transparent; +extern conky::simple_config_setting own_window_class; +extern conky::simple_config_setting own_window_title; +extern conky::simple_config_setting own_window_type; // this setting is not checked for validity when set, we leave that to the caller // the reason for that is that we need to have X initialised in order to call XParseColor() -extern conky::config_setting background_colour; +extern conky::simple_config_setting background_colour; #ifdef BUILD_ARGB -extern conky::config_setting use_argb_visual; +extern conky::simple_config_setting use_argb_visual; /* range of 0-255 for alpha */ -extern conky::config_setting> own_window_argb_value; +extern conky::range_config_setting own_window_argb_value; #endif #endif /*OWN_WINDOW*/