Improve consistency of WML name validation functions

* moved the new function next to valid_id()
* renamed to valid_tag(), and renamed valid_id() to valid_attribute()
* removed unnecessary parentheses I forgot in
* changed the name of valid_id()'s parameter from "id" to "name"
* changed WML parser to use valid_tag() when it validates tag names

Thanks to @CelticMinstrel who told me about config::valid_id().

src/config.cpp

@@ -178,13 +178,30 @@ config& config::operator=(config&& cfg)
 	return *this;
 }
 
-bool config::valid_id(config_key_type id)
+bool config::valid_tag(config_key_type name)
 {
-	if(id.empty()) {
+	if(name == "") {
+		// Empty strings not allowed
+		return false;
+	} else if(name[0] == '_') {
+		// Underscore can't be the first character
+		return false;
+	} else {
+		return std::all_of(name.begin(), name.end(), [](const char& c)
+		{
+			// Only alphanumeric ASCII characters and underscores are allowed
+			return std::isalnum(c, std::locale::classic()) || c == '_';
+		});
+	}
+}
+
+bool config::valid_attribute(config_key_type name)
+{
+	if(name.empty()) {
 		return false;
 	}
 
-	for(char c : id) {
+	for(char c : name) {
 		if((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_') {
 			// valid character.
 		} else {
@@ -1305,28 +1322,11 @@ void swap(config& lhs, config& rhs)
 	lhs.swap(rhs);
 }
 
-bool config::is_valid_wml_tag_name(config_key_type name)
-{
-	if(name == "") {
-		// Empty strings not allowed
-		return false;
-	} else if(name[0] == '_') {
-		// Underscore can't be the first character
-		return false;
-	} else {
-		return std::all_of(name.begin(), name.end(), [](const char& c)
-		{
-			// Only alphanumeric ASCII characters and underscores are allowed
-			return std::isalnum((c), std::locale::classic()) || c == '_';
-		});
-	}
-}
-
 bool config::validate_wml() const
 {
 	return std::all_of(children_.begin(), children_.end(), [](const child_map::value_type& pair)
 	{
-		return is_valid_wml_tag_name(pair.first) &&
+		return valid_tag(pair.first) &&
 			std::all_of(pair.second.begin(), pair.second.end(),
 			[](const std::unique_ptr<config>& c) { return c->validate_wml(); });
 	});

src/config.hpp

@@ -132,8 +132,11 @@ public:
 
 	~config();
 
-	// Verifies that the string can be used as an attribute or tag name
-	static bool valid_id(config_key_type id);
+	// Verifies that the string can be used as a tag name
+	static bool valid_tag(config_key_type name);
+
+	// Verifies that the string can be used as an attribute name
+	static bool valid_attribute(config_key_type name);
 
 	explicit operator bool() const
 	{ return this != &invalid; }
@@ -761,11 +764,6 @@ public:
 	//this is a cheap O(1) operation
 	void swap(config& cfg);
 
-	/**
-	 * Returns true for valid WML tag names, false for all other strings.
-	 */
-	static bool is_valid_wml_tag_name(config_key_type name);
-
 	/**
 	 * Returns true if this object represents valid WML,
 	 * i.e. can be saved to disk and again loaded by the WML parser.

src/scripting/lua_common.cpp

@@ -735,7 +735,7 @@ bool luaW_toconfig(lua_State *L, int index, config &cfg)
 		if (!lua_istable(L, -1)) return_misformed();
 		lua_rawgeti(L, -1, 1);
 		char const *m = lua_tostring(L, -1);
-		if (!m || !config::is_valid_wml_tag_name(m)) return_misformed();
+		if (!m || !config::valid_tag(m)) return_misformed();
 		lua_rawgeti(L, -2, 2);
 		if (!luaW_toconfig(L, -1, cfg.add_child(m)))
 			return_misformed();

src/serialization/parser.cpp

@@ -702,7 +702,7 @@ static void write_internal(config const& cfg, std::ostream& out, std::string& te
 	}
 
 	for(const config::attribute& i : cfg.attribute_range()) {
-		if(!config::valid_id(i.first)) {
+		if(!config::valid_attribute(i.first)) {
 			ERR_CF << "Config contains invalid attribute name '" << i.first << "', skipping...\n";
 			continue;
 		}
@@ -711,7 +711,7 @@ static void write_internal(config const& cfg, std::ostream& out, std::string& te
 	}
 
 	for(const config::any_child& item : cfg.all_children_range()) {
-		if(!config::valid_id(item.key)) {
+		if(!config::valid_tag(item.key)) {
 			ERR_CF << "Config contains invalid tag name '" << item.key << "', skipping...\n";
 			continue;
 		}
@@ -735,7 +735,7 @@ static void write_internal(configr_of const& cfg, std::ostream& out, std::string
 	for(const auto& pair : cfg.subtags_) {
 		assert(pair.first && pair.second);
 
-		if(!config::valid_id(*pair.first)) {
+		if(!config::valid_tag(*pair.first)) {
 			ERR_CF << "Config contains invalid tag name '" << *pair.first << "', skipping...\n";
 			continue;
 		}

src/variable_info_private.hpp

@@ -128,7 +128,7 @@ public:
 	get_variable_key_visitor(const std::string& key)
 		: key_(key)
 	{
-		if(!config::valid_id(key_)) {
+		if(!config::valid_tag(key_)) {
 			throw invalid_variablename_exception();
 		}
 	}