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.

Sign up for GitHub

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

Jump to bottom

Fix for issue 109 #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix for issue 109 #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion src/main/java/com/mojang/brigadier/builder/LiteralArgumentBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
public class LiteralArgumentBuilder<S> extends ArgumentBuilder<S, LiteralArgumentBuilder<S>> {
private final String literal;

private boolean isCaseInsensitive = false;
Copy link

Choose a reason for hiding this comment

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

This should be final. Also rename the variable to ignoreCase as that is the convention.


protected LiteralArgumentBuilder(final String literal) {
Copy link

Choose a reason for hiding this comment

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

Add the additional ignoreCase parameter to the constructor.

Then create the method public static <S> LiteralArgumentBuilder<S> literal(final String name, final boolean ignoreCase) {

Then modify literal(String) to call the created method with false so that old code is still compatible.

this.literal = literal;
}
Expand All @@ -26,9 +28,16 @@ public String getLiteral() {
return literal;
}

public boolean isCaseInsensitive() { return isCaseInsensitive; }
Copy link

Choose a reason for hiding this comment

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

Rename to ignoresCase()


public LiteralArgumentBuilder<S> caseInsensitive(boolean isCaseInsensitive) {
Copy link

Choose a reason for hiding this comment

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

This method should be removed as ignoreCase should be final.

this.isCaseInsensitive = isCaseInsensitive;
return getThis();
}

@Override
public LiteralCommandNode<S> build() {
final LiteralCommandNode<S> result = new LiteralCommandNode<>(getLiteral(), getCommand(), getRequirement(), getRedirect(), getRedirectModifier(), isFork());
final LiteralCommandNode<S> result = new LiteralCommandNode<>(getLiteral(), getCommand(), getRequirement(), getRedirect(), getRedirectModifier(), isFork(), isCaseInsensitive());

for (final CommandNode<S> argument : getArguments()) {
result.addChild(argument);
Expand Down
17 changes: 15 additions & 2 deletions src/main/java/com/mojang/brigadier/tree/CommandNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.mojang.brigadier.RedirectModifier;
import com.mojang.brigadier.StringReader;
import com.mojang.brigadier.builder.ArgumentBuilder;
import com.mojang.brigadier.builder.LiteralArgumentBuilder;
import com.mojang.brigadier.context.CommandContext;
import com.mojang.brigadier.context.CommandContextBuilder;
import com.mojang.brigadier.exceptions.CommandSyntaxException;
Expand Down Expand Up @@ -82,7 +83,13 @@ public void addChild(final CommandNode<S> node) {
} else {
children.put(node.getName(), node);
if (node instanceof LiteralCommandNode) {
literals.put(node.getName(), (LiteralCommandNode<S>) node);
LiteralCommandNode literalNode = (LiteralCommandNode)node;

if (literalNode.isCaseInsensitive()) {
literals.put(node.getName().toLowerCase(), (LiteralCommandNode<S>) node);
} else {
literals.put(node.getName(), (LiteralCommandNode<S>) node);
}
} else if (node instanceof ArgumentCommandNode) {
arguments.put(node.getName(), (ArgumentCommandNode<S, ?>) node);
}
Expand Down Expand Up @@ -158,7 +165,13 @@ public Collection<? extends CommandNode<S>> getRelevantNodes(final StringReader
}
final String text = input.getString().substring(cursor, input.getCursor());
input.setCursor(cursor);
final LiteralCommandNode<S> literal = literals.get(text);

boolean caseInsensitive = false;
if (this instanceof LiteralCommandNode) {
caseInsensitive = ((LiteralCommandNode)this).isCaseInsensitive();
}

final LiteralCommandNode<S> literal = literals.get(caseInsensitive ? text.toLowerCase() : text);
if (literal != null) {
return Collections.singleton(literal);
} else {
Expand Down
23 changes: 21 additions & 2 deletions src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
public class LiteralCommandNode<S> extends CommandNode<S> {
private final String literal;
private final String literalLowerCase;
private boolean isCaseInsensitive = false;
Copy link

Choose a reason for hiding this comment

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

This should be final, also rename to ignoreCase as that is convention


public LiteralCommandNode(final String literal, final Command<S> command, final Predicate<S> requirement, final CommandNode<S> redirect, final RedirectModifier<S> modifier, final boolean forks) {
public LiteralCommandNode(final String literal, final Command<S> command, final Predicate<S> requirement, final CommandNode<S> redirect, final RedirectModifier<S> modifier, final boolean forks,
final boolean isCaseInsensitive) {
super(command, requirement, redirect, modifier, forks);
this.literal = literal;
this.literalLowerCase = literal.toLowerCase(Locale.ROOT);
this.isCaseInsensitive = isCaseInsensitive;
}

public String getLiteral() {
Expand All @@ -39,6 +42,12 @@ public String getName() {
return literal;
}

public boolean isCaseInsensitive() { return isCaseInsensitive; }
Copy link

Choose a reason for hiding this comment

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

rename to ignoresCase()


public void caseInsensitive(boolean isCaseInsensitive) {
Copy link

Choose a reason for hiding this comment

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

Remove this method, nodes should never be able to be modified after they have been created, otherwise it is possible to violate the guarantees stipulated in CommandDispatcher#getPath() and CommandDispatcher#findNode()

this.isCaseInsensitive = isCaseInsensitive;
}

@Override
public void parse(final StringReader reader, final CommandContextBuilder<S> contextBuilder) throws CommandSyntaxException {
final int start = reader.getCursor();
Expand All @@ -55,7 +64,17 @@ private int parse(final StringReader reader) {
final int start = reader.getCursor();
if (reader.canRead(literal.length())) {
final int end = start + literal.length();
if (reader.getString().substring(start, end).equals(literal)) {

boolean foundMatch = false;
String subString = reader.getString().substring(start, end);

if (isCaseInsensitive) {
foundMatch = subString.equalsIgnoreCase(literal);
} else {
foundMatch = subString.equals(literal);
}

if (foundMatch) {
reader.setCursor(end);
if (!reader.canRead() || reader.peek() == ' ') {
return end;
Expand Down
19 changes: 19 additions & 0 deletions src/test/java/com/mojang/brigadier/CommandDispatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package com.mojang.brigadier;

import com.google.common.collect.Lists;
import com.mojang.brigadier.arguments.ArgumentType;
import com.mojang.brigadier.arguments.StringArgumentType;
import com.mojang.brigadier.context.CommandContext;
import com.mojang.brigadier.context.CommandContextBuilder;
import com.mojang.brigadier.exceptions.CommandSyntaxException;
Expand Down Expand Up @@ -61,6 +63,15 @@ public void testCreateAndExecuteCommand() throws Exception {
verify(command).run(any(CommandContext.class));
}

@SuppressWarnings("unchecked")
@Test
public void testCreateAndExecuteCaseInsensitiveCommand() throws Exception {
subject.register(literal("Foo").then(argument("bar", integer()).executes(command)).caseInsensitive(true));

assertThat(subject.execute("foo 123", source), is(42));
verify(command).run(any(CommandContext.class));
}

@SuppressWarnings("unchecked")
@Test
public void testCreateAndExecuteOffsetCommand() throws Exception {
Expand All @@ -70,6 +81,14 @@ public void testCreateAndExecuteOffsetCommand() throws Exception {
verify(command).run(any(CommandContext.class));
}

@Test
public void testCreateAndExecuteCaseInsensitiveOffsetCommand() throws Exception {
subject.register(literal("Foo").executes(command).caseInsensitive(true));

assertThat(subject.execute(inputWithOffset("/foo", 1), source), is(42));
verify(command).run(any(CommandContext.class));
}

Copy link

Choose a reason for hiding this comment

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

Additional tests for using ignoreCase as a command argument should be added. Currently this only these tests only test the base node.

@SuppressWarnings("unchecked")
@Test
public void testCreateAndMergeCommands() throws Exception {
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/com/mojang/brigadier/tree/LiteralCommandNodeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ public void testParse() throws Exception {
assertThat(reader.getRemaining(), equalTo(" bar"));
}

@Test
public void testCaseInsensitiveParse() throws Exception {
try {
node.caseInsensitive(true);
final StringReader reader = new StringReader("FoO bar");
node.parse(reader, contextBuilder);
assertThat(reader.getRemaining(), equalTo(" bar"));
} finally {
node.caseInsensitive(false);
}
}

@Test
public void testParseExact() throws Exception {
final StringReader reader = new StringReader("foo");
Expand Down