pugixml icon indicating copy to clipboard operation
pugixml copied to clipboard

Add support for std::filesystem in pugi::xml_document::load_file()

Open DanForever opened this issue 3 years ago • 1 comments

This is kinda minor, but it would be awesome if I could pass in a std::filesystem::path instead of having to convert it to const char*: std::filesystem::path path = FindPathToBuildXml(filename); if (path.empty()) return 1;

pugi::xml_document doc;
pugi::xml_parse_result result = doc.load_file(path.c_str()); // <-- with native support, we wouldn't need to call .c_str(), and internally, you might choose to get the unicode path instead
if (!result)
	return -2;

DanForever avatar Sep 20 '22 11:09 DanForever

@DanForever Note that path::c_str() returns path::value_type const *, so wchar_t on Windows and char on Unixoids. The resulting string is hence already a "Unicode path". (similar for path::native() etc., but not for path::string(), path::wstring() etc.) Also this would pick the correct overload of xml_document::load_file() on each platform.

brandl-muc avatar Sep 20 '22 12:09 brandl-muc

Yeah, .c_str() is already the correct way to handle this. My understanding is that it's not really legal to forward-declare std::filesystem::path (and, in practical terms, pugixml used to try to forward declare a set of STL objects and this was a significant compatibility hazard, so since then I've resigned to include the necessary STL headers) - thus implementing this would require including <filesystem> header when available, which can negatively impact compilation times. Finally, some platforms implement C++17 but don't provide fully functional <filesystem> headers either. Overall I feel like the extra maintenance burden here doesn't justify the minor convenience of not having to type .c_str().

zeux avatar Oct 04 '22 03:10 zeux