"Escape" certain characters

tools/gn/visual_studio_writer.cc

Index: tools/gn/visual_studio_writer.cc
diff --git a/tools/gn/visual_studio_writer.cc b/tools/gn/visual_studio_writer.cc
index 8a8cf528fc15c5ae99b1081a62b187f3c03d5ae7..f8021753247b7adcda50809e0f03fe682792cf38 100644
--- a/tools/gn/visual_studio_writer.cc
+++ b/tools/gn/visual_studio_writer.cc
@@ -37,9 +37,32 @@
 
 namespace {
 
+std::string EscapeString(const std::string& value) {
+  std::string result;
+  if (value.find_first_of("<>&\"\t\r\n") != std::string::npos) {

[brettw, 2017/04/07 18:16:43]

I'd delete this check and always do the loop below. find_first_of will basically
do the same thing your loop+switch does and you code copies the strings in all
cases anyway, so this extra check is just extra code and runtime work.

[kylix_rd, 2017/04/07 19:18:53]

Done.

+    for (std::string::const_iterator it = value.cbegin();

[brettw, 2017/04/07 18:16:43]

I would have used a range-based for loop here which is a lot easier to read:

  for (char c : value) {
    switch (c) {
      ...
    }

[kylix_rd, 2017/04/07 19:18:53]

Yes, that's better.
Done.

+         it != value.cend();
+         ++it) {
+      switch (*it) {
+        case '\n': result += "
"; break;

[brucedawson, 2017/04/05 18:51:39]

Do you have references for the \n, \r, and \t sequences? This document suggests
that there are only five legal escape sequences in XML:

http://stackoverflow.com/questions/1091945/what-characters-do-i-need-to-escap...

Speaking of which, you might as well support '.

[kylix_rd, 2017/04/05 18:55:02]

I based it off the code in libxml here:
https://cs.chromium.org/chromium/src/third_party/libxml/src/xmlsave.c?l=2049

+        case '\r': result += "
"; break;
+        case '\t': result += "	"; break;
+        case '"': result += """; break;
+        case '<': result += "&lt;"; break;
+        case '>': result += "&gt;"; break;
+        case '&': result += "&amp;"; break;
+        default:
+          result += *it;
+      }
+    }
+  } else

[brettw, 2017/04/07 18:16:43]

Google style says all arms in an if statement should match so this should have a
{ } (although I suggest deleting it above so this is just an FYI).

+    result = value;
+  return result;
+}
+
 struct SemicolonSeparatedWriter {
   void operator()(const std::string& value, std::ostream& out) const {
-    out << value + ';';
+    out << EscapeString(value) + ';';
   }
 };