ICU-21351 Don't coalesce adjacent list formatter fields in ICU4J
diff --git a/icu4c/source/i18n/formattedval_sbimpl.cpp b/icu4c/source/i18n/formattedval_sbimpl.cpp
index 84c2d00..64601f4 100644
--- a/icu4c/source/i18n/formattedval_sbimpl.cpp
+++ b/icu4c/source/i18n/formattedval_sbimpl.cpp
@@ -170,7 +170,10 @@
auto elementField = fString.getFieldPtr()[i-1];
if (elementField == Field(UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD)
&& cfpos.matchesField(elementField.getCategory(), elementField.getField())
- && (cfpos.getLimit() < i - fString.fZero || cfpos.getCategory() != elementField.getCategory())) {
+ && (
+ cfpos.getLimit() < i - fString.fZero
+ || cfpos.getCategory() != elementField.getCategory()
+ || cfpos.getField() != elementField.getField())) {
int64_t si = cfpos.getInt64IterationContext() - 1;
cfpos.setState(
elementField.getCategory(),
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java
index 43f0c4d..938eb13 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java
@@ -34,6 +34,7 @@
public UFormat.SpanField spanField;
public Field normalField;
public Object value;
+ public int length;
}
/**
@@ -142,12 +143,6 @@
if (currField != null) {
if (currField != _field) {
int end = i - self.zero;
- // Handle span fields; don't trim them
- if (currField instanceof SpanFieldPlaceholder) {
- boolean handleResult = handleSpan(currField, cfpos, fieldStart, end);
- assert handleResult;
- return true;
- }
// Grouping separators can be whitespace; don't throw them out!
if (isTrimmable(currField)) {
end = trimBack(self, end);
@@ -188,6 +183,7 @@
&& (i - self.zero > cfpos.getLimit() || cfpos.getField() != numericField)
&& isNumericField(self.fields[i - 1])
&& !isNumericField(_field)) {
+ // Re-wind to the beginning of the field and then emit it
int j = i - 1;
for (; j >= self.zero && isNumericField(self.fields[j]); j--) {}
cfpos.setState(numericField, null, j - self.zero + 1, i - self.zero);
@@ -196,9 +192,11 @@
// Special case: emit normalField if we are pointing at the end of spanField.
if (i > self.zero
&& self.fields[i-1] instanceof SpanFieldPlaceholder) {
- int j = i - 1;
- for (; j >= self.zero && self.fields[j] == self.fields[i-1]; j--) {}
- if (handleSpan(self.fields[i-1], cfpos, j - self.zero + 1, i - self.zero)) {
+ SpanFieldPlaceholder ph = (SpanFieldPlaceholder) self.fields[i-1];
+ if (cfpos.matchesField(ph.normalField, null)
+ && (cfpos.getLimit() < i - self.zero
+ || cfpos.getField() != ph.normalField)) {
+ cfpos.setState(ph.normalField, null, i - self.zero - ph.length, i - self.zero);
return true;
}
}
@@ -214,9 +212,15 @@
// Case 3a: SpanField placeholder
if (_field instanceof SpanFieldPlaceholder) {
SpanFieldPlaceholder ph = (SpanFieldPlaceholder) _field;
- if (cfpos.matchesField(ph.normalField, null) || cfpos.matchesField(ph.spanField, ph.value)) {
+ if (cfpos.matchesField(ph.spanField, ph.value)) {
fieldStart = i - self.zero;
- currField = _field;
+ int end = fieldStart + ph.length;
+ cfpos.setState(ph.spanField, ph.value, fieldStart, end);
+ return true;
+ } else {
+ // Failed to match; jump ahead
+ i += ph.length - 1;
+ continue;
}
}
// Case 3b: No SpanField
@@ -258,19 +262,4 @@
return StaticUnicodeSets.get(StaticUnicodeSets.Key.DEFAULT_IGNORABLES)
.span(self, start, UnicodeSet.SpanCondition.CONTAINED);
}
-
- private static boolean handleSpan(Object field, ConstrainedFieldPosition cfpos, int start, int limit) {
- SpanFieldPlaceholder ph = (SpanFieldPlaceholder) field;
- if (cfpos.matchesField(ph.spanField, ph.value)
- && cfpos.getLimit() < limit) {
- cfpos.setState(ph.spanField, ph.value, start, limit);
- return true;
- }
- if (cfpos.matchesField(ph.normalField, null)
- && (cfpos.getLimit() < limit || cfpos.getField() != ph.normalField)) {
- cfpos.setState(ph.normalField, null, start, limit);
- return true;
- }
- return false;
- }
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java b/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java
index 10503d4..e746e8f 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java
@@ -724,14 +724,16 @@
}
private void appendElement(Object element, int position) {
+ String elementString = element.toString();
if (needsFields) {
SpanFieldPlaceholder field = new SpanFieldPlaceholder();
field.spanField = SpanField.LIST_SPAN;
field.normalField = Field.ELEMENT;
field.value = position;
- string.append(element.toString(), field);
+ field.length = elementString.length();
+ string.append(elementString, field);
} else {
- string.append(element.toString(), null);
+ string.append(elementString, null);
}
}
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java
index 51a39f4..227abdd 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java
@@ -148,6 +148,11 @@
public static void checkFormattedValue(String message, FormattedValue fv, String expectedString,
Object[][] expectedFieldPositions) {
+ checkFormattedValue(message, fv, expectedString, expectedFieldPositions, false);
+ }
+
+ public static void checkFormattedValue(String message, FormattedValue fv, String expectedString,
+ Object[][] expectedFieldPositions, boolean skipAttributedCharacterIterator) {
// Calculate some initial expected values
int stringLength = fv.toString().length();
HashSet<Format.Field> uniqueFields = new HashSet<>();
@@ -169,6 +174,12 @@
assertEquals(baseMessage + "Iterator should have length of string output", stringLength, fpi.getEndIndex());
int i = 0;
for (char c = fpi.first(); c != AttributedCharacterIterator.DONE; c = fpi.next(), i++) {
+ // Strings with adjacent fields cannot be disambiguated using AttributedCharacterIterator,
+ // so skip this part of the test for strings with adjacent fields.
+ if (skipAttributedCharacterIterator) {
+ i = stringLength;
+ break;
+ }
Set<AttributedCharacterIterator.Attribute> currentAttributes = fpi.getAttributes().keySet();
int attributesRemaining = currentAttributes.size();
for (Object[] cas : expectedFieldPositions) {
@@ -181,22 +192,22 @@
continue;
}
- assertTrue(baseMessage + "Character at " + i + " should have field " + expectedField,
+ assertTrue(baseMessage + "A Character at " + i + " should have field " + expectedField,
currentAttributes.contains(expectedField));
- assertTrue(baseMessage + "Field " + expectedField + " should be a known attribute",
+ assertTrue(baseMessage + "A Field " + expectedField + " should be a known attribute",
allAttributes.contains(expectedField));
int actualBeginIndex = fpi.getRunStart(expectedField);
int actualEndIndex = fpi.getRunLimit(expectedField);
Object actualValue = fpi.getAttribute(expectedField);
- assertEquals(baseMessage + expectedField + " begin @" + i, expectedBeginIndex, actualBeginIndex);
- assertEquals(baseMessage + expectedField + " end @" + i, expectedEndIndex, actualEndIndex);
- assertEquals(baseMessage + expectedField + " value @" + i, expectedValue, actualValue);
+ assertEquals(baseMessage + expectedField + " A begin @" + i, expectedBeginIndex, actualBeginIndex);
+ assertEquals(baseMessage + expectedField + " A end @" + i, expectedEndIndex, actualEndIndex);
+ assertEquals(baseMessage + expectedField + " A value @" + i, expectedValue, actualValue);
attributesRemaining--;
}
- assertEquals(baseMessage + "Should have looked at every field: " + i + ": " + currentAttributes,
+ assertEquals(baseMessage + "A Should have looked at every field: " + i + ": " + currentAttributes,
0, attributesRemaining);
}
- assertEquals(baseMessage + "Should have looked at every character", stringLength, i);
+ assertEquals(baseMessage + "A Should have looked at every character", stringLength, i);
// Check nextPosition over all fields
ConstrainedFieldPosition cfpos = new ConstrainedFieldPosition();
@@ -207,16 +218,16 @@
int expectedStart = (Integer) cas[1];
int expectedLimit = (Integer) cas[2];
Object expectedValue = cas.length == 4 ? cas[3] : null;
- assertEquals(baseMessage + "field " + i, expectedField, cfpos.getField());
- assertEquals(baseMessage + "start " + i, expectedStart, cfpos.getStart());
- assertEquals(baseMessage + "limit " + i, expectedLimit, cfpos.getLimit());
- assertEquals(baseMessage + "value " + i, expectedValue, cfpos.getFieldValue());
+ assertEquals(baseMessage + "B field " + i, expectedField, cfpos.getField());
+ assertEquals(baseMessage + "B start " + i, expectedStart, cfpos.getStart());
+ assertEquals(baseMessage + "B limit " + i, expectedLimit, cfpos.getLimit());
+ assertEquals(baseMessage + "B value " + i, expectedValue, cfpos.getFieldValue());
i++;
}
boolean afterLoopResult = fv.nextPosition(cfpos);
- assertFalse(baseMessage + "after loop: " + cfpos, afterLoopResult);
+ assertFalse(baseMessage + "B after loop: " + cfpos, afterLoopResult);
afterLoopResult = fv.nextPosition(cfpos);
- assertFalse(baseMessage + "after loop again: " + cfpos, afterLoopResult);
+ assertFalse(baseMessage + "B after loop again: " + cfpos, afterLoopResult);
// Check nextPosition constrained over each class one at a time
for (Class<?> classConstraint : uniqueFieldClasses) {
@@ -232,22 +243,22 @@
int expectedStart = (Integer) cas[1];
int expectedLimit = (Integer) cas[2];
Object expectedValue = cas.length == 4 ? cas[3] : null;
- assertEquals(baseMessage + "field " + i, expectedField, cfpos.getField());
- assertEquals(baseMessage + "start " + i, expectedStart, cfpos.getStart());
- assertEquals(baseMessage + "limit " + i, expectedLimit, cfpos.getLimit());
- assertEquals(baseMessage + "value " + i, expectedValue, cfpos.getFieldValue());
+ assertEquals(baseMessage + "C field " + i, expectedField, cfpos.getField());
+ assertEquals(baseMessage + "C start " + i, expectedStart, cfpos.getStart());
+ assertEquals(baseMessage + "C limit " + i, expectedLimit, cfpos.getLimit());
+ assertEquals(baseMessage + "C value " + i, expectedValue, cfpos.getFieldValue());
i++;
}
afterLoopResult = fv.nextPosition(cfpos);
- assertFalse(baseMessage + "after loop: " + cfpos, afterLoopResult);
+ assertFalse(baseMessage + "C after loop: " + cfpos, afterLoopResult);
afterLoopResult = fv.nextPosition(cfpos);
- assertFalse(baseMessage + "after loop again: " + cfpos, afterLoopResult);
+ assertFalse(baseMessage + "C after loop again: " + cfpos, afterLoopResult);
}
// Check nextPosition constrained over an unrelated class
cfpos.reset();
cfpos.constrainClass(HashSet.class);
- assertFalse(baseMessage + "unrelated class", fv.nextPosition(cfpos));
+ assertFalse(baseMessage + "C unrelated class", fv.nextPosition(cfpos));
// Check nextPosition constrained over each field one at a time
for (Format.Field field : uniqueFields) {
@@ -263,16 +274,16 @@
int expectedStart = (Integer) cas[1];
int expectedLimit = (Integer) cas[2];
Object expectedValue = cas.length == 4 ? cas[3] : null;
- assertEquals(baseMessage + "field " + i, expectedField, cfpos.getField());
- assertEquals(baseMessage + "start " + i, expectedStart, cfpos.getStart());
- assertEquals(baseMessage + "limit " + i, expectedLimit, cfpos.getLimit());
- assertEquals(baseMessage + "value " + i, expectedValue, cfpos.getFieldValue());
+ assertEquals(baseMessage + "D field " + i, expectedField, cfpos.getField());
+ assertEquals(baseMessage + "D start " + i, expectedStart, cfpos.getStart());
+ assertEquals(baseMessage + "D limit " + i, expectedLimit, cfpos.getLimit());
+ assertEquals(baseMessage + "D value " + i, expectedValue, cfpos.getFieldValue());
i++;
}
afterLoopResult = fv.nextPosition(cfpos);
- assertFalse(baseMessage + "after loop: " + cfpos, afterLoopResult);
+ assertFalse(baseMessage + "D after loop: " + cfpos, afterLoopResult);
afterLoopResult = fv.nextPosition(cfpos);
- assertFalse(baseMessage + "after loop again: " + cfpos, afterLoopResult);
+ assertFalse(baseMessage + "D after loop again: " + cfpos, afterLoopResult);
}
}
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java
index 9297752..9407bd5 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java
@@ -262,15 +262,15 @@
{ListFormatter.Field.ELEMENT, 2, 9},
{ListFormatter.SpanField.LIST_SPAN, 9, 12, 2},
{ListFormatter.Field.ELEMENT, 9, 12}};
- if (!logKnownIssue("21351", "Java still coalesces adjacent elements")) {
- FormattedValueTest.checkFormattedValue(
- message,
- result,
- expectedString,
- expectedFieldPositions);
- }
+ FormattedValueTest.checkFormattedValue(
+ message,
+ result,
+ expectedString,
+ expectedFieldPositions,
+ // Adjacent fields: skip AttributedCharacterIterator test
+ true);
}
-
+
{
ListFormatter fmt = ListFormatter.getInstance(ULocale.ENGLISH, Type.UNITS, Width.SHORT);
String message = "ICU-21383 Long list";
