From 708febacbbd9fa6abe09ae7da591ee85c3bccffd Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 23 Feb 2025 20:52:34 -0500 Subject: [PATCH] Use Jenkins theme CSS color variables for unset states The original support for negative/inverse text uses a combination off `currentColor` and a bright-level-7 value. When using a default Jenkins theme, these color definitions work as expected. However, clients using the dark theme may not be able to read the text since both the text and background are a bright color. Modern Jenkins themes provide CSS color variables. Instead of trying to rely on `currentColor`, use the default text color (`var(--text-color)`) and the default background color (`var(--background)`). This should allow negative/inverse text to render appropriately no matter what theme mode is used. Signed-off-by: James Knight --- .../ansicolor/AnsiHtmlOutputStream.java | 54 ++--------------- .../ansicolor/AnsiHtmlOutputStreamTest.java | 59 ++++++++----------- 2 files changed, 32 insertions(+), 81 deletions(-) diff --git a/src/main/java/hudson/plugins/ansicolor/AnsiHtmlOutputStream.java b/src/main/java/hudson/plugins/ansicolor/AnsiHtmlOutputStream.java index 153bde4..8d46cac 100644 --- a/src/main/java/hudson/plugins/ansicolor/AnsiHtmlOutputStream.java +++ b/src/main/java/hudson/plugins/ansicolor/AnsiHtmlOutputStream.java @@ -313,27 +313,8 @@ private String getDefaultForegroundColor() { Integer defaultFgIndex = colorMap.getDefaultForeground(); if (defaultFgIndex != null) color = colorMap.getNormal(defaultFgIndex); if (color == null) { - // with no default foreground set, we need to guess about (currently happened in xterm and css themes) - // possible approaches are: - // • hardcoded black "#000000" - // • colorMap.getNormal(0) - // • "currentColor" → see also: http://stackoverflow.com/a/42586457/2880699 - // but note, that this approach only works, if all are currently closed - - // Note that in my default szenario with, the default text color is #333333. Tested with: - // • Jenkins 1.6.5.3, AnsiColor plugin 0.4.3, xterm scheme - // • Firefox 51.0.1, Kubuntu 16.04 - // • Chromium 56.0.2924.76, Kubuntu 16.04 - // • Firefox 51.0.1, Windows XP - - // So I finally decide for the "currentColor" approach, because: - // • It gives the best results for negative / inverse text (what is the major reason for implementing this function). - // • It looks also the best alternative, if e.g. someone customizes Jenkins colors. - // • Finally, the clause "only works, if all are currently closed" is fulfilled for the negative / inverse case - - // color = "#000000"; // hardcoded black - // color = colorMap.getNormal(0); // hardcoded index 0 - color = "currentColor"; // see http://stackoverflow.com/a/42586457/2880699 + // with no default foreground set, use the default theme text color + color = "var(--text-color)"; } return color; } @@ -343,16 +324,8 @@ private String getDefaultBackgroundColor() { Integer defaultBgIndex = colorMap.getDefaultBackground(); if (defaultBgIndex != null) color = colorMap.getNormal(defaultBgIndex); if (color == null) { - // with no default foreground set, we need to guess about (currently happened in xterm and css themes) - // possible approaches are: - // • hardcoded white "#FFFFFF" - // • colorMap.getNormal(7) - // • colorMap.getBright(7) - // I finally decide for colorMap.getBright(7). - - //color "#FFFFFF"; // hardcoded white - //color = colorMap.getNormal(7); // hardcoded normal index 7 - color = colorMap.getBright(7); // hardcoded bright index 7 + // with no default foreground set, use the default theme background color + color = "var(--background)"; } return color; } @@ -362,22 +335,9 @@ private void setForegroundColor(String color) { AnsiAttrType attrType = !swapColors ? AnsiAttrType.FG : AnsiAttrType.BG; String attrName = !swapColors ? "color" : "background-color"; if (color == null && swapColors) color = getDefaultForegroundColor(); - boolean restorebg = false; - if (swapColors && color.equals("currentColor")) { - // need also to temporarily unwind textcolor, to having correct access to the "currentColor" value - closeTagOfType(AnsiAttrType.FGBG); - restorebg = true; - } else { - closeTagOfType(attrType); - } + closeTagOfType(attrType); if (color != null) openTag(new AnsiAttributeElement(attrType, "span", "style=\"" + attrName + ": " + color + ";\"")); - if (restorebg) { - // Because of the "currentColor" trick, we always need to use two seperate tags for this case. - String bg = currentBackgroundColor; - if (bg == null) bg = getDefaultBackgroundColor(); - openTag(new AnsiAttributeElement(AnsiAttrType.FG, "span", "style=\"color: " + bg + ";\"")); - } currentForegroundColor = color; } @@ -475,9 +435,7 @@ else switch (attribute) { bg = tmp; } closeTagOfType(AnsiAttrType.FGBG); - if (fg != null && bg != null && !bg.equals("currentColor")) { - // In case of "currentColor" trick, we need to use two seperate tags. - // But if not, then we can use one single tag to set both background and foreground color. + if (fg != null && bg != null) { openTag(new AnsiAttributeElement(AnsiAttrType.FGBG, "span", "style=\"background-color: " + bg + "; color: " + fg + ";\"")); } else { if (bg != null) openTag(new AnsiAttributeElement(AnsiAttrType.BG, "span", "style=\"background-color: " + bg + ";\"")); diff --git a/src/test/java/hudson/plugins/ansicolor/AnsiHtmlOutputStreamTest.java b/src/test/java/hudson/plugins/ansicolor/AnsiHtmlOutputStreamTest.java index 32b70da..30bb47b 100644 --- a/src/test/java/hudson/plugins/ansicolor/AnsiHtmlOutputStreamTest.java +++ b/src/test/java/hudson/plugins/ansicolor/AnsiHtmlOutputStreamTest.java @@ -114,34 +114,33 @@ void testNegative() throws IOException { // simple tests assertThatAnnotateIs( "\033[7mon\033[moff", - "onoff"); + "onoff"); assertThatAnnotateIs( "\033[7mon\033[27moff", - "onoff"); + "onoff"); assertThatAnnotateIs( "\033[33;7mon\033[27moff", "" + // unnecessary tag, could be removed … - "on" + + "on" + "off"); assertThatAnnotateIs( "\033[7;33mon\033[27moff", - "" + // unnecessary tag, could be removed … - "on" + // could be optimized to be a single tag + "" + + "on" + "off"); assertThatAnnotateIs( "\033[41;7mon\033[27moff", "" + // unnecessary tag, could be removed … - "on" + + "on" + "off"); assertThatAnnotateIs( "\033[7;41mon\033[27moff", - "" + - "" + // unnecessary tag, could be removed … + "" + "on" + "" + "off"); @@ -154,9 +153,8 @@ void testNegative() throws IOException { assertThatAnnotateIs( "\033[7;33;41mon\033[27moff", - "" + // unnecessary tag, could be removed … - "" + // unnecessary tag, could be removed … - "on" + // could be optimized to be a single tag + "" + + "on" + // could be optimized to be a single tag "off"); @@ -164,61 +162,56 @@ void testNegative() throws IOException { assertThatAnnotateIs( "\033[33;7mon\033[39mdefault", "" + // unnecessary tag, could be removed … - "on" + - "default"); + "on" + + "default"); assertThatAnnotateIs( "\033[7;33mon\033[39mdefault", - "" + // unnecessary tag, could be removed … - "on" + // could be optimized to be a single tag - "default"); + "" + + "on" + + "default"); assertThatAnnotateIs( "\033[41;7mon\033[49mdefault", "" + // unnecessary tag, could be removed … - "" + - "on" + - "default" + + "on" + + "default" + ""); assertThatAnnotateIs( "\033[7;41mon\033[49mdefault", - "" + - "" + // unnecessary tag, could be removed … + "" + "on" + - "default" + + "default" + ""); assertThatAnnotateIs( "\033[33;41;7mon\033[39mdefault", "" + // unnecessary tag, could be removed … - "on" + - "default"); + "on" + + "default"); assertThatAnnotateIs( "\033[7;33;41mon\033[39mdefault", - "" + // unnecessary tag, could be removed … - "" + // unnecessary tag, could be removed … + "" + "on" + - "default"); + "default"); assertThatAnnotateIs( "\033[33;41;7mon\033[49mdefault", "" + // unnecessary tag, could be removed … "" + "on" + - "default" + + "default" + ""); assertThatAnnotateIs( "\033[7;33;41mon\033[49mdefault", - "" + // unnecessary tag, could be removed … - "" + // unnecessary tag, could be removed … + "" + "" + "on" + - "default" + - ""); - + "default" + + ""); // simple tests with dark theme, as there has been default foreground / background colors defined (in contrast to xterm scheme) assertThatAnnotateIs(AnsiColorMap.VGA,