ok so it's still a little blocky, could prolly improve more, and i haven't compiled yet, no doubt many issues, but, i did my idea for a better config parser, and it went pretty well

srcs/ConfigParser.cpp

@@ -15,21 +15,11 @@
 
 /***** Stuf to rework
 
-I don't love the use of EMPTY and FAILURE and those
-	slowly transitioning away...
-
-I would rather throw exceptions everywhere...
-
-
-where do i check if there is only a ";"
-
 
 need to figure out why return std::vector<ServerConfig> * rather than just simple
 	not a pointer...
 	is there a good reason?
 
-I need to better understand what check_proper_line does
-
 
 */
 
@@ -103,11 +93,11 @@ ConfigParser &	ConfigParser::operator=(const ConfigParser& rhs)
 
 
 // ok what exactly is the benefit of returning a pointer here...
-std::vector<Server> *	ConfigParser::parse()
+//std::vector<Server> *	ConfigParser::parse()
+std::vector<Server>	ConfigParser::parse()
 {
 	// is this the best way to do this? new?
-	std::vector<ServerConfig> *ret = new std::vector<Server>();
+//	std::vector<ServerConfig> *ret = new std::vector<Server>();
-	// yea i could do
 	std::vector<ServerConfig>	ret;
 
 	size_t	prev = 0;
@@ -115,7 +105,7 @@ std::vector<Server> *	ConfigParser::parse()
 
 	if (curr == std::string::npos)
 		throw std::invalid_argument("empty config file");
-	while (cur != std::string::npos)
+	while (curr != std::string::npos)
 	{
 		// ok i don't love her way of scoping out the "server" strings...
 		prev = _content.find_first_not_of(" \t\n", curr);
@@ -126,44 +116,36 @@ std::vector<Server> *	ConfigParser::parse()
 //		Server	server = parse_server(&curr);
 //		ret->push_back(server);
 		// why not this?
-		ret->push_back(parse_server(&curr);
+		ret.push_back(parse_server(&curr);
 	}
 	return (ret);
-	// or
-//	return (&ret);	// wait no, that doesn't work...
 }
 
-// could we return a ref instead?
-	// i think no
 // might need new names for Prev and Curr, not super descriptive...
 ServerConfig	ConfigParser::parse_server(size_t *start)
 {
 	ServerConfig	ret;
-	size_t			key_start;
+//	size_t			key_start;
-	size_t			value_end;
+//	size_t			value_end;
 	size_t			prev = _content.find_first_not_of(" \t\n", *start);
 
+
 	if (prev == std::string::npos || _content[prev] != '{')
 		throw std::invalid_argument("bad config file syntax");
 
 	size_t	curr = _content.find_first_of(" \t\n", ++prev);
-	if (curr == std::string::npos)	// are there other things to check for?
+//	if (curr == std::string::npos)	// are there other things to check for?
-		throw std::invalid_argument("bad config file syntax");
+//		throw std::invalid_argument("bad config file syntax");
-	while (curr != std::string::npos)
+	while (curr != std::string::npos)	// here curr == { + 1
 	{
-		_get_key(&word_start, &blank_start);
-		_check_proper_line_end(&prev, &curr);	// nope, not an option i think...
-/*		if ((prev = _content.find_first_not_of(" \t\n", curr)) == std::string::npos)
-			throw std::invalid_argument("bad config file arguments");
-		if ((curr = _content.find_first_of(" \t\n", prev)) == std::string::npos)
-			throw std::invalid_argument("bad config file arguments");
-*/		key_start = prev;
 
-		std::string	key = _content.substr(prev, curr - prev);
+	// so this moves curr to past the word...
-		std::string key = _get_key(&word_start, &blank_start);
+		std::string key = _get_first_word(&curr);
+	// now curr is on space after 1st word.
 		switch (key)
 		{
 			case "}":
+				// why +1 curr is already after it no?
 				*start = _content.find_first_not_of(" \t\n" curr + 1);
 				break ;
 			case "location":
@@ -171,32 +153,11 @@ ServerConfig	ConfigParser::parse_server(size_t *start)
 					// the name but it's so clear...
 				ret.location.push_back(parse_location(&curr));
 			default:
+				std::string	values = _get_rest_of_line(&curr);
+			// curr now should be \n
+			// checking for ; in _set_value, check key and value
 
-	// ok i figured it out, there's a slight difference!
+				_set_server_values(&ret, key, values); // it handles the throws
-	
-	// curr below is this actually, she does go all the way to the end of the line...
-	// there has to be a better way of doing this, just like
-		// get me the first word, get me all the rest... but like in sub functions...
-	if ((curr = _content.find_first_of("\n", prev)) == std::string::npos)
-					throw std::invalid_argument("bad config file arguments");
-
-				_check_proper_line_end(&prev, &curr);
-		/*		if ((prev = _content.find_first_not_of(" \t\n", curr)) == std::string::npos)
-					throw std::invalid_argument("bad config file arguments");
-				if ((curr = _content.find_first_of(" \t\n", prev)) == std::string::npos)
-					throw std::invalid_argument("bad config file arguments");
-		*/
-				// why bother with the if.. why not throw exception in check_line...
-				if ((value_end = _check_for_semicolon(_content.substr(key_start, curr - key_start))) == FAILED)
-					throw std::invalid_argument("bad config file arguments");
-//				if ((int)value_end == EMPTY)
-//					continue ;
-			// could i shove this in the _set_server_value() func call?
-				std::string	value = _content.substr(prev, value_end - prev + key_start + 1)
-// since there's not return anymore
-//				if (_set_server_value(&ret, key, value) == FAILED)
-//					throw std::invalid_argument("bad config file arguments");
-				_set_server_value(&ret, key, value); // it handles the throws
 		}
 	}
 	return (ret);
@@ -215,181 +176,252 @@ LocationConfig	ConfigParser::parse_location(size_t *start)
 		throw std::invalid_argument("bad config file syntax");
 
 	size_t	curr = _content.find_first_of(" \t\n", ++prev);
+//	if (curr == std::string::npos)	// are there other things to check for?
+//		throw std::invalid_argument("bad config file syntax");
 	while (curr != std::string::npos)
 	{
-		_check_proper_line_end(&prev, &curr);
+	// so this moves curr to past the word...
-		key_start = prev;
+		std::string key = _get_first_word(&curr);
+	// now curr is on space after 1st word.
 
-		std::string	key = _content.substr(prev, curr - prev);
 		switch (key)
 		{
 			case "}":
 				*start = curr;
 				break ;
 			default:
-				_check_proper_line_end(&prev, &curr);
+				std::string	values = _get_rest_of_line(&curr);
-	// why bother with the if.. why not throw exception in check_line...
+			// curr now should be \n
-	// ok no we need the if cuz have to set value_end
+			// checking for ; in _set_value, check key and value
-				if ((value_end = _check_for_semicolon(_content.substr(key_start, curr - key_start))) == FAILED)
+
-					throw std::invalid_argument("bad config file arguments");
+				_set_location_values(&ret, key, values); // it handles the throws
-	// if we remove all empty lines at the start no need for this
-//				if ((int)value_end == EMPTY)
-//					continue ;
-			// shove this in _set_location_value() func call?
-				std::string	value = _content.substr(prev, value_end - prev + key_start + 1)
-//				if (_set_location_value(&ret, key, value) == FAILED)
-//					throw std::invalid_argument("bad config file arguments");
-				_set_location_value(&ret, key, value);
 		}
 	}
 	return (ret);
 }
 
-// yea ok new plan, this is where a lot of the checks will happen...
 
 
-void	ConfigParser::_set_server_value(Server *server, const std::string key, const std::string value)
+// ok you need to think through these throws, when will each occur?
+
+
+
+void	ConfigParser::_set_server_values(Server *server, const std::string key, const std::string value)
 {
-	// so turns out here is where i should be checking for the semicolon i think
-	// and also making sure there are the right number of values depending on which key it is...
-	// like for error page there can be a bunch but you can't have say 1 error code and several error files
-		// it's 1 error file per line and 
 
-	// i could do some checks at the start for value being bad or empty? or is
+	// check key for ;
-	// that unnecessary since in theory i've already checked that it's good...
+	// check values for ; at end and right number of words depending on key
-	switch (key)
+
+	if (key.find_first_of(";") != std::string::npos)
+		throw std::invalid_argument("bad config file arguments");
+
+	// there shouldn't be any tabs, right? not between values...
+	if (value.find_first_of("\t") != std::string::npos)
+		throw std::invalid_argument("bad config file arguments");
+		
+	size_t	i = value.find_first_of(";");
+	// so you can't have no ;
+	// you can't have just ;
+	// and you can't have a ; not at the end or several ;
+	// in theory value_find_last_of should find the only ;
+	if (i == std::string::npos || (value.find_last_not_of(" \n")) != i \
+		|| value.compare(";") == 0)
+		throw std::invalid_argument("bad config file arguments");
+
+
+	// we Trim value.
+	// is this valid?
+	value = value.substr(0, i - 1);
+
+	std::vector<std::string> tmp = ::split(value, ' ');
+
+	if (tmp.size() == 1)
 	{
-		case "server_name":
+		switch (key)
-			server->server_name = value;
+		{
-		case "listen":
+			case "server_name":
-			if (value.find_first_of(":") == std::string::npos)
+				server->server_name = value;
-			{
+			case "listen":
-				// why not store as vector<int> [4] ?
+			// ok yea i don't get this one anymore..
-				server->host = "0.0.0.0";
+				if (value.find_first_of(":") == std::string::npos)
-				server->value = value;
+				{
-			}
+					// why not store as vector<int> [4] ?
-			else
+					server->host = "0.0.0.0";
-			{
+					server->value = value;
-			// maybe do this differently?
+				}
-				std::vector<std::string> tmp = split(value, ':');
+				else
-			// i might take issue with this, will see
+				{
-				if (server->host != "" && server->host != tmp[0])
+				// maybe do this differently?
-					throw std::invalid_argument("bad config file arguments");
+					std::vector<std::string> tmp2 = split(value, ':');
-				server->host = tmp[0];
+				// i might take issue with this, will see
-				server->port = tmp[1];
+					if (server->host != "" && server->host != tmp2[0])
-			}
+						throw std::invalid_argument("bad listen");
-		case "root":
+					server->host = tmp[0];
-			server->root = value;
+					server->port = tmp[1];
-		case "index":
+				}
-			std::vector<std::string> tmp = split(value, ' ');
+			case "root":
-			for (unsigned long i = 0; i != tmp.size(); i++)
+				server->root = value;
-				server->index.push_back(tmp[i]);
+			case "autoindex":
-		case "allow_methods":
+				server->autoindex = (value == "on" ? true : false);
-			std::vector<std::string> tmp = split(value, ' ');
+			case "client_body_limit":
-			// might do something different here
+				server->client_body_limit = atoi(value.c_str());
-				// like change how methods are stored?
+			case "recv_timeout":
-			for (unsigned long i = 0; i != tmp.size(); i++)
+				// what is tv_sec and do i need it?
-				server->allow_methods.push_back(str_to_method_type(tmp[i]));
+				server->recv_timeout.tv_sec = atoi(value.c_str());
-		case "autoindex":
+			case "send_timeout":
-			server->autoindex = (value == "on" ? true : false);
+				server->send_timeout.tv_sec = atoi(value.c_str());
-		case "client_body_limit":
+			default :
-			server->client_body_limit = atoi(value.c_str());
+				throw std::invalid_argument("should only have 1 value");
-		case "recv_timeout":
+			// yea ok but it could also be something else like too many
-			// what is tv_sec and do i need it?
+			// args
-			server->recv_timeout.tv_sec = atoi(value.c_str());
+		}
-		case "send_timeout":
-			server->send_timeout.tv_sec = atoi(value.c_str());
-		case "return":
-			std::vector<std::string> tmp = split(value, ' ');
-			server->redirect_status = atoi(tmp[0].c_str());
-			server->redirect_uri = tmp[1];
-		case "error_page":
-			std::vector<std::string> tmp = split(value, ' ');
-			std::string	path = tmp[tmp.size() - 1];
-			for (unsigned long i = 0; i != tmp.size() - 1; i++)
-			{
-				int	status_code = atoi(tmp[i].c_str());
-				// yea IDK i might not want to store this like that...
-				if (server->error_pages.find(status_code) != server->error_pages.end())
-					continue ;
-				server->error_pages[status_code] = path;
-			}
-		default :
-			throw std::invalid_argument("bad config file arguments");
 	}
-//	return (1);	// for now but prolly will change...
+	else if (tmp.size() > 1)
+	{
+		switch (key)
+		{
+			case "index":
+				// could run more tests on value content but meh...
+				for (unsigned long i = 0; i != tmp.size(); i++)
+					server->index.push_back(tmp[i]);
+			case "allow_methods":
+				// might do something different here
+					// like change how methods are stored?
+				for (unsigned long i = 0; i != tmp.size(); i++)
+					server->allow_methods.push_back(str_to_method_type(tmp[i]));
+			case "return":
+				// could run more checks here too
+				// like tmp.size() must be 2
+				// and tmp[0] should be a number and tmp[1] a string?
+				server->redirect_status = atoi(tmp[0].c_str());
+				server->redirect_uri = tmp[1];
+			case "error_page":
+				// something more complicated?
+				// like make sure ints then 1 string?
+				std::string	path = tmp[tmp.size() - 1];
+				for (unsigned long i = 0; i != tmp.size() - 1; i++)
+				{
+					int	status_code = atoi(tmp[i].c_str());
+					// yea IDK i might not want to store this like that...
+					if (server->error_pages.find(status_code) != server->error_pages.end())
+						continue ;
+					server->error_pages[status_code] = path;
+				}
+			default :
+				throw std::invalid_argument("bad config file arguments");
+		}
+	}
+	else
+		throw std::invalid_argument("missing value");
+		
 }
 
 // again not sure i want an int ret
-int	ConfigParser::_set_location_value(Location *location, const std::string key, const std::string value)
+int	ConfigParser::_set_location_values(Location *location, const std::string key, const std::string value)
 {
-	switch (key)
+	// check key for ;
+	// check values for ; at end and right number of words depending on key
+
+	if (key.find_first_of(";") != std::string::npos)
+		throw std::invalid_argument("bad config file arguments");
+
+	// there shouldn't be any tabs, right? not between values...
+	if (value.find_first_of("\t") != std::string::npos)
+		throw std::invalid_argument("bad config file arguments");
+		
+	size_t	i = value.find_first_of(";");
+	// so you can't have no ;
+	// you can't have just ;
+	// and you can't have a ; not at the end or several ;
+	// in theory value_find_last_of should find the only ;
+	if (i == std::string::npos || (value.find_last_not_of(" \n")) != i \
+		|| value.compare(";") == 0)
+		throw std::invalid_argument("bad config file arguments");
+
+
+	// we Trim value.
+	// is this valid?
+	value = value.substr(0, i - 1);
+
+	std::vector<std::string> tmp = ::split(value, ' ');
+
+	if (tmp.size() == 1)
 	{
-		case "root":
+		switch (key)
-			location->root = value;
+		{
-		case "index":
+			case "root":
-			std::vector<std::string> tmp = split(value, ' ');
+				location->root = value;
-			for (unsigned long i = 0; i != tmp.size(); i++)
+			case "client_body_limit":
-				location->index.push_back(tmp[i]);
+				location->client_body_limit = atoi(value.c_str());
-		case "allow_methods":
+			default :
-			std::vector<std::string> tmp = split(value, ' ');
+				throw std::invalid_argument("should only have 1 argument");
-			for (unsigned long i = 0; i != tmp.size(); i++)
+
-				location->allow_methods.push_back(str_to_methodtype(tmp[i]));
+		}
-		case "cgi_info":
-			// ok wtf is all this even doing, figure that out
-			unsigned long	i = value.find_first_of(" ");
-			if (i == std::string::npos)
-				throw std::invalid_argument("bad config file arguments");
-			// ok why an int now, we gotta be more consistent!
-			int	j = value.find_first_not_of(" ", i);
-			location->cgi_info[value.substr(0, i)] = value.substr(j, value.length());
-		case "client_body_limit":
-			location->client_body_limit = atoi(value.c_str());
-		default :
-			throw std::invalid_argument("bad config file arguments");
 	}
-	return (1);	// again prolly want to change this...
+	else if (tmp.size() > 1)
+	{
+		switch (key)
+		{
+			case "index":
+				std::vector<std::string> tmp = split(value, ' ');
+				for (unsigned long i = 0; i != tmp.size(); i++)
+					location->index.push_back(tmp[i]);
+			case "allow_methods":
+				std::vector<std::string> tmp = split(value, ' ');
+				for (unsigned long i = 0; i != tmp.size(); i++)
+					location->allow_methods.push_back(str_to_methodtype(tmp[i]));
+			case "cgi_info":
+				// ok wtf is all this even doing, figure that out
+				unsigned long	i = value.find_first_of(" ");
+				if (i == std::string::npos)
+					throw std::invalid_argument("bad config file arguments");
+				// ok why an int now, we gotta be more consistent!
+				int	j = value.find_first_not_of(" ", i);
+				location->cgi_info[value.substr(0, i)] = value.substr(j, value.length());
+			default :
+				throw std::invalid_argument("bad config file arguments");
+
+		}
+	}
+	else
+		throw std::invalid_argument("missing a value");
+		
 }
 
-
+// assumes curr is on a space or \t or \n
-// shit, this doesn't work...
+// get first word? next word? word?
-	// wait it might...
+std::string	ConfigParser::_get_first_word(size_t *curr)
-	// definitely need a better name...
-// _check_proper_word_break ?
-// get next word ?
-void	ConfigParser::_check_proper_line_end(size_t *prev, size_t *curr)
 {
-	if ((*prev = _content.find_first_not_of(" \t\n", *curr)) == std::string::npos)
+	size_t	start;
+
+// are these checks excessive?
+	if ((start = _content.find_first_not_of(" \t\n", *curr)) == std::string::npos)
 		throw std::invalid_argument("bad config file arguments");
-	if ((*curr = _content.find_first_of(" \t\n", *prev)) == std::string::npos)
+	if ((*curr = _content.find_first_of(" \t\n", start)) == std::string::npos)
 		throw std::invalid_argument("bad config file arguments");
+
+	std::string	key = _content.substr(start, *curr - start);
+
+	return (key);
 }
 
-
+// also assumes curr is on a space \t or \n
-// no longer returning an int
+std::string	ConfigParser::_get_rest_of_line(size_t *curr)
-// fuck no we have to return an int...
-	// ok keep FAILED as being -1
-// i mean we could send a pointer to value_end and then ret void, but why
-	// bother chaning it now...
-int ConfigParser::_check_for_semicolon(std::string line)
 {
-	// yea ok i just don't like this...
+	size_t	start;
 
-	// line must be end with semicolon
+	if ((start = _content.find_first_not_of(" \t\n", *curr)) == std::string::npos)
-	size_t semicol;
+		throw std::invalid_argument("bad config file arguments");
-	size_t find;
+	if ((*curr = _content.find_first_of("\n", start)) == std::string::npos)
+		throw std::invalid_argument("bad config file arguments");
 
-	semicol = line.find_first_of(";");
+	std::string	values = _content.substr(start, *curr);
-	if (semicol == std::string::npos)
+
-		return FAILED;
+	return (values);
-	find = line.find_first_not_of(" \t\n", semicol + 1, line.length() - semicol - 1);
-	if (find != std::string::npos)
-		return FAILED;
-	find = line.find_last_not_of(" \t", semicol - 1);
-	return find;
 }
 
 
+
 void	ConfigParser::_print_content() const
 {
 	std::cout << _content;