Merge branch 'fix-the-comments-again' into 'master'

Restore and clarify comments damaged by !2971

Closes #7350 and #7349

See merge request OpenMW/openmw!2979

components/config/gamesettings.cpp

@@ -179,11 +179,18 @@ bool Config::GameSettings::writeFile(QTextStream& stream)
     {
         i--;
 
+        // path lines (e.g. 'data=...') need quotes and ampersands escaping to match how boost::filesystem::path uses
+        // boost::io::quoted
+        // We don't actually use boost::filesystem::path anymore, but use a custom class MaybeQuotedPath which uses
+        // Boost-like quoting rules but internally stores as a std::filesystem::path.
+        // Caution: This is intentional behaviour to duplicate how Boost and what we replaced it with worked, and we
+        // rely on that.
         if (i.key() == QLatin1String("data") || i.key() == QLatin1String("data-local")
             || i.key() == QLatin1String("resources") || i.key() == QLatin1String("load-savegame"))
         {
             stream << i.key() << "=";
 
+            // Equivalent to stream << std::quoted(i.value(), '"', '&'), which won't work on QStrings.
             QChar delim = '\"';
             QChar escape = '&';
             QString string = i.value();
@@ -396,13 +403,18 @@ bool Config::GameSettings::writeFileWithComments(QFile& file)
     {
         it--;
 
+        // path lines (e.g. 'data=...') need quotes and ampersands escaping to match how boost::filesystem::path uses
+        // boost::io::quoted
+        // We don't actually use boost::filesystem::path anymore, but use a custom class MaybeQuotedPath which uses
+        // Boost-like quoting rules but internally stores as a std::filesystem::path.
+        // Caution: This is intentional behaviour to duplicate how Boost and what we replaced it with worked, and we
+        // rely on that.
         if (it.key() == QLatin1String("data") || it.key() == QLatin1String("data-local")
             || it.key() == QLatin1String("resources") || it.key() == QLatin1String("load-savegame"))
         {
             settingLine = it.key() + "=";
 
-            // The following is based on boost::io::detail::quoted_manip.hpp, but calling those functions did not work
-            // as there are too may QStrings involved
+            // Equivalent to settingLine += std::quoted(it.value(), '"', '&'), which won't work on QStrings.
             QChar delim = '\"';
             QChar escape = '&';
             QString string = it.value();

components/files/configurationmanager.cpp

@@ -440,12 +440,22 @@ namespace Files
 
     std::istream& operator>>(std::istream& istream, MaybeQuotedPath& MaybeQuotedPath)
     {
-        // If the stream starts with a double quote, read from stream using std::filesystem::path rules, then discard
-        // anything remaining. This prevents boost::program_options getting upset that we've not consumed the whole
-        // stream. If it doesn't start with a double quote, read the whole thing verbatim
+        // If the stream starts with a double quote, read from stream using boost::filesystem::path rules (which are not
+        // the same as std::filesystem::path rules), then discard anything remaining. This prevents
+        // boost::program_options getting upset that we've not consumed the whole stream. If it doesn't start with a
+        // double quote, read the whole thing verbatim.
+
+        // This function's implementation and comments have a history of being changed by well-meaning but incorrect
+        // OpenMW developers. Please be very careful you aren't joining them. If you're insufficiently apprehensive,
+        // here's a 737-word GitLab comment to scare you:
+        // https://gitlab.com/OpenMW/openmw/-/merge_requests/2979#note_1371082428.
         if (istream.peek() == '"')
         {
             std::string intermediate;
+            // std::filesystem::path would use '"', '\' here.
+            // We use & because it's easier when copying and pasting Windows paths, which have many backslashes and few
+            // ampersands, and because it's backwards-compatible with the previous format, which used
+            // boost::filesystem::path's operator>>.
             istream >> std::quoted(intermediate, '"', '&');
             static_cast<std::filesystem::path&>(MaybeQuotedPath) = Misc::StringUtils::stringToU8String(intermediate);
             if (istream && !istream.eof() && istream.peek() != EOF)