Skip to content

Commit

Permalink
api: Fix improper stripping of colors when compacting
Browse files Browse the repository at this point in the history
Fixes GH-780
  • Loading branch information
zml2008 committed Jun 10, 2022
1 parent 7adc8e5 commit a5d8105
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
Expand Up @@ -50,7 +50,7 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa
if (childrenSize == 0) {
// no children, style can be further simplified if self is blank
if (isBlank(optimized)) {
optimized = optimized.style(simplifyStyleForBlank(optimized.style()));
optimized = optimized.style(simplifyStyleForBlank(optimized.style(), parentStyle));
}

// leaf nodes do not need to be further optimized - there is no point
Expand Down Expand Up @@ -147,7 +147,7 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa

// no children, style can be further simplified if self is blank
if (childrenToAppend.isEmpty() && isBlank(optimized)) {
optimized = optimized.style(simplifyStyleForBlank(optimized.style()));
optimized = optimized.style(simplifyStyleForBlank(optimized.style(), parentStyle));
}

return optimized.children(childrenToAppend);
Expand Down Expand Up @@ -212,24 +212,28 @@ private static boolean isBlank(final Component component) {
final char c = content.charAt(i);
if (c != ' ') return false;
}

return true;
}
return false;
}

/**
* Simplify the provided style to remove any information that is redundant,
* given that the content is blank.
*
* @param style style to simplify
* @param parentStyle style from component's parents, for context
* @return a new, simplified style
*/
private static @NotNull Style simplifyStyleForBlank(final @NotNull Style style) {
private static @NotNull Style simplifyStyleForBlank(final @NotNull Style style, final @Nullable Style parentStyle) {
final Style.Builder builder = style.toBuilder();

// TextColor doesn't affect spaces
builder.color(null);
// TextColor doesn't affect spaces, unless there is other decoration present
if (!(style.hasDecoration(TextDecoration.UNDERLINED) || style.hasDecoration(TextDecoration.STRIKETHROUGH))
&& (parentStyle == null || !(parentStyle.hasDecoration(TextDecoration.UNDERLINED) || parentStyle.hasDecoration(TextDecoration.STRIKETHROUGH)))) {
builder.color(null);
}

// ITALIC/OBFUSCATED don't affect spaces (in modern versions), as these styles only affect glyph rendering
builder.decoration(TextDecoration.ITALIC, TextDecoration.State.NOT_SET);
Expand All @@ -239,7 +243,7 @@ private static boolean isBlank(final Component component) {
// BOLD affects spaces because it increments the character advance by 1

// font affects spaces in 1.19+ (since 22w11a), due to the font glyph provider for spaces

return builder.build();
}

Expand Down
Expand Up @@ -396,4 +396,17 @@ void testEmptyCompactionWithManyNonRemovableStyle() {
assertEquals(expectedCompact, notCompact.compact());
}

// https://github.com/KyoriPowered/adventure/issues/780
@Test
void testColorPreservedWithDecorations() {
final Component expectedComponent = text()
.decorate(TextDecoration.UNDERLINED)
.append(text(" ", NamedTextColor.RED))
.append(text(" ", NamedTextColor.GREEN))
.append(text(" ", NamedTextColor.BLUE))
.build();

assertEquals(expectedComponent, expectedComponent.compact());
}

}

0 comments on commit a5d8105

Please sign in to comment.