ICU-21351 Don't coalesce adjacent list formatter fields in ICU4J

This commit is contained in:
Shane F. Carr 2021-02-04 01:25:53 -06:00
parent 790dfcf0b7
commit 9ac51f21ed
5 changed files with 68 additions and 63 deletions

View file

@ -170,7 +170,10 @@ bool FormattedValueStringBuilderImpl::nextPositionImpl(ConstrainedFieldPosition&
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(),

View file

@ -34,6 +34,7 @@ public class FormattedValueStringBuilderImpl {
public UFormat.SpanField spanField;
public Field normalField;
public Object value;
public int length;
}
/**
@ -142,12 +143,6 @@ public class FormattedValueStringBuilderImpl {
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 @@ public class FormattedValueStringBuilderImpl {
&& (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 @@ public class FormattedValueStringBuilderImpl {
// 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 @@ public class FormattedValueStringBuilderImpl {
// 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 @@ public class FormattedValueStringBuilderImpl {
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;
}
}

View file

@ -724,14 +724,16 @@ final public class ListFormatter {
}
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);
}
}

View file

@ -148,6 +148,11 @@ public class FormattedValueTest {
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 @@ public class FormattedValueTest {
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 @@ public class FormattedValueTest {
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 @@ public class FormattedValueTest {
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 @@ public class FormattedValueTest {
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 @@ public class FormattedValueTest {
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);
}
}

View file

@ -262,15 +262,15 @@ public class ListFormatterTest extends TestFmwk {
{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";