Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text-minimessage: Allow tags to be closed in one tag #707

Merged
merged 2 commits into from Feb 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -99,6 +99,7 @@ private void processTokens(final @NotNull StringBuilder sb, final @NotNull Strin
break;
case OPEN_TAG:
case CLOSE_TAG:
case OPEN_CLOSE_TAG:
// extract tag name
if (token.childTokens().isEmpty()) {
sb.append(richMessage, token.startIndex(), token.endIndex());
Expand Down
Expand Up @@ -82,6 +82,18 @@ private static void visit(final @NotNull Component component, final Collector em
}

static final class Collector implements TokenEmitter, ClaimConsumer {
enum TagState {
TEXT(false),
MID(true),
MID_SELF_CLOSING(true);

final boolean isTag;

TagState(final boolean isTag) {
this.isTag = isTag;
}
}

/**
* mark tag boundaries within the stack, without needing to mess with typing too much.
*/
Expand All @@ -96,7 +108,7 @@ static final class Collector implements TokenEmitter, ClaimConsumer {
private final StringBuilder consumer;
private String[] activeTags = new String[4];
private int tagLevel = 0;
private boolean midTag;
private TagState tagState = TagState.TEXT;

Collector(final SerializableResolver resolver, final boolean strict, final StringBuilder consumer) {
this.resolver = resolver;
Expand Down Expand Up @@ -128,7 +140,6 @@ void mark() {
}

void popToMark() {
this.completeTag();
if (this.tagLevel == 0) {
return;
}
Expand All @@ -139,7 +150,6 @@ void popToMark() {
}

void popAll() {
this.completeTag();
while (this.tagLevel > 0) {
final String tag = this.activeTags[--this.tagLevel];
if (tag != MARK) {
Expand All @@ -149,9 +159,9 @@ void popAll() {
}

void completeTag() {
if (this.midTag) {
if (this.tagState.isTag) {
this.consumer.append(TokenParser.TAG_END);
this.midTag = false;
this.tagState = TagState.TEXT;
}
}

Expand All @@ -162,14 +172,23 @@ public Collector tag(final String token) {
this.completeTag();
this.consumer.append(TokenParser.TAG_START);
this.escapeTagContent(token, QuotingOverride.UNQUOTED);
this.midTag = true;
this.tagState = TagState.MID;
this.pushActiveTag(token);
return this;
}

@Override
public @NotNull TokenEmitter selfClosingTag(@NotNull final String token) {
this.completeTag();
this.consumer.append(TokenParser.TAG_START);
this.escapeTagContent(token, QuotingOverride.UNQUOTED);
this.tagState = TagState.MID_SELF_CLOSING;
return null;
}

@Override
public TokenEmitter argument(final String arg) {
if (!this.midTag) {
if (!this.tagState.isTag) {
throw new IllegalStateException("Not within a tag!");
}
this.consumer.append(TokenParser.SEPARATOR);
Expand All @@ -179,7 +198,7 @@ public TokenEmitter argument(final String arg) {

@Override
public @NotNull TokenEmitter argument(final @NotNull String arg, final @NotNull QuotingOverride quotingPreference) {
if (!this.midTag) {
if (!this.tagState.isTag) {
throw new IllegalStateException("Not within a tag!");
}
this.consumer.append(TokenParser.SEPARATOR);
Expand All @@ -193,15 +212,6 @@ public TokenEmitter argument(final String arg) {
return this.argument(serialized, QuotingOverride.QUOTED); // always quote tokens
}

@Override
public Collector selfClosing(final String token) {
this.completeTag();
this.consumer.append(TokenParser.TAG_START);
this.escapeTagContent(token, QuotingOverride.UNQUOTED);
this.midTag = true; // TODO: `<tag/>` syntax
return this;
}

@Override
public Collector text(final String text) {
this.completeTag();
Expand Down Expand Up @@ -276,17 +286,24 @@ static void appendEscaping(final StringBuilder builder, final String text, final

@Override
public Collector pop() {
this.completeTag();
this.emitClose(this.popTag(false));
return this;
}

private void emitClose(final @NotNull String tag) {
// currently: we don't keep any arguments, does it ever make sense to?
this.consumer.append(TokenParser.TAG_START)
.append(TokenParser.CLOSE_TAG);
this.escapeTagContent(tag, QuotingOverride.UNQUOTED);
this.consumer.append(TokenParser.TAG_END);
if (this.tagState.isTag) {
if (this.tagState == TagState.MID) { // not _SELF_CLOSING
this.consumer.append(TokenParser.CLOSE_TAG);
}
this.consumer.append(TokenParser.TAG_END);
this.tagState = TagState.TEXT;
} else {
this.consumer.append(TokenParser.TAG_START)
.append(TokenParser.CLOSE_TAG);
this.escapeTagContent(tag, QuotingOverride.UNQUOTED);
this.consumer.append(TokenParser.TAG_END);
}
}

// ClaimCollector
Expand Down
Expand Up @@ -215,8 +215,10 @@ public static void parseString(final String message, final MatchedTokenConsumer<

// closing tags start with </
TokenType thisType = TokenType.OPEN_TAG;
if (boundsCheck(message, marker, 1) && message.charAt(marker + 1) == CLOSE_TAG) {
if (boundsCheck(message, marker, 1) && message.charAt(marker + 1) == CLOSE_TAG) { // </content>
thisType = TokenType.CLOSE_TAG;
} else if (boundsCheck(message, marker, 2) && message.charAt(i - 1) == CLOSE_TAG) { // <content/>
thisType = TokenType.OPEN_CLOSE_TAG;
}
consumer.accept(marker, currentTokenEnd, thisType);
state = FirstPassState.NORMAL;
Expand Down Expand Up @@ -256,13 +258,13 @@ public static void parseString(final String message, final MatchedTokenConsumer<
private static void parseSecondPass(final String message, final List<Token> tokens) {
for (final Token token : tokens) {
final TokenType type = token.type();
if (type != TokenType.OPEN_TAG && type != TokenType.CLOSE_TAG) {
if (type != TokenType.OPEN_TAG && type != TokenType.OPEN_CLOSE_TAG && type != TokenType.CLOSE_TAG) {
continue;
}

// Only look inside the tag <[/] and >
final int startIndex = type == TokenType.OPEN_TAG ? token.startIndex() + 1 : token.startIndex() + 2;
final int endIndex = token.endIndex() - 1;
final int startIndex = type == TokenType.CLOSE_TAG ? token.startIndex() + 2 : token.startIndex() + 1;
final int endIndex = type == TokenType.OPEN_CLOSE_TAG ? token.endIndex() - 2 : token.endIndex() - 1;

SecondPassState state = SecondPassState.NORMAL;
boolean escaped = false;
Expand Down Expand Up @@ -365,6 +367,7 @@ private static RootNode buildTree(
break;

case OPEN_TAG:
case OPEN_CLOSE_TAG:
final TagNode tagNode = new TagNode(node, token, message, tagProvider);
if (tagNameChecker.test(tagNode.name())) {
final Tag tag = tagProvider.resolve(tagNode);
Expand All @@ -383,8 +386,8 @@ private static RootNode buildTree(
// This is a recognized tag, goes in the tree
tagNode.tag(tag);
node.addChild(tagNode);
if (!(tag instanceof Inserting) || ((Inserting) tag).allowsChildren()) {
node = tagNode; // TODO: self-terminating tags (i.e. <tag/>) don't set this, so they don't have children
if (type != TokenType.OPEN_CLOSE_TAG && (!(tag instanceof Inserting) || ((Inserting) tag).allowsChildren())) {
node = tagNode;
}
}
} else {
Expand Down
Expand Up @@ -31,6 +31,7 @@
public enum TokenType {
TEXT,
OPEN_TAG,
OPEN_CLOSE_TAG, // one token that both opens and closes a tag
CLOSE_TAG,
TAG_VALUE;
}
Expand Up @@ -42,18 +42,20 @@ public interface TokenEmitter {
@NotNull TokenEmitter tag(final @NotNull String token); // TODO: some sort of TagFlags, with things like SELF_CLOSING, CLOSE_WITH_ARGUMENTS, etc?

/**
* Create a self-contained tag without arguments.
* Open a tag with or without arguments that cannot have children.
*
* <p>These sorts of tags will be closed even without any sort of closing indicator.</p>
*
* @param token the token to emit
* @return this emitter
* @since 4.10.0
*/
@NotNull TokenEmitter selfClosing(final @NotNull String token);
@NotNull TokenEmitter selfClosingTag(final @NotNull String token); // TODO: some sort of TagFlags, with things like SELF_CLOSING, CLOSE_WITH_ARGUMENTS, etc?

/**
* Add arguments to the current tag.
*
* <p>Must be called after {@link #tag(String)} or {@link #selfClosing(String)}, but before any call to {@link #text(String)}.</p>
* <p>Must be called after {@link #tag(String)}, but before any call to {@link #text(String)}.</p>
*
* @param args args to add
* @return this emitter
Expand All @@ -69,7 +71,7 @@ public interface TokenEmitter {
/**
* Add a single argument to the current tag.
*
* <p>Must be called after {@link #tag(String)} or {@link #selfClosing(String)}, but before any call to {@link #text(String)}.</p>
* <p>Must be called after {@link #tag(String)}, but before any call to {@link #text(String)}.</p>
*
* @param arg argument to add
* @return this emitter
Expand All @@ -80,7 +82,7 @@ public interface TokenEmitter {
/**
* Add a single argument to the current tag.
*
* <p>Must be called after {@link #tag(String)} or {@link #selfClosing(String)}, but before any call to {@link #text(String)}.</p>
* <p>Must be called after {@link #tag(String)}, but before any call to {@link #text(String)}.</p>
*
* @param arg argument to add
* @param quotingPreference an argument-specific quoting instruction
Expand All @@ -92,7 +94,7 @@ public interface TokenEmitter {
/**
* Add a single argument to the current tag.
*
* <p>Must be called after {@link #tag(String)} or {@link #selfClosing(String)}, but before any call to {@link #text(String)}.</p>
* <p>Must be called after {@link #tag(String)}, but before any call to {@link #text(String)}.</p>
*
* @param arg argument to add, serialized as a nested MiniMessage string
* @return this emitter
Expand Down
Expand Up @@ -52,12 +52,12 @@ private NewlineTag() {
}

static Tag create(final ArgumentQueue args, final Context ctx) throws ParsingException {
return Tag.inserting(Component.newline());
return Tag.selfClosingInserting(Component.newline());
}

static @Nullable Emitable claimComponent(final Component input) {
if (Component.newline().equals(input)) {
return emit -> emit.tag(BR);
return emit -> emit.selfClosingTag(BR);
} else {
return null;
}
Expand Down
Expand Up @@ -440,4 +440,33 @@ void testTreeOutput() {

assertEquals(expected, tree.toString());
}

@Test
void testTagsSelfClosable() {
final String input = "<red>hello <lang:gameMode.creative/> there";

final Component parsed = Component.text()
.content("hello ")
.color(NamedTextColor.RED)
.append(
Component.translatable("gameMode.creative"),
Component.text(" there")
)
.build();

this.assertParsedEquals(parsed, input);
}

@Test
void testIgnorableSelfClosable() {
final String input = "<red/>things";

final Component parsed = Component.text().append(
Component.text("", NamedTextColor.RED),
Component.text("things")
)
.build();

this.assertParsedEquals(parsed, input);
}
}
Expand Up @@ -36,7 +36,7 @@
class KeybindTagTest extends AbstractTest {
@Test
void testSerializeKeyBind() {
final String expected = "Press <key:key.jump></key> to jump!";
final String expected = "Press <key:key.jump/> to jump!";

final TextComponent.Builder builder = Component.text()
.content("Press ")
Expand Down
Expand Up @@ -31,9 +31,16 @@
import static net.kyori.adventure.text.format.NamedTextColor.GRAY;
import static net.kyori.adventure.text.format.NamedTextColor.RED;
import static net.kyori.adventure.text.format.TextColor.color;
import static org.junit.jupiter.api.Assertions.assertEquals;

class NewlineTagTest extends AbstractTest {

@Test
void testRoundtripNewline() {
final String input = "<red>Line</red><br><gray>break!";
assertEquals(input, PARSER.serialize(PARSER.deserialize(input)));
}

@Test
void testNewLine() {
final String input = "<red>Line<br><gray>break!";
Expand Down
Expand Up @@ -38,7 +38,7 @@
class TranslatableTagTest extends AbstractTest {
@Test
void testSerializeTranslatable() {
final String expected = "You should get a <lang:block.minecraft.diamond_block></lang>!";
final String expected = "You should get a <lang:block.minecraft.diamond_block/>!";

final TextComponent.Builder builder = Component.text()
.content("You should get a ")
Expand Down