From 6d161ce1d498f522520c9dda6e4255b755fdcdbe Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Tue, 5 Nov 2024 08:54:35 -0800 Subject: [PATCH] Polish ANSI formatting and underlying code (#779) - change line numbers from blue to faint to improve legibility on dark backgrounds - use EnumSet throughout - move all ANSI classes into package core.util (no need to split them over util and runtime) - rename AnsiCodingStringBuilder to AnsiStringBuilder --- .../pkl/core/runtime/StackTraceRenderer.java | 14 ++++---- .../java/org/pkl/core/runtime/TestRunner.java | 13 +++---- .../pkl/core/runtime/VmExceptionRenderer.java | 10 +++--- .../core/stdlib/test/report/SimpleReport.java | 16 ++++----- .../AnsiStringBuilder.java} | 36 +++++++++---------- .../java/org/pkl/core/util/AnsiTheme.java | 4 +-- .../java/org/pkl/core/util/StringUtils.java | 3 +- .../core/runtime/StackTraceRendererTest.kt | 3 +- .../AnsiStringBuilderTest.kt} | 16 ++++----- 9 files changed, 57 insertions(+), 58 deletions(-) rename pkl-core/src/main/java/org/pkl/core/{runtime/AnsiCodingStringBuilder.java => util/AnsiStringBuilder.java} (86%) rename pkl-core/src/test/kotlin/org/pkl/core/{runtime/AnsiCodingStringBuilderTest.kt => util/AnsiStringBuilderTest.kt} (82%) diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java b/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java index fae805b1..d3112e23 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; import java.util.function.Function; import org.pkl.core.StackFrame; +import org.pkl.core.util.AnsiStringBuilder; import org.pkl.core.util.AnsiTheme; import org.pkl.core.util.Nullable; @@ -29,7 +30,7 @@ public final class StackTraceRenderer { this.frameTransformer = frameTransformer; } - public void render(List frames, @Nullable String hint, AnsiCodingStringBuilder out) { + public void render(List frames, @Nullable String hint, AnsiStringBuilder out) { var compressed = compressFrames(frames); doRender(compressed, hint, out, "", true); } @@ -38,7 +39,7 @@ public final class StackTraceRenderer { void doRender( List frames, @Nullable String hint, - AnsiCodingStringBuilder out, + AnsiStringBuilder out, String leftMargin, boolean isFirstElement) { for (var frame : frames) { @@ -76,13 +77,13 @@ public final class StackTraceRenderer { } } - private void renderFrame(StackFrame frame, AnsiCodingStringBuilder out, String leftMargin) { + private void renderFrame(StackFrame frame, AnsiStringBuilder out, String leftMargin) { var transformed = frameTransformer.apply(frame); renderSourceLine(transformed, out, leftMargin); renderSourceLocation(transformed, out, leftMargin); } - private void renderHint(@Nullable String hint, AnsiCodingStringBuilder out, String leftMargin) { + private void renderHint(@Nullable String hint, AnsiStringBuilder out, String leftMargin) { if (hint == null || hint.isEmpty()) return; out.append('\n') @@ -91,7 +92,7 @@ public final class StackTraceRenderer { .append('\n'); } - private void renderSourceLine(StackFrame frame, AnsiCodingStringBuilder out, String leftMargin) { + private void renderSourceLine(StackFrame frame, AnsiStringBuilder out, String leftMargin) { var originalSourceLine = frame.getSourceLines().get(0); var leadingWhitespace = VmUtils.countLeadingWhitespace(originalSourceLine); var sourceLine = originalSourceLine.strip(); @@ -112,8 +113,7 @@ public final class StackTraceRenderer { .append('\n'); } - private void renderSourceLocation( - StackFrame frame, AnsiCodingStringBuilder out, String leftMargin) { + private void renderSourceLocation(StackFrame frame, AnsiStringBuilder out, String leftMargin) { out.append(AnsiTheme.STACK_TRACE_MARGIN, leftMargin) .append( AnsiTheme.STACK_FRAME, diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/TestRunner.java b/pkl-core/src/main/java/org/pkl/core/runtime/TestRunner.java index bfc7d529..5364b77c 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/TestRunner.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/TestRunner.java @@ -34,6 +34,7 @@ import org.pkl.core.ast.member.ObjectMember; import org.pkl.core.module.ModuleKeys; import org.pkl.core.stdlib.PklConverter; import org.pkl.core.stdlib.base.PcfRenderer; +import org.pkl.core.util.AnsiStringBuilder; import org.pkl.core.util.AnsiTheme; import org.pkl.core.util.EconomicMaps; import org.pkl.core.util.MutableBoolean; @@ -395,7 +396,7 @@ public final class TestRunner { } private Failure factFailure(SourceSection sourceSection, String location) { - var sb = new AnsiCodingStringBuilder(useColor); + var sb = new AnsiStringBuilder(useColor); sb.append(AnsiTheme.TEST_FACT_SOURCE, sourceSection.getCharacters().toString()).append(" "); appendLocation(sb, location); return new Failure("Fact Failure", sb.toString()); @@ -403,7 +404,7 @@ public final class TestRunner { private Failure exampleLengthMismatchFailure( String location, String property, int expectedLength, int actualLength) { - var sb = new AnsiCodingStringBuilder(useColor); + var sb = new AnsiStringBuilder(useColor); appendLocation(sb, location); sb.append('\n') @@ -433,7 +434,7 @@ public final class TestRunner { missingIn = "actual"; } - var sb = new AnsiCodingStringBuilder(useColor); + var sb = new AnsiStringBuilder(useColor); appendLocation(sb, location); sb.append('\n') @@ -457,7 +458,7 @@ public final class TestRunner { String actualLocation, String actualValue, int exampleNumber) { - var sb = new AnsiCodingStringBuilder(useColor); + var sb = new AnsiStringBuilder(useColor); sb.append(AnsiTheme.TEST_NAME, "#" + exampleNumber + ": "); sb.append( AnsiTheme.TEST_FAILURE_MESSAGE, @@ -475,14 +476,14 @@ public final class TestRunner { return new Failure("Example Failure", sb.toString()); } - private void appendLocation(AnsiCodingStringBuilder stringBuilder, String location) { + private void appendLocation(AnsiStringBuilder stringBuilder, String location) { stringBuilder.append( AnsiTheme.STACK_FRAME, () -> stringBuilder.append("(").appendUntrusted(location).append(")")); } private Failure writtenExampleOutputFailure(String testName, String location) { - var sb = new AnsiCodingStringBuilder(useColor); + var sb = new AnsiStringBuilder(useColor); appendLocation(sb, location); sb.append(AnsiTheme.TEST_FAILURE_MESSAGE, "\nWrote expected output for test ").append(testName); return new Failure("Example Output Written", sb.toString()); diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java index a61db746..0e72a5cd 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java @@ -20,6 +20,7 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.stream.Collectors; import org.pkl.core.Release; +import org.pkl.core.util.AnsiStringBuilder; import org.pkl.core.util.AnsiTheme; import org.pkl.core.util.ErrorMessages; import org.pkl.core.util.Nullable; @@ -39,12 +40,12 @@ public final class VmExceptionRenderer { @TruffleBoundary public String render(VmException exception) { - var formatter = new AnsiCodingStringBuilder(color); + var formatter = new AnsiStringBuilder(color); render(exception, formatter); return formatter.toString(); } - private void render(VmException exception, AnsiCodingStringBuilder out) { + private void render(VmException exception, AnsiStringBuilder out) { if (exception instanceof VmBugException bugException) { renderBugException(bugException, out); } else { @@ -52,7 +53,7 @@ public final class VmExceptionRenderer { } } - private void renderBugException(VmBugException exception, AnsiCodingStringBuilder out) { + private void renderBugException(VmBugException exception, AnsiStringBuilder out) { // if a cause exists, it's more useful to report just that var exceptionToReport = exception.getCause() != null ? exception.getCause() : exception; var exceptionUrl = URLEncoder.encode(exceptionToReport.toString(), StandardCharsets.UTF_8); @@ -75,8 +76,7 @@ public final class VmExceptionRenderer { exceptionToReport.printStackTrace(out.toPrintWriter()); } - private void renderException( - VmException exception, AnsiCodingStringBuilder out, boolean withHeader) { + private void renderException(VmException exception, AnsiStringBuilder out, boolean withHeader) { String message; var hint = exception.getHint(); if (exception.isExternalMessage()) { diff --git a/pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java b/pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java index 27662299..60bff1a1 100644 --- a/pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java +++ b/pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java @@ -22,8 +22,8 @@ import java.util.stream.Collectors; import org.pkl.core.TestResults; import org.pkl.core.TestResults.TestResult; import org.pkl.core.TestResults.TestSectionResults; -import org.pkl.core.runtime.AnsiCodingStringBuilder; -import org.pkl.core.runtime.AnsiCodingStringBuilder.AnsiCode; +import org.pkl.core.util.AnsiStringBuilder; +import org.pkl.core.util.AnsiStringBuilder.AnsiCode; import org.pkl.core.util.AnsiTheme; import org.pkl.core.util.StringUtils; @@ -40,7 +40,7 @@ public final class SimpleReport implements TestReport { @Override public void report(TestResults results, Writer writer) throws IOException { - var builder = new AnsiCodingStringBuilder(useColor); + var builder = new AnsiStringBuilder(useColor); builder.append("module ").append(results.moduleName()).append('\n'); @@ -75,7 +75,7 @@ public final class SimpleReport implements TestReport { totalAsserts += testResults.totalAsserts(); totalFailedAsserts += testResults.totalAssertsFailed(); } - var builder = new AnsiCodingStringBuilder(useColor); + var builder = new AnsiStringBuilder(useColor); if (isFailed && isExampleWrittenFailure) { builder.append(totalFailedTests).append(" examples written"); } else { @@ -87,7 +87,7 @@ public final class SimpleReport implements TestReport { writer.append(builder.toString()); } - private void reportResults(TestSectionResults section, AnsiCodingStringBuilder builder) { + private void reportResults(TestSectionResults section, AnsiStringBuilder builder) { if (!section.results().isEmpty()) { builder.append(" ").append(section.name()).append('\n'); StringUtils.joinToStringBuilder( @@ -96,7 +96,7 @@ public final class SimpleReport implements TestReport { } } - private void reportResult(TestResult result, AnsiCodingStringBuilder builder) { + private void reportResult(TestResult result, AnsiStringBuilder builder) { builder.append(" "); if (result.isExampleWritten()) { @@ -125,7 +125,7 @@ public final class SimpleReport implements TestReport { } } - private static void appendPadded(AnsiCodingStringBuilder builder, String lines, String padding) { + private static void appendPadded(AnsiStringBuilder builder, String lines, String padding) { StringUtils.joinToStringBuilder( builder, lines.lines().collect(Collectors.toList()), @@ -136,7 +136,7 @@ public final class SimpleReport implements TestReport { } private void makeStatsLine( - AnsiCodingStringBuilder sb, String kind, int total, int failed, boolean isFailed) { + AnsiStringBuilder sb, String kind, int total, int failed, boolean isFailed) { var passed = total - failed; var passRate = total > 0 ? 100.0 * passed / total : 0.0; diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/AnsiCodingStringBuilder.java b/pkl-core/src/main/java/org/pkl/core/util/AnsiStringBuilder.java similarity index 86% rename from pkl-core/src/main/java/org/pkl/core/runtime/AnsiCodingStringBuilder.java rename to pkl-core/src/main/java/org/pkl/core/util/AnsiStringBuilder.java index 847cff32..8649c229 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/AnsiCodingStringBuilder.java +++ b/pkl-core/src/main/java/org/pkl/core/util/AnsiStringBuilder.java @@ -13,31 +13,29 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.pkl.core.runtime; +package org.pkl.core.util; import java.io.PrintWriter; -import java.util.Collections; import java.util.EnumSet; import java.util.Set; -import org.pkl.core.util.StringBuilderWriter; @SuppressWarnings("DuplicatedCode") -public final class AnsiCodingStringBuilder { +public final class AnsiStringBuilder { private final StringBuilder builder = new StringBuilder(); private final boolean usingColor; /** The set of ansi codes currently applied. */ - private Set currentCodes = Collections.emptySet(); + private Set currentCodes = EnumSet.noneOf(AnsiCode.class); /** The set of ansi codes intended to be applied the next time text is written. */ - private Set declaredCodes = Collections.emptySet(); + private Set declaredCodes = EnumSet.noneOf(AnsiCode.class); - public AnsiCodingStringBuilder(boolean usingColor) { + public AnsiStringBuilder(boolean usingColor) { this.usingColor = usingColor; } /** Append {@code value} to the string, ensuring it is formatted with {@code codes}. */ - public AnsiCodingStringBuilder append(Set codes, String value) { + public AnsiStringBuilder append(Set codes, String value) { if (!usingColor) { builder.append(value); return this; @@ -51,7 +49,7 @@ public final class AnsiCodingStringBuilder { } /** Append {@code value} to the string, ensuring it is formatted with {@code codes}. */ - public AnsiCodingStringBuilder append(AnsiCode code, int value) { + public AnsiStringBuilder append(AnsiCode code, int value) { if (!usingColor) { builder.append(value); return this; @@ -65,7 +63,7 @@ public final class AnsiCodingStringBuilder { } /** Append {@code value} to the string, ensuring it is formatted with {@code codes}. */ - public AnsiCodingStringBuilder append(AnsiCode code, String value) { + public AnsiStringBuilder append(AnsiCode code, String value) { if (!usingColor) { builder.append(value); return this; @@ -97,7 +95,7 @@ public final class AnsiCodingStringBuilder { * } * */ - public AnsiCodingStringBuilder append(AnsiCode code, Runnable runnable) { + public AnsiStringBuilder append(AnsiCode code, Runnable runnable) { if (!usingColor) { runnable.run(); return this; @@ -115,7 +113,7 @@ public final class AnsiCodingStringBuilder { * *

Always add a reset and re-apply all colors after appending the string. */ - public AnsiCodingStringBuilder appendUntrusted(String value) { + public AnsiStringBuilder appendUntrusted(String value) { appendCodes(); builder.append(value); if (usingColor) { @@ -131,7 +129,7 @@ public final class AnsiCodingStringBuilder { *

If called within {@link #append(AnsiCode, Runnable)}, applies any styles in the current * context. */ - public AnsiCodingStringBuilder append(String value) { + public AnsiStringBuilder append(String value) { appendCodes(); builder.append(value); return this; @@ -143,7 +141,7 @@ public final class AnsiCodingStringBuilder { *

If called within {@link #append(AnsiCode, Runnable)}, applies any styles in the current * context. */ - public AnsiCodingStringBuilder append(char value) { + public AnsiStringBuilder append(char value) { appendCodes(); builder.append(value); return this; @@ -155,7 +153,7 @@ public final class AnsiCodingStringBuilder { *

If called within {@link #append(AnsiCode, Runnable)}, applies any styles in the current * context. */ - public AnsiCodingStringBuilder append(int value) { + public AnsiStringBuilder append(int value) { appendCodes(); builder.append(value); return this; @@ -167,15 +165,15 @@ public final class AnsiCodingStringBuilder { *

If called within {@link #append(AnsiCode, Runnable)}, applies any styles in the current * context. */ - public AnsiCodingStringBuilder append(Object value) { + public AnsiStringBuilder append(Object value) { appendCodes(); builder.append(value); return this; } /** Returns a fresh instance of this string builder. */ - public AnsiCodingStringBuilder newInstance() { - return new AnsiCodingStringBuilder(usingColor); + public AnsiStringBuilder newInstance() { + return new AnsiStringBuilder(usingColor); } public PrintWriter toPrintWriter() { @@ -220,7 +218,7 @@ public final class AnsiCodingStringBuilder { private void reset() { if (!usingColor || currentCodes.isEmpty()) return; doReset(); - currentCodes = Collections.emptySet(); + currentCodes = EnumSet.noneOf(AnsiCode.class); } private void doReset() { diff --git a/pkl-core/src/main/java/org/pkl/core/util/AnsiTheme.java b/pkl-core/src/main/java/org/pkl/core/util/AnsiTheme.java index b48b4fda..3dc0b19e 100644 --- a/pkl-core/src/main/java/org/pkl/core/util/AnsiTheme.java +++ b/pkl-core/src/main/java/org/pkl/core/util/AnsiTheme.java @@ -17,7 +17,7 @@ package org.pkl.core.util; import java.util.EnumSet; import java.util.Set; -import org.pkl.core.runtime.AnsiCodingStringBuilder.AnsiCode; +import org.pkl.core.util.AnsiStringBuilder.AnsiCode; public final class AnsiTheme { private AnsiTheme() {} @@ -28,7 +28,7 @@ public final class AnsiTheme { public static final AnsiCode STACK_FRAME = AnsiCode.FAINT; public static final AnsiCode STACK_TRACE_MARGIN = AnsiCode.YELLOW; - public static final AnsiCode STACK_TRACE_LINE_NUMBER = AnsiCode.BLUE; + public static final AnsiCode STACK_TRACE_LINE_NUMBER = AnsiCode.FAINT; public static final AnsiCode STACK_TRACE_LOOP_COUNT = AnsiCode.MAGENTA; public static final AnsiCode STACK_TRACE_CARET = AnsiCode.RED; diff --git a/pkl-core/src/main/java/org/pkl/core/util/StringUtils.java b/pkl-core/src/main/java/org/pkl/core/util/StringUtils.java index 57f70d35..744a7c77 100644 --- a/pkl-core/src/main/java/org/pkl/core/util/StringUtils.java +++ b/pkl-core/src/main/java/org/pkl/core/util/StringUtils.java @@ -16,7 +16,6 @@ package org.pkl.core.util; import java.util.function.Consumer; -import org.pkl.core.runtime.AnsiCodingStringBuilder; // Some code in this class was taken from the following Google Guava classes: // * com.google.common.base.CharMatcher @@ -87,7 +86,7 @@ public final class StringUtils { } public static void joinToStringBuilder( - AnsiCodingStringBuilder builder, Iterable coll, String delimiter, Consumer eachFn) { + AnsiStringBuilder builder, Iterable coll, String delimiter, Consumer eachFn) { var i = 0; for (var v : coll) { if (i != 0) { diff --git a/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt index 43d3ad13..72d03e9a 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt @@ -20,6 +20,7 @@ import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.pkl.core.* +import org.pkl.core.util.AnsiStringBuilder class StackTraceRendererTest { companion object { @@ -190,7 +191,7 @@ class StackTraceRendererTest { } val loop = StackTraceRenderer.StackFrameLoop(loopFrames, 1) val frames = listOf(createFrame("bar", 1), createFrame("baz", 2), loop) - val formatter = AnsiCodingStringBuilder(false) + val formatter = AnsiStringBuilder(false) renderer.doRender(frames, null, formatter, "", true) val renderedFrames = formatter.toString() assertThat(renderedFrames) diff --git a/pkl-core/src/test/kotlin/org/pkl/core/runtime/AnsiCodingStringBuilderTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/util/AnsiStringBuilderTest.kt similarity index 82% rename from pkl-core/src/test/kotlin/org/pkl/core/runtime/AnsiCodingStringBuilderTest.kt rename to pkl-core/src/test/kotlin/org/pkl/core/util/AnsiStringBuilderTest.kt index 1197afe8..85b2c722 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/runtime/AnsiCodingStringBuilderTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/util/AnsiStringBuilderTest.kt @@ -13,17 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.pkl.core.runtime +package org.pkl.core.util import java.util.* import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test -import org.pkl.core.runtime.AnsiCodingStringBuilder.AnsiCode +import org.pkl.core.util.AnsiStringBuilder.AnsiCode -class AnsiCodingStringBuilderTest { +class AnsiStringBuilderTest { @Test fun `no formatting`() { - val result = AnsiCodingStringBuilder(false).append(AnsiCode.RED, "hello").toString() + val result = AnsiStringBuilder(false).append(AnsiCode.RED, "hello").toString() assertThat(result).isEqualTo("hello") } @@ -39,14 +39,14 @@ class AnsiCodingStringBuilderTest { @Test fun `don't emit same color code`() { val result = - AnsiCodingStringBuilder(true).append(AnsiCode.RED, "hi").append(AnsiCode.RED, "hi").toString() + AnsiStringBuilder(true).append(AnsiCode.RED, "hi").append(AnsiCode.RED, "hi").toString() assertThat(result.escaped).isEqualTo("${red}hihi${reset}".escaped) } @Test fun `only add needed codes`() { val result = - AnsiCodingStringBuilder(true) + AnsiStringBuilder(true) .append(AnsiCode.RED, "hi") .append(EnumSet.of(AnsiCode.RED, AnsiCode.BOLD), "hi") .toString() @@ -56,7 +56,7 @@ class AnsiCodingStringBuilderTest { @Test fun `reset if need to subtract`() { val result = - AnsiCodingStringBuilder(true) + AnsiStringBuilder(true) .append(EnumSet.of(AnsiCode.RED, AnsiCode.BOLD), "hi") .append(AnsiCode.RED, "hi") .toString() @@ -66,7 +66,7 @@ class AnsiCodingStringBuilderTest { @Test fun `plain text in between`() { val result = - AnsiCodingStringBuilder(true) + AnsiStringBuilder(true) .append(AnsiCode.RED, "hi") .append("hi") .append(AnsiCode.RED, "hi")