Change setting setter call order, add cleanup function

Call setting setters in the order in which they were registered.
Add a cleanup function, called in the reverse registration order.

src/conky.cc

@@ -3983,7 +3983,7 @@ int main(int argc, char **argv)
 				"    own_window_hints='above, undecorated,,below'};\n"
 				);
 		l.call(0, 0);
-		conky::check_config_settings(l);
+		conky::set_config_settings(l);
 		std::cout << "config.alignment = " << text_alignment.get(l) << std::endl;
 		l.pushstring("X");
 		text_alignment.lua_set(l);

src/data-source.cc

@@ -70,7 +70,7 @@ namespace conky {
 		int data_source_astext(lua::state *l)
 		{
 			std::string x = get_data_source(l).get_text();
-			l->pushstring(x.c_str());
+			l->pushstring(x);
 			return 1;
 		}
 

src/setting.cc

@@ -25,26 +25,82 @@
 
 #include "setting.hh"
 
+#include <algorithm>
+#include <memory>
+#include <vector>
+#include <unordered_map>
+
 namespace conky {
-	namespace priv {
+
+	namespace {
+		typedef std::unordered_map<std::string, priv::config_setting_base *> settings_map;
+		typedef std::vector<priv::config_setting_base *> settings_vector;
+
 		/*
 		 * We cannot construct this object statically, because order of object construction in
 		 * different modules is not defined, so config_setting_base could be called before this
 		 * object is constructed. Therefore, we create it on the first call to
 		 * config_setting_base constructor.
 		 */
-		config_settings_t *config_settings;
+		settings_map *settings;
 
-		config_setting_base::config_setting_base(const std::string &name_)
-			: name(name_)
+		/*
+		 * Returns the setting record corresponding to the value at the specified index. If the
+		 * value is not valid, returns NULL and prints an error.
+		 */
+		priv::config_setting_base* get_setting(lua::state &l, int index)
 		{
-			struct config_settings_constructor {
-				config_settings_constructor() { priv::config_settings = new config_settings_t; }
-				~config_settings_constructor() { delete config_settings; config_settings = NULL; }
+			lua::Type type = l.type(index);
+			if(type != lua::TSTRING) {
+				NORM_ERR("invalid setting of type '%s'", l.type_name(type));
+				return NULL;
+			}
+
+			const std::string &name = l.tostring(index);
+			auto iter = settings->find(name);
+			if(iter == settings->end()) {
+				NORM_ERR("Unknown setting '%s'", name.c_str());
+				return NULL;
+			}
+
+			return iter->second;
+		}
+
+		// returns a vector of all settings, sorted in order of registration
+		settings_vector make_settings_vector()
+		{
+			settings_vector ret;
+			ret.reserve(settings->size());
+
+			for(auto i = settings->begin(); i != settings->end(); ++i)
+				ret.push_back(i->second);
+			sort(ret.begin(), ret.end(), &priv::config_setting_base::seq_compare);
+
+			return ret;
+		}
+
+		/*
+		 * Returns the seq_no for the new setting object. Also constructs settings object
+		 * if needed.
+		 */
+		size_t get_next_seq_no()
+		{
+			struct settings_constructor {
+				settings_constructor() { settings = new settings_map; }
+				~settings_constructor() { delete settings; settings = NULL; }
 			};
-			static config_settings_constructor constructor;
+			static settings_constructor constructor;
+
+			return settings->size();
+		}
+	}
+
+	namespace priv {
 
-			bool inserted = config_settings->insert({name, this}).second;
+		config_setting_base::config_setting_base(const std::string &name_)
+			: name(name_), seq_no(get_next_seq_no())
+		{
+			bool inserted = settings->insert({name, this}).second;
 			if(not inserted)
 				throw std::logic_error("Setting with name '" + name + "' already registered");
 		}
@@ -59,37 +115,25 @@ namespace conky {
 			l.replace(-2);
 			l.insert(-2);
 
-			l.pushstring(name.c_str());
-			l.insert(-2);
-
-			l.settable(-3);
+			l.setfield(-2, name.c_str());
 			l.pop();
 		}
 
 		/*
 		 * Performs the actual assignment of settings. Calls the setting-specific setter after
 		 * some sanity-checking.
 		 * stack on entry: | ..., new_config_table, key, value, old_value |
-		 * stack on exit:  | ..., new_config_table, key |
+		 * stack on exit:  | ..., new_config_table |
 		 */
 		void config_setting_base::process_setting(lua::state &l, bool init)
 		{
-			lua::stack_sentry s(l, -2);
+			lua::stack_sentry s(l, -3);
 
-			lua::Type type = l.type(-3);
-			if(type != lua::TSTRING) {
-				NORM_ERR("invalid setting of type '%s'", l.type_name(type));
+			config_setting_base *ptr = get_setting(l, -3);
+			if(not ptr)
 				return;
-			}
 
-			std::string name = l.tostring(-3);
-			auto iter = priv::config_settings->find(name);
-			if(iter == priv::config_settings->end()) {
-				NORM_ERR("Unknown setting '%s'", name.c_str());
-				return;
-			}
-
-			iter->second->lua_setter(l, init);
+			ptr->lua_setter(l, init);
 			l.pushvalue(-2);
 			l.insert(-2);
 			l.rawset(-4);
@@ -114,51 +158,93 @@ namespace conky {
 
 			return 0;
 		}
+
+		/*
+		 * conky.config will not be a table, but a userdata with some metamethods we do this
+		 * because we want to control access to the settings we use the metatable for storing the
+		 * settings, that means having a setting whose name starts with "__" is a bad idea
+		 * stack on entry: | ... |
+		 * stack on exit:  | ... new_config_table |
+		 */
+		void config_setting_base::make_conky_config(lua::state &l)
+		{
+			lua::stack_sentry s(l);
+			l.checkstack(3);
+
+			l.newuserdata(1);
+
+			l.newtable(); {
+				l.pushboolean(false);
+				l.rawsetfield(-2, "__metatable");
+
+				l.pushvalue(-1);
+				l.rawsetfield(-2, "__index");
+
+				l.pushfunction(&priv::config_setting_base::config__newindex);
+				l.rawsetfield(-2, "__newindex");
+			} l.setmetatable(-2);
+
+			++s;
+		}
 	}
 
-	/*
-	 * Called after the initial loading of the config file. Performs the initial assignments.
-	 * at least one setting should always be registered, so config_settings will not be null
-	 * stack on entry: | ... |
-	 * stack on exit:  | ... |
-	 */
-	void check_config_settings(lua::state &l)
+	void set_config_settings(lua::state &l)
 	{
 		lua::stack_sentry s(l);
 		l.checkstack(6);
 
+		// Force creation of settings map. In the off chance we have no settings.
+		get_next_seq_no();
+
 		l.getglobal("conky"); {
+			if(l.type(-1) != lua::TTABLE)
+				throw std::runtime_error("conky must be a table");
+
 			l.rawgetfield(-1, "config"); {
 				if(l.type(-1) != lua::TTABLE)
 					throw std::runtime_error("conky.config must be a table");
 
-				// new conky.config table, containing only valid settings
-				l.newtable(); {
-					l.pushnil();
-					while(l.next(-3)) {
+				priv::config_setting_base::make_conky_config(l);
+				l.rawsetfield(-3, "config");
+
+				l.rawgetfield(-2, "config"); l.getmetatable(-1); l.replace(-2); {
+					const settings_vector &v = make_settings_vector();
+
+					for(size_t i = 0; i < v.size(); ++i) {
+						l.pushstring(v[i]->name);
+						l.rawgetfield(-3, v[i]->name.c_str());
 						l.pushnil();
 						priv::config_setting_base::process_setting(l, true);
 					}
-				} l.replace(-2);
+				} l.pop();
 
-				l.pushboolean(false);
-				l.rawsetfield(-2, "__metatable");
+				// print error messages for unknown settings
+				l.pushnil();
+				while(l.next(-2)) {
+					l.pop();
+					get_setting(l, -1);
+				}
 
-				l.pushvalue(-1);
-				l.rawsetfield(-2, "__index");
+			} l.pop();
+		} l.pop();
+	}
 
-				l.pushfunction(&priv::config_setting_base::config__newindex);
-				l.rawsetfield(-2, "__newindex");
+	void cleanup_config_settings(lua::state &l)
+	{
+		lua::stack_sentry s(l);
+		l.checkstack(2);
 
-				// conky.config will not be a table, but a userdata with some metamethods
-				// we do this because we want to control access to the settings
-				// we use the metatable for storing the settings, that means having a setting
-				// whose name stars with "__" is a bad idea
-				l.newuserdata(1);
-				l.insert(-2);
-				l.setmetatable(-2);
-			} l.rawsetfield(-2, "config");
-		} l.pop();
+		l.getglobal("conky");
+		l.rawgetfield(-1, "config");
+		l.replace(-2);
+
+		const settings_vector &v = make_settings_vector();
+		for(size_t i = v.size(); i > 0; --i) {
+			l.getfield(-1, v[i-1]->name.c_str());
+			v[i-1]->cleanup(l);
+		}
+
+		l.pop();
 	}
 
 /////////// example settings, remove after real settings are available ///////

src/setting.hh

@@ -27,14 +27,27 @@
 #include <limits>
 #include <string>
 #include <type_traits>
-#include <unordered_map>
 
 #include "logging.h"
 #include "luamm.hh"
 
 namespace conky {
 
-	void check_config_settings(lua::state &l);
+	/*
+	 * Checks settings, and does initial calls to the setters.
+	 * Should be called after reading the user config.
+	 * stack on entry: | ... |
+	 * stack on exit:  | ... |
+	 */
+	void set_config_settings(lua::state &l);
+
+	/*
+	 * Calls cleanup functions.
+	 * Should be called before exit/restart.
+	 * stack on entry: | ... |
+	 * stack on exit:  | ... |
+	 */
+	void cleanup_config_settings(lua::state &l);
 
 	template<typename T,
 		bool is_integral = std::is_integral<T>::value,
@@ -120,6 +133,7 @@ namespace conky {
 		private:
 			static void process_setting(lua::state &l, bool init);
 			static int config__newindex(lua::state *l);
+			static void make_conky_config(lua::state &l);
 
 			// copying is a REALLY bad idea
 			config_setting_base(const config_setting_base &) = delete;
@@ -134,8 +148,19 @@ namespace conky {
 			 */
 			virtual void lua_setter(lua::state &l, bool init) = 0;
 
+			/*
+			 * Called on exit/restart.
+			 * stack on entry: | ... new_value |
+			 * stack on exit:  | ... |
+			 */
+			virtual void cleanup(lua::state &l) { l.pop(); }
+
 		public:
 			const std::string name;
+			const size_t seq_no;
+
+			static bool seq_compare(const config_setting_base *a, const config_setting_base *b)
+			{ return a->seq_no < b->seq_no; }
 
 			explicit config_setting_base(const std::string &name_);
 			virtual ~config_setting_base() {}
@@ -147,12 +172,9 @@ namespace conky {
 			 */
 			void lua_set(lua::state &l);
 
-			friend void conky::check_config_settings(lua::state &l);
+			friend void conky::set_config_settings(lua::state &l);
+			friend void conky::cleanup_config_settings(lua::state &l);
 		};
-
-		typedef std::unordered_map<std::string, config_setting_base *> config_settings_t;
-
-		extern config_settings_t *config_settings;
 	}
 
 	// If you need some very exotic setting, derive it from this class. Otherwise, scroll down.