[ui/base;css] adding string template expression replacement

ui/base/template_expressions.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/base/template_expressions.h"
+
+#include "base/logging.h"
+
+namespace {
+
+template <class FormatStringType, class OutStringType>
+OutStringType DoReplaceTemplateExpressions(
+    const FormatStringType& format_string,
+    const std::map<FormatStringType, OutStringType>& substitutions) {
+  OutStringType formatted;
+  const size_t kValueLengthGuess = 16;
+  formatted.reserve(format_string.length() +
+                    substitutions.size() * kValueLengthGuess);
+#if 1

[dschuyler, 2015/07/14 16:45:36]

Both the #if section and the #else section work properly.  It's a matter of
preference about which is more readable, I believe.  Which reads nicer to you?

[Nico, 2015/07/14 21:44:18]

I feel the first block is nicer, seems to be at a higher level of abstraction

[dschuyler, 2015/07/15 00:11:04]

I'm ok with either version.  I have a leaning toward the lower version (#else
side) because I can see what is happening easier.  

+  size_t copy_begin = 0;
+  size_t format_string_length = format_string.length();
+  while (copy_begin < format_string_length) {
+    size_t copy_end = format_string.find('$', copy_begin);
+    formatted.append(format_string.data(), copy_begin, copy_end - copy_begin);
+    if (copy_end == FormatStringType::npos)
+      break;
+    copy_begin = copy_end + 1;
+    if (format_string[copy_begin] == '{') {
+      ++copy_begin;
+      size_t key_end = format_string.find('}', copy_begin);
+      if (copy_end == FormatStringType::npos)
+        break;
+      FormatStringType key =
+          format_string.substr(copy_begin, key_end - copy_begin);
+      const auto& replacement = substitutions.find(key);
+      copy_begin = key_end + 1;
+      if (replacement != substitutions.end())
+        formatted.append(replacement->second);
+      // else
+      //   NOTREACHED() << "Missing template expression " << key;

[dschuyler, 2015/07/14 16:45:36]

What is a good way to handle this case so that it does the right thing in
release builds as well as unit tests?

[Nico, 2015/07/14 21:44:18]

Right Thing in what sense?

[dschuyler, 2015/07/15 00:11:04]

Some of the unit test try things that will fail to test those cases.   Right
now, if we fail out when there is something wrong in the replacement, the unit
test fails (even though I'm trying to test a failure case).  I see value in
testing both success and failure cases in unit tests, so I'd like to do so.  If
my opinion is wrong :) and I should only test successes, that would also answer
this. 

+    } else if (format_string[copy_begin] == '$') {
+      copy_end = format_string.find_first_not_of('$', copy_begin);
+      formatted.append(format_string.data(), copy_begin, copy_end - copy_begin);
+      copy_begin = copy_end;
+    }
+  }
+#else
+  typename FormatStringType::const_iterator i = format_string.begin();
+  while (i < format_string.end()) {
+    if (*i == '$') {
+      ++i;
+      if (i < format_string.end()) {
+        if (*i == '{') {
+          ++i;
+          OutStringType index;
+          while (i < format_string.end()) {
+            if (*i == '}') {
+              ++i;
+              break;
+            }
+            index.push_back(*i);
+            ++i;
+          }
+          const auto& replacement = substitutions.find(index);
+          if (replacement != substitutions.end()) {
+            formatted.append(replacement->second);
+          } else {
+            // NOTREACHED() << "Missing template expression " << index;

[dschuyler, 2015/07/14 16:45:36]

What is a good way to handle this case so that it does the right thing in
release builds as well as unit tests?

+          }
+        } else if (*i == '$') {
+          while (i < format_string.end() && *i == '$') {
+            formatted.push_back('$');
+            ++i;
+          }
+        }
+      }
+    } else {
+      formatted.push_back(*i);
+      ++i;
+    }
+  }
+#endif
+  return formatted;
+}
+
+}  // namespace
+
+namespace ui {
+
+std::string ReplaceTemplateExpressions(
+    base::StringPiece format_string,
+    const std::map<base::StringPiece, std::string>& substitutions) {
+  return DoReplaceTemplateExpressions(format_string, substitutions);
+}
+
+base::string16 ReplaceTemplateExpressions(
+    const base::string16& format_string,
+    const std::map<base::string16, base::string16>& substitutions) {
+  return DoReplaceTemplateExpressions(format_string, substitutions);
+}
+
+std::string ReplaceTemplateExpressions(
+    const std::string& format_string,
+    const std::map<std::string, std::string>& substitutions) {
+  return DoReplaceTemplateExpressions(format_string, substitutions);
+}
+
+}  // namespace ui