Add support for localized time strings with two units, eg. "2 hours 17 minutes"

ui/base/l10n/formatter.cc

Index: ui/base/l10n/formatter.cc
diff --git a/ui/base/l10n/formatter.cc b/ui/base/l10n/formatter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9733d4170c85428214745bc9e412254941005dd6
--- /dev/null
+++ b/ui/base/l10n/formatter.cc
@@ -0,0 +1,312 @@
+// Copyright 2014 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/l10n/formatter.h"
+
+#include <vector>
+
+#include "grit/ui_strings.h"
+#include "third_party/icu/source/common/unicode/unistr.h"
+#include "ui/base/l10n/l10n_util_plurals.h"
+
+namespace ui {
+
+// static
+bool FormatterContainer::ForceFallback_ = false;
+
+static const size_t kNPluralities = 6;
+struct Pluralities {
+  int Ids[kNPluralities];

[bartfab (slow), 2014/02/18 12:04:04]

Nit: User hacker_style, not CamelCaps for member names.

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  const char *FallbackOne;

[bartfab (slow), 2014/02/18 12:04:04]

Nit: Put * on the type, not the member.

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  const char *FallbackOther;
+};
+
+static const Pluralities IDS_ELAPSED_SHORT_SEC = {
+  { IDS_TIME_ELAPSED_SECS_DEFAULT, IDS_TIME_ELAPSED_SEC_SINGULAR,
+    IDS_TIME_ELAPSED_SECS_ZERO, IDS_TIME_ELAPSED_SECS_TWO,
+    IDS_TIME_ELAPSED_SECS_FEW, IDS_TIME_ELAPSED_SECS_MANY },
+  "one{# sec ago}",
+  " other{# secs ago}"
+};
+static const Pluralities IDS_ELAPSED_SHORT_MIN = {
+  { IDS_TIME_ELAPSED_MINS_DEFAULT, IDS_TIME_ELAPSED_MIN_SINGULAR,
+    IDS_TIME_ELAPSED_MINS_ZERO, IDS_TIME_ELAPSED_MINS_TWO,
+    IDS_TIME_ELAPSED_MINS_FEW, IDS_TIME_ELAPSED_MINS_MANY },
+  "one{# min ago}",
+  " other{# mins ago}"
+};
+static const Pluralities IDS_ELAPSED_HOUR = {
+  { IDS_TIME_ELAPSED_HOURS_DEFAULT, IDS_TIME_ELAPSED_HOUR_SINGULAR,
+    IDS_TIME_ELAPSED_HOURS_ZERO, IDS_TIME_ELAPSED_HOURS_TWO,
+    IDS_TIME_ELAPSED_HOURS_FEW, IDS_TIME_ELAPSED_HOURS_MANY },
+  "one{# hour ago}",
+  " other{# hours ago}"
+};
+static const Pluralities IDS_ELAPSED_DAY = {
+  { IDS_TIME_ELAPSED_DAYS_DEFAULT, IDS_TIME_ELAPSED_DAY_SINGULAR,
+    IDS_TIME_ELAPSED_DAYS_ZERO, IDS_TIME_ELAPSED_DAYS_TWO,
+    IDS_TIME_ELAPSED_DAYS_FEW, IDS_TIME_ELAPSED_DAYS_MANY },
+  "one{# day ago}",
+  " other{# days ago}"
+};
+
+static const Pluralities IDS_REMAINING_SHORT_SEC = {
+  { IDS_TIME_REMAINING_SECS_DEFAULT, IDS_TIME_REMAINING_SEC_SINGULAR,
+    IDS_TIME_REMAINING_SECS_ZERO, IDS_TIME_REMAINING_SECS_TWO,
+    IDS_TIME_REMAINING_SECS_FEW, IDS_TIME_REMAINING_SECS_MANY },
+  "one{# sec left}",
+  " other{# secs left}"
+};
+static const Pluralities IDS_REMAINING_SHORT_MIN = {
+  { IDS_TIME_REMAINING_MINS_DEFAULT, IDS_TIME_REMAINING_MIN_SINGULAR,
+    IDS_TIME_REMAINING_MINS_ZERO, IDS_TIME_REMAINING_MINS_TWO,
+    IDS_TIME_REMAINING_MINS_FEW, IDS_TIME_REMAINING_MINS_MANY },
+  "one{# min left}",
+  " other{# mins left}"
+};
+
+static const Pluralities IDS_REMAINING_LONG_SEC = {
+  { IDS_TIME_REMAINING_LONG_SECS_DEFAULT, IDS_TIME_REMAINING_LONG_SEC_SINGULAR,
+    IDS_TIME_REMAINING_LONG_SECS_ZERO, IDS_TIME_REMAINING_LONG_SECS_TWO,
+    IDS_TIME_REMAINING_LONG_SECS_FEW, IDS_TIME_REMAINING_LONG_SECS_MANY },
+  "one{# second left}",
+  " other{# seconds left}"
+};
+static const Pluralities IDS_REMAINING_LONG_MIN = {
+  { IDS_TIME_REMAINING_LONG_MINS_DEFAULT, IDS_TIME_REMAINING_LONG_MIN_SINGULAR,
+    IDS_TIME_REMAINING_LONG_MINS_ZERO, IDS_TIME_REMAINING_LONG_MINS_TWO,
+    IDS_TIME_REMAINING_LONG_MINS_FEW, IDS_TIME_REMAINING_LONG_MINS_MANY },
+  "one{# minute left}",
+  " other{# minutes left}"
+};
+static const Pluralities IDS_REMAINING_HOUR = {
+  { IDS_TIME_REMAINING_HOURS_DEFAULT, IDS_TIME_REMAINING_HOUR_SINGULAR,
+    IDS_TIME_REMAINING_HOURS_ZERO, IDS_TIME_REMAINING_HOURS_TWO,
+    IDS_TIME_REMAINING_HOURS_FEW, IDS_TIME_REMAINING_HOURS_MANY },
+  "one{# hour left}",
+  " other{# hours left}"
+};
+static const Pluralities IDS_REMAINING_DAY = {
+  { IDS_TIME_REMAINING_DAYS_DEFAULT, IDS_TIME_REMAINING_DAY_SINGULAR,
+    IDS_TIME_REMAINING_DAYS_ZERO, IDS_TIME_REMAINING_DAYS_TWO,
+    IDS_TIME_REMAINING_DAYS_FEW, IDS_TIME_REMAINING_DAYS_MANY },
+  "one{# day left}",
+  " other{# days left}"
+};
+
+static const Pluralities IDS_DURATION_SHORT_SEC = {
+  { IDS_TIME_SECS_DEFAULT, IDS_TIME_SEC_SINGULAR, IDS_TIME_SECS_ZERO,
+    IDS_TIME_SECS_TWO, IDS_TIME_SECS_FEW, IDS_TIME_SECS_MANY },
+  "one{# sec}",
+  " other{# secs}"
+};
+static const Pluralities IDS_DURATION_SHORT_MIN = {
+  { IDS_TIME_MINS_DEFAULT, IDS_TIME_MIN_SINGULAR, IDS_TIME_MINS_ZERO,
+    IDS_TIME_MINS_TWO, IDS_TIME_MINS_FEW, IDS_TIME_MINS_MANY },
+  "one{# min}",
+  " other{# mins}"
+};
+
+static const Pluralities IDS_LONG_SEC = {
+  { IDS_TIME_LONG_SECS_DEFAULT, IDS_TIME_LONG_SEC_SINGULAR,
+    IDS_TIME_LONG_SECS_ZERO, IDS_TIME_LONG_SECS_TWO,
+    IDS_TIME_LONG_SECS_FEW, IDS_TIME_LONG_SECS_MANY },
+  "one{# second}",
+  " other{# seconds}"
+};
+static const Pluralities IDS_LONG_MIN = {
+  { IDS_TIME_LONG_MINS_DEFAULT, IDS_TIME_LONG_MIN_SINGULAR,
+    IDS_TIME_LONG_MINS_ZERO, IDS_TIME_LONG_MINS_TWO,
+    IDS_TIME_LONG_MINS_FEW, IDS_TIME_LONG_MINS_MANY },
+  "one{# minute}",
+  " other{# minutes}"
+};
+static const Pluralities IDS_DURATION_HOUR = {
+  { IDS_TIME_HOURS_DEFAULT, IDS_TIME_HOUR_SINGULAR, IDS_TIME_HOURS_ZERO,
+    IDS_TIME_HOURS_TWO, IDS_TIME_HOURS_FEW, IDS_TIME_HOURS_MANY },
+  "one{# hour}",
+  " other{# hours}"
+};
+static const Pluralities IDS_DURATION_DAY = {
+  { IDS_TIME_DAYS_DEFAULT, IDS_TIME_DAY_SINGULAR, IDS_TIME_DAYS_ZERO,
+    IDS_TIME_DAYS_TWO, IDS_TIME_DAYS_FEW, IDS_TIME_DAYS_MANY },
+  "one{# day}",
+  " other{# days}"
+};
+
+static const Pluralities IDS_LONG_MIN_1ST = {
+  { IDS_TIME_LONG_MINS_1ST_DEFAULT, IDS_TIME_LONG_MIN_1ST_SINGULAR,
+    IDS_TIME_LONG_MINS_1ST_ZERO, IDS_TIME_LONG_MINS_1ST_TWO,
+    IDS_TIME_LONG_MINS_1ST_FEW, IDS_TIME_LONG_MINS_1ST_MANY },
+  "one{# minute }",
+  " other{# minutes }"
+};
+static const Pluralities IDS_LONG_SEC_2ND = {
+  { IDS_TIME_LONG_SECS_2ND_DEFAULT, IDS_TIME_LONG_SEC_2ND_SINGULAR,
+    IDS_TIME_LONG_SECS_2ND_ZERO, IDS_TIME_LONG_SECS_2ND_TWO,
+    IDS_TIME_LONG_SECS_2ND_FEW, IDS_TIME_LONG_SECS_2ND_MANY },
+  "one{# second}",
+  " other{# seconds}"
+};
+static const Pluralities IDS_DURATION_HOUR_1ST = {
+  { IDS_TIME_HOURS_1ST_DEFAULT, IDS_TIME_HOUR_1ST_SINGULAR,
+    IDS_TIME_HOURS_1ST_ZERO, IDS_TIME_HOURS_1ST_TWO,
+    IDS_TIME_HOURS_1ST_FEW, IDS_TIME_HOURS_1ST_MANY },
+  "one{# hour }",
+  " other{# hours }"
+};
+static const Pluralities IDS_LONG_MIN_2ND = {
+  { IDS_TIME_LONG_MINS_2ND_DEFAULT, IDS_TIME_LONG_MIN_2ND_SINGULAR,
+    IDS_TIME_LONG_MINS_2ND_ZERO, IDS_TIME_LONG_MINS_2ND_TWO,
+    IDS_TIME_LONG_MINS_2ND_FEW, IDS_TIME_LONG_MINS_2ND_MANY },
+  "one{# minute}",
+  " other{# minutes}"
+};
+static const Pluralities IDS_DURATION_DAY_1ST = {
+  { IDS_TIME_DAYS_1ST_DEFAULT, IDS_TIME_DAY_1ST_SINGULAR,
+    IDS_TIME_DAYS_1ST_ZERO, IDS_TIME_DAYS_1ST_TWO,
+    IDS_TIME_DAYS_1ST_FEW, IDS_TIME_DAYS_1ST_MANY },
+  "one{# day }",
+  " other{# days }"
+};
+static const Pluralities IDS_DURATION_HOUR_2ND = {
+  { IDS_TIME_HOURS_2ND_DEFAULT, IDS_TIME_HOUR_2ND_SINGULAR,
+    IDS_TIME_HOURS_2ND_ZERO, IDS_TIME_HOURS_2ND_TWO,
+    IDS_TIME_HOURS_2ND_FEW, IDS_TIME_HOURS_2ND_MANY },
+  "one{# hour}",
+  " other{# hours}"
+};
+
+Formatter::Formatter(const Pluralities& sec_pluralities,
+                     const Pluralities& min_pluralities,
+                     const Pluralities& hour_pluralities,
+                     const Pluralities& day_pluralities) {
+  format_[SEC].reset(InitFormat(sec_pluralities));
+  format_[MIN].reset(InitFormat(min_pluralities));
+  format_[HOUR].reset(InitFormat(hour_pluralities));
+  format_[DAY].reset(InitFormat(day_pluralities));
+}
+
+Formatter::Formatter(const Pluralities& sec_pluralities,
+                     const Pluralities& min_pluralities,
+                     const Pluralities& hour_pluralities,
+                     const Pluralities& day_pluralities,
+                     const Pluralities& min_sec_pluralities1,
+                     const Pluralities& min_sec_pluralities2,
+                     const Pluralities& hour_min_pluralities1,
+                     const Pluralities& hour_min_pluralities2,
+                     const Pluralities& day_hour_pluralities1,
+                     const Pluralities& day_hour_pluralities2) {
+  format_[SEC].reset(InitFormat(sec_pluralities));
+  format_[MIN].reset(InitFormat(min_pluralities));
+  format_[HOUR].reset(InitFormat(hour_pluralities));
+  format_[DAY].reset(InitFormat(day_pluralities));
+  detailed_format_[MIN_SEC][0].reset(InitFormat(min_sec_pluralities1));
+  detailed_format_[MIN_SEC][1].reset(InitFormat(min_sec_pluralities2));
+  detailed_format_[HOUR_MIN][0].reset(InitFormat(hour_min_pluralities1));
+  detailed_format_[HOUR_MIN][1].reset(InitFormat(hour_min_pluralities2));
+  detailed_format_[DAY_HOUR][0].reset(InitFormat(day_hour_pluralities1));
+  detailed_format_[DAY_HOUR][1].reset(InitFormat(day_hour_pluralities2));
+}
+
+void Formatter::Format(Unit unit, int value,

[bartfab (slow), 2014/02/18 12:04:04]

Nit: Per the style guide, a declaration/definition must either fit entirely on
one line or must specify one argument per line.

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+                       icu::UnicodeString& formatted_string) const {
+  UErrorCode error = U_ZERO_ERROR;

[bartfab (slow), 2014/02/18 12:04:04]

Nit: Why not move down the definition of |error| down to where you actually use
it?

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  DCHECK(format_[unit].get()) << "This can never happen.";

[bartfab (slow), 2014/02/18 12:04:04]

Nit 1: #include "base/logging.h"
Nit 2: format_[unit] can be truth-checked directly. No need for get().
Nit 3: "This can never happen" is the very premise of a DCHECK. No need to spell
it out explicitly.

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  formatted_string = format_[unit].get()->format(value, error);

[bartfab (slow), 2014/02/18 12:04:04]

Nit: No need for get() before ->

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  DCHECK(U_SUCCESS(error)) << "Formatter error.";
+  return;
+}
+
+void Formatter::Format(TwoUnits units, int value1, int value2,

[bartfab (slow), 2014/02/18 12:04:04]

Nit: Per the style guide, a declaration/definition must either fit entirely on
one line or must specify one argument per line.

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+                       icu::UnicodeString& formatted_string) const {
+  UErrorCode error = U_ZERO_ERROR;

[bartfab (slow), 2014/02/18 12:04:04]

Nit: Why not move down the definition of |error| down to where you actually use
it?

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  DCHECK(detailed_format_[units][0].get())

[bartfab (slow), 2014/02/18 12:04:04]

Nit: detailed_format_[unit][0] can be truth-checked directly. No need for get().

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+      << "Detailed() not implemented for your (type,length) combination!";
+  DCHECK(detailed_format_[units][1].get())

[bartfab (slow), 2014/02/18 12:04:04]

Nit: detailed_format_[unit][1] can be truth-checked directly. No need for get().

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+      << "Detailed() not implemented for your (type,length) combination!";
+  formatted_string = detailed_format_[units][0].get()->format(value1, error);

[bartfab (slow), 2014/02/18 12:04:04]

Nit: No need for get() before ->

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  DCHECK(U_SUCCESS(error));
+  formatted_string += detailed_format_[units][1].get()->format(value2, error);

[bartfab (slow), 2014/02/18 12:04:04]

Nit: No need for get() before ->

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+  DCHECK(U_SUCCESS(error));
+  return;
+}
+
+icu::PluralFormat* Formatter::CreateFallbackFormat(

[bartfab (slow), 2014/02/18 12:04:04]

Nit: Per the style guide, a declaration/definition must either fit entirely on
one line or must specify one argument per line.

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+    const icu::PluralRules& rules, const Pluralities& pluralities) const {
+  icu::UnicodeString pattern;
+  if (rules.isKeyword(UNICODE_STRING_SIMPLE("one"))) {

[bartfab (slow), 2014/02/18 12:04:04]

Nit: No curly braces necessary for single-line if.

[Thiemo Nagel, 2014/02/19 17:08:44]

Not necessary but optional, if I read the style guide correctly.

[bartfab (slow), 2014/02/20 16:31:22]

Yes, it is a matter of choice. But if you look at Chrome code, you will find
that single-line ifs do not use {}, except for rare cases where a multi-line if
was shortened over time, reaching one line eventually but nobody remembered to
remove the {}.

[Thiemo Nagel, 2014/02/22 21:44:09]

Done, though I'm not certain the code is more readable now.

+    pattern += icu::UnicodeString(pluralities.FallbackOne);
+  }
+  pattern += icu::UnicodeString(pluralities.FallbackOther);
+
+  UErrorCode err = U_ZERO_ERROR;
+  icu::PluralFormat* format = new icu::PluralFormat(rules, pattern, err);
+  DCHECK(U_SUCCESS(err));
+  return format;
+}
+
+icu::PluralFormat* Formatter::InitFormat(const Pluralities& pluralities) {
+  icu::UnicodeString pattern;
+  std::vector<int> ids;
+  for (size_t j = 0; j < kNPluralities; ++j) {

[bartfab (slow), 2014/02/18 12:04:04]

Nit: No need for curly braces when the body of a loop is only one line long.

[Thiemo Nagel, 2014/02/19 17:08:44]

I can't find a rule for that.

[bartfab (slow), 2014/02/20 16:31:22]

It's hiding in the rule for conditionals:

"In general, curly braces are not required for single-line statements, but they
are allowed if you like them; conditional or loop statements with complex
conditions or statements may be more readable with curly braces."

As you see, this sentence refers to both conditionals and loops. Again, it is a
matter of taste. Again, there is a strong preference in Chrome code toward no {}
for single-line statements.

[Thiemo Nagel, 2014/02/22 21:44:09]

Done.

[Thiemo Nagel, 2014/02/22 21:44:09]

Oh well...

+    ids.push_back(pluralities.Ids[j]);
+  }
+  scoped_ptr<icu::PluralFormat> format = l10n_util::BuildPluralFormat(ids);
+  if (format.get() && !FormatterContainer::GetFallbackForTesting())

[bartfab (slow), 2014/02/18 12:04:04]

Nit 1: scoped_ptr supports truth testing out of the box: if (format) works. No
need for format.get().

Nit 2: You could wrap the entire construction of |format| in an if
(!FormatterContainer::GetFallbackForTesting())

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+    return format.release();
+
+  scoped_ptr<icu::PluralRules> rules(l10n_util::BuildPluralRules());
+  return CreateFallbackFormat(*rules, pluralities);
+}
+
+const Formatter* FormatterContainer::Get(TimeFormat::Type type,
+                                         TimeFormat::Length length) const {
+  DCHECK(formatter_[type][length])
+      << "Combination of kElapsed and kLong is not implemented!";
+  return formatter_[type][length];
+}
+
+FormatterContainer::FormatterContainer() {

[bartfab (slow), 2014/02/18 12:04:04]

Nit: Can you not initialize formatter_ in the initializer list?

[Thiemo Nagel, 2014/02/19 17:08:44]

I'd love to, but that's C++11.

[bartfab (slow), 2014/02/20 16:31:22]

That's too bad...

+  formatter_[TimeFormat::kElapsed][TimeFormat::kShort] =
+      new Formatter(IDS_ELAPSED_SHORT_SEC,
+                    IDS_ELAPSED_SHORT_MIN,
+                    IDS_ELAPSED_HOUR,
+                    IDS_ELAPSED_DAY);
+  formatter_[TimeFormat::kElapsed][TimeFormat::kLong] = NULL;
+  formatter_[TimeFormat::kRemaining][TimeFormat::kShort] =
+      new Formatter(IDS_REMAINING_SHORT_SEC,
+                    IDS_REMAINING_SHORT_MIN,
+                    IDS_REMAINING_HOUR,
+                    IDS_REMAINING_DAY);
+  formatter_[TimeFormat::kRemaining][TimeFormat::kLong] =
+      new Formatter(IDS_REMAINING_LONG_SEC,
+                    IDS_REMAINING_LONG_MIN,
+                    IDS_REMAINING_HOUR,
+                    IDS_REMAINING_DAY);
+  formatter_[TimeFormat::kDuration][TimeFormat::kShort] =
+      new Formatter(IDS_DURATION_SHORT_SEC,
+                    IDS_DURATION_SHORT_MIN,
+                    IDS_DURATION_HOUR,
+                    IDS_DURATION_DAY);
+  formatter_[TimeFormat::kDuration][TimeFormat::kLong] =
+      new Formatter(IDS_LONG_SEC,
+                    IDS_LONG_MIN,
+                    IDS_DURATION_HOUR,
+                    IDS_DURATION_DAY,
+                    IDS_LONG_MIN_1ST,
+                    IDS_LONG_SEC_2ND,
+                    IDS_DURATION_HOUR_1ST,
+                    IDS_LONG_MIN_2ND,
+                    IDS_DURATION_DAY_1ST,
+                    IDS_DURATION_HOUR_2ND);
+}
+
+FormatterContainer::~FormatterContainer() {
+  for (int type = 0; type < TimeFormat::kNType; ++type)

[bartfab (slow), 2014/02/18 12:04:04]

Nit: This for loop needs curly braces because its body is two lines long.

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+    for (int length = 0; length < TimeFormat::kNLength; ++length)
+      delete formatter_[type][length];

[bartfab (slow), 2014/02/18 12:04:04]

Can you make formatter_ be an array of scoped_ptrs so that it destroys its
contents automatically?

[Thiemo Nagel, 2014/02/19 17:08:44]

Done.

+}
+
+}  // namespace ui