Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 106 additions & 6 deletions poi/src/main/java/org/apache/poi/ss/format/CellFormatPart.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more

import java.awt.*;
import java.util.*;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -50,16 +51,76 @@ public class CellFormatPart {
private static final Logger LOG = PoiLogManager.getLogger(CellFormatPart.class);

static final Map<String, Color> NAMED_COLORS;
static final List<Color> INDEXED_COLORS;

private final Color color;
private final CellFormatCondition condition;
private final CellFormatter format;
private final CellFormatType type;

static {
INDEXED_COLORS = new ArrayList<Color>(56);
INDEXED_COLORS.add(new Color(0x000000)); // Color 1 / black
INDEXED_COLORS.add(new Color(0xFFFFFF)); // Color 2 / white
INDEXED_COLORS.add(new Color(0xFF0000)); // Color 3 / red
INDEXED_COLORS.add(new Color(0x00FF00)); // Color 4 / green
INDEXED_COLORS.add(new Color(0x0000FF)); // Color 5 / blue
INDEXED_COLORS.add(new Color(0xFFFF00)); // Color 6 / yellow
INDEXED_COLORS.add(new Color(0xFF00FF)); // Color 7 / magenta
INDEXED_COLORS.add(new Color(0x00FFFF)); // Color 8 / cyan
INDEXED_COLORS.add(new Color(0x800000)); // Color 9
INDEXED_COLORS.add(new Color(0x008000)); // Color 10
INDEXED_COLORS.add(new Color(0x000080)); // Color 11
INDEXED_COLORS.add(new Color(0x808000)); // Color 12
INDEXED_COLORS.add(new Color(0x800080)); // Color 13
INDEXED_COLORS.add(new Color(0x008080)); // Color 14
INDEXED_COLORS.add(new Color(0xC0C0C0)); // Color 15
INDEXED_COLORS.add(new Color(0x808080)); // Color 16
INDEXED_COLORS.add(new Color(0x9999FF)); // Color 17
INDEXED_COLORS.add(new Color(0x993366)); // Color 18
INDEXED_COLORS.add(new Color(0xFFFFCC)); // Color 19
INDEXED_COLORS.add(new Color(0xCCFFFF)); // Color 20
INDEXED_COLORS.add(new Color(0x660066)); // Color 21
INDEXED_COLORS.add(new Color(0xFF8080)); // Color 22
INDEXED_COLORS.add(new Color(0x0066CC)); // Color 23
INDEXED_COLORS.add(new Color(0xCCCCFF)); // Color 24
INDEXED_COLORS.add(new Color(0x000080)); // Color 25
INDEXED_COLORS.add(new Color(0xFF00FF)); // Color 26
INDEXED_COLORS.add(new Color(0xFFFF00)); // Color 27
INDEXED_COLORS.add(new Color(0x00FFFF)); // Color 28
INDEXED_COLORS.add(new Color(0x800080)); // Color 29
INDEXED_COLORS.add(new Color(0x800000)); // Color 30
INDEXED_COLORS.add(new Color(0x008080)); // Color 31
INDEXED_COLORS.add(new Color(0x0000FF)); // Color 32
INDEXED_COLORS.add(new Color(0x00CCFF)); // Color 33
INDEXED_COLORS.add(new Color(0xCCFFFF)); // Color 34
INDEXED_COLORS.add(new Color(0xCCFFCC)); // Color 35
INDEXED_COLORS.add(new Color(0xFFFF99)); // Color 36
INDEXED_COLORS.add(new Color(0x99CCFF)); // Color 37
INDEXED_COLORS.add(new Color(0xFF99CC)); // Color 38
INDEXED_COLORS.add(new Color(0xCC99FF)); // Color 39
INDEXED_COLORS.add(new Color(0xFFCC99)); // Color 40
INDEXED_COLORS.add(new Color(0x3366FF)); // Color 41
INDEXED_COLORS.add(new Color(0x33CCCC)); // Color 42
INDEXED_COLORS.add(new Color(0x99CC00)); // Color 43
INDEXED_COLORS.add(new Color(0xFFCC00)); // Color 44
INDEXED_COLORS.add(new Color(0xFF9900)); // Color 45
INDEXED_COLORS.add(new Color(0xFF6600)); // Color 46
INDEXED_COLORS.add(new Color(0x666699)); // Color 47
INDEXED_COLORS.add(new Color(0x969696)); // Color 48
INDEXED_COLORS.add(new Color(0x003366)); // Color 49
INDEXED_COLORS.add(new Color(0x339966)); // Color 50
INDEXED_COLORS.add(new Color(0x003300)); // Color 51
INDEXED_COLORS.add(new Color(0x333300)); // Color 52
INDEXED_COLORS.add(new Color(0x993300)); // Color 53
INDEXED_COLORS.add(new Color(0x993366)); // Color 54
INDEXED_COLORS.add(new Color(0x333399)); // Color 55
INDEXED_COLORS.add(new Color(0x333333)); // Color 56
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you create a local list and then do approx this
INDEXED_COLORS = Collections.unmodifiableList(indexedColors)? indexedColors is a suggested name for the local list

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe, you could so something equivalent for the NAMED_COLORS map

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can pretty that up for you, sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like what I have now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it is unnecessary to expose that INDEXED_COLORS list in the first place. It might be useful for future logic, or perhaps some tests, but I understand if you'd prefer to hide that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future proofs the code - someone next year might decide to add a method to expose the list and not read the code and see the list is mutable.
I code mainly in Scala and it's regarded as pretty bad to use mutable collections.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This also feels like the wrong place to be storing color tables in the first place, but I'm at a slight loss as to where to actually place that without creating Yet Another Source Of Truth.


NAMED_COLORS = new TreeMap<>(
String.CASE_INSENSITIVE_ORDER);

// Retain compatibility with original implementation
for (HSSFColor.HSSFColorPredefined color : HSSFColor.HSSFColorPredefined.values()) {
String name = color.name();
short[] rgb = color.getTriplet();
Expand All @@ -71,6 +132,18 @@ public class CellFormatPart {
NAMED_COLORS.put(name.replace("_PERCENT", "%").replace('_',
' '), c);
}

// Add missing color values and replace incorrectly defined standard color
// used in Excel, Google Sheets, etc. The first eight indexed colors
// correspond exactly to named colors.
NAMED_COLORS.put("black", INDEXED_COLORS.get(0));
NAMED_COLORS.put("white", INDEXED_COLORS.get(1));
NAMED_COLORS.put("red", INDEXED_COLORS.get(2));
NAMED_COLORS.put("green", INDEXED_COLORS.get(3));
NAMED_COLORS.put("blue", INDEXED_COLORS.get(4));
NAMED_COLORS.put("yellow", INDEXED_COLORS.get(5));
NAMED_COLORS.put("magenta", INDEXED_COLORS.get(6));
NAMED_COLORS.put("cyan", INDEXED_COLORS.get(7));
}

/** Pattern for the color part of a cell format part. */
Expand Down Expand Up @@ -110,8 +183,16 @@ public class CellFormatPart {
// A currency symbol / string, in a specific locale
String currency = "(\\[\\$.{0,3}(-[0-9a-f]{3,4})?])";

String color =
"\\[(black|blue|cyan|green|magenta|red|white|yellow|color [0-9]+)]";
// Build the color code matching expression. We should match any named color
// in the set as well as a string in the form of "Color 8" or "Color 15".
String color = "\\[(";
for (String key : NAMED_COLORS.keySet()) {
// Escape special characters in the color name
color += key.replaceAll("([^a-zA-Z0-9])", "\\\\$1") + "|";
}
// Match the indexed color table (accept both e.g. COLOR2 and COLOR 2)
// Both formats are accepted as input in other products
color += "color\\s*[0-9]+)\\]";

// A number specification
// Note: careful that in something like ##, that the trailing comma is not caught up in the integer part
Expand Down Expand Up @@ -159,6 +240,14 @@ public class CellFormatPart {
CONDITION_OPERATOR_GROUP = findGroup(FORMAT_PAT, "[>=1]@", ">=");
CONDITION_VALUE_GROUP = findGroup(FORMAT_PAT, "[>=1]@", "1");
SPECIFICATION_GROUP = findGroup(FORMAT_PAT, "[Blue][>1]\\a ?", "\\a ?");

// Once patterns have been compiled, add indexed colors to
// NAMED_COLORS so they can be easily picked up by getColor().
for (int i = 0; i < INDEXED_COLORS.size(); ++i) {
NAMED_COLORS.put("color" + (i + 1), INDEXED_COLORS.get(i));
// Also support space between "color" and number.
NAMED_COLORS.put("color " + (i + 1), INDEXED_COLORS.get(i));
}
}

interface PartHandler {
Expand Down Expand Up @@ -250,12 +339,23 @@ private static int findGroup(Pattern pat, String str, String marker) {
* @return The color specification or {@code null}.
*/
private static Color getColor(Matcher m) {
String cdesc = m.group(COLOR_GROUP);
if (cdesc == null || cdesc.isEmpty())
return getColor(m.group(COLOR_GROUP));
}

/**
* Get the Color object matching a color name, or {@code null} if the
* color name is not recognized.
*
* @param cname Color name, such as "red" or "Color 15"
*
* @return a Color object or {@code null}.
*/
static Color getColor(String cname) {
if (cname == null || cname.isEmpty())
return null;
Color c = NAMED_COLORS.get(cdesc);
Color c = NAMED_COLORS.get(cname);
if (c == null) {
LOG.warn("Unknown color: {}", quote(cdesc));
LOG.warn("Unknown color: {}", quote(cname));
}
return c;
}
Expand Down
126 changes: 123 additions & 3 deletions poi/src/test/java/org/apache/poi/ss/format/TestCellFormat.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.awt.Color;
import java.io.IOException;
import java.text.ParseException;
import java.text.SimpleDateFormat;
Expand All @@ -33,6 +34,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.hssf.util.HSSFColor;
import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.CellType;
import org.apache.poi.ss.usermodel.DateUtil;
import org.apache.poi.ss.usermodel.Row;
import org.apache.poi.ss.usermodel.Sheet;
Expand Down Expand Up @@ -1041,11 +1043,129 @@ void testBug62865() {

@Test
void testNamedColors() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the tests - quick one, can the testNamedColors be retained?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I don't see why not. Gimme a few minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed the testNamedColorsExist test to testNamedColors; it does roughly the same thing the previous test did.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...unless the line assertTrue(CellFormatPart.NAMED_COLORS.size() >= HSSFColor.HSSFColorPredefined.values().length); did something very significant which I just didn't understand.

assertTrue(CellFormatPart.NAMED_COLORS.size() >= HSSFColor.HSSFColorPredefined.values().length);
Stream.of("GREEN", "Green", "RED", "Red", "BLUE", "Blue", "YELLOW", "Yellow")
.map(CellFormatPart.NAMED_COLORS::get)
// Make sure we have all standard named colors defined
// and are returned as non-null regardless of case
Stream.of("black", "white", "red", "green", "blue", "yellow", "magenta", "cyan",
"Black", "White", "Red", "Green", "Blue", "Yellow", "Magenta", "Cyan",
"BLACK", "WHITE", "RED", "GREEN", "BLUE", "YELLOW", "MAGENTA", "CYAN")
.map(CellFormatPart::getColor)
.forEach(Assertions::assertNotNull);
}

@Test
void testIndexedColorsExist() {
// Make sure the standard indexed colors are returned correctly and regardless of case
for (int i = 0; i < 56; ++i) {
assertNotNull(CellFormatPart.getColor("Color " + (i + 1)));
assertNotNull(CellFormatPart.getColor("COLOR" + (i + 1)));
assertNotNull(CellFormatPart.getColor("color" + (i + 1)));
}
}

@Test
void verifyNamedColors() {
assertEquals(CellFormatPart.getColor("Black"), new Color(0x000000));
assertEquals(CellFormatPart.getColor("white"), new Color(0xFFFFFF));
assertEquals(CellFormatPart.getColor("RED"), new Color(0xFF0000));
assertEquals(CellFormatPart.getColor("Green"), new Color(0x00FF00));
assertEquals(CellFormatPart.getColor("blue"), new Color(0x0000FF));
assertEquals(CellFormatPart.getColor("YELLOW"), new Color(0xFFFF00));
assertEquals(CellFormatPart.getColor("Magenta"), new Color(0xFF00FF));
assertEquals(CellFormatPart.getColor("cyan"), new Color(0x00FFFF));
}

@Test
void verifyIndexedColors() {
assertEquals(CellFormatPart.getColor("Color1"), CellFormatPart.getColor("black"));
assertEquals(CellFormatPart.getColor("color2"), CellFormatPart.getColor("white"));
assertEquals(CellFormatPart.getColor("Color3"), CellFormatPart.getColor("red"));
assertEquals(CellFormatPart.getColor("color4"), CellFormatPart.getColor("green"));
assertEquals(CellFormatPart.getColor("Color5"), CellFormatPart.getColor("blue"));
assertEquals(CellFormatPart.getColor("color6"), CellFormatPart.getColor("yellow"));
assertEquals(CellFormatPart.getColor("Color7"), CellFormatPart.getColor("magenta"));
assertEquals(CellFormatPart.getColor("color8"), CellFormatPart.getColor("cyan"));
assertEquals(CellFormatPart.getColor("Color9"), new Color(0x800000));
assertEquals(CellFormatPart.getColor("color10"), new Color(0x008000));
assertEquals(CellFormatPart.getColor("Color11"), new Color(0x000080));
assertEquals(CellFormatPart.getColor("color12"), new Color(0x808000));
assertEquals(CellFormatPart.getColor("Color13"), new Color(0x800080));
assertEquals(CellFormatPart.getColor("color14"), new Color(0x008080));
assertEquals(CellFormatPart.getColor("Color15"), new Color(0xC0C0C0));
assertEquals(CellFormatPart.getColor("color16"), new Color(0x808080));
assertEquals(CellFormatPart.getColor("Color17"), new Color(0x9999FF));
assertEquals(CellFormatPart.getColor("COLOR18"), new Color(0x993366));
assertEquals(CellFormatPart.getColor("Color19"), new Color(0xFFFFCC));
assertEquals(CellFormatPart.getColor("color20"), new Color(0xCCFFFF));
assertEquals(CellFormatPart.getColor("Color21"), new Color(0x660066));
assertEquals(CellFormatPart.getColor("COLOR22"), new Color(0xFF8080));
assertEquals(CellFormatPart.getColor("Color23"), new Color(0x0066CC));
assertEquals(CellFormatPart.getColor("color24"), new Color(0xCCCCFF));
assertEquals(CellFormatPart.getColor("Color25"), new Color(0x000080));
assertEquals(CellFormatPart.getColor("color26"), new Color(0xFF00FF));
assertEquals(CellFormatPart.getColor("Color27"), new Color(0xFFFF00));
assertEquals(CellFormatPart.getColor("COLOR28"), new Color(0x00FFFF));
assertEquals(CellFormatPart.getColor("Color29"), new Color(0x800080));
assertEquals(CellFormatPart.getColor("color30"), new Color(0x800000));
assertEquals(CellFormatPart.getColor("Color31"), new Color(0x008080));
assertEquals(CellFormatPart.getColor("Color32"), new Color(0x0000FF));
assertEquals(CellFormatPart.getColor("Color33"), new Color(0x00CCFF));
assertEquals(CellFormatPart.getColor("Color34"), new Color(0xCCFFFF));
assertEquals(CellFormatPart.getColor("Color35"), new Color(0xCCFFCC));
assertEquals(CellFormatPart.getColor("Color36"), new Color(0xFFFF99));
assertEquals(CellFormatPart.getColor("Color37"), new Color(0x99CCFF));
assertEquals(CellFormatPart.getColor("Color38"), new Color(0xFF99CC));
assertEquals(CellFormatPart.getColor("Color39"), new Color(0xCC99FF));
assertEquals(CellFormatPart.getColor("Color40"), new Color(0xFFCC99));
assertEquals(CellFormatPart.getColor("Color41"), new Color(0x3366FF));
assertEquals(CellFormatPart.getColor("Color42"), new Color(0x33CCCC));
assertEquals(CellFormatPart.getColor("Color43"), new Color(0x99CC00));
assertEquals(CellFormatPart.getColor("Color44"), new Color(0xFFCC00));
assertEquals(CellFormatPart.getColor("Color45"), new Color(0xFF9900));
assertEquals(CellFormatPart.getColor("Color46"), new Color(0xFF6600));
assertEquals(CellFormatPart.getColor("Color47"), new Color(0x666699));
assertEquals(CellFormatPart.getColor("Color48"), new Color(0x969696));
assertEquals(CellFormatPart.getColor("Color49"), new Color(0x003366));
assertEquals(CellFormatPart.getColor("Color50"), new Color(0x339966));
assertEquals(CellFormatPart.getColor("Color51"), new Color(0x003300));
assertEquals(CellFormatPart.getColor("Color52"), new Color(0x333300));
assertEquals(CellFormatPart.getColor("Color53"), new Color(0x993300));
assertEquals(CellFormatPart.getColor("Color54"), new Color(0x993366));
assertEquals(CellFormatPart.getColor("Color55"), new Color(0x333399));
assertEquals(CellFormatPart.getColor("Color56"), new Color(0x333333));
}

@Test
void testColorsInWorkbook() throws IOException {
// Create a workbook, row and cell to test with
try (Workbook wb = new HSSFWorkbook()) {
Sheet sheet = wb.createSheet();
Row row = sheet.createRow(0);
Cell cell = row.createCell(0);
CellFormatResult result;
CellFormat cf = CellFormat.getInstance(
"[GREEN]#,##0.0;[RED]\\(#,##0.0\\);[COLOR22]\"===\";[COLOR 8]\\\"@\\\"");

cell.setCellValue(100.0);
result = cf.apply(cell);
assertEquals("100.0", result.text);
assertEquals(result.textColor, CellFormatPart.getColor("color 4"));

cell.setCellValue(-50.0);
result = cf.apply(cell);
assertEquals("(50.0)", result.text);
assertEquals(result.textColor, CellFormatPart.getColor("red"));

cell.setCellValue("foo");
result = cf.apply(cell);
assertEquals("\"foo\"", result.text);
assertEquals(result.textColor, CellFormatPart.getColor("cyan"));

cell.setCellValue(0.0);
result = cf.apply(cell);
assertEquals("===", result.text);
assertEquals(result.textColor, CellFormatPart.getColor("color 22"));
}
}

@Test
void testElapsedSecondsRound() {
Expand Down