Fix for issue 109 #126
Fix for issue 109 #126
Conversation
| @@ -9,6 +9,8 @@ | |||
| public class LiteralArgumentBuilder<S> extends ArgumentBuilder<S, LiteralArgumentBuilder<S>> { | |||
| private final String literal; | |||
|
|
|||
| private boolean isCaseInsensitive = false; | |||
There was a problem hiding this comment.
This should be final. Also rename the variable to ignoreCase as that is the convention.
| @@ -9,6 +9,8 @@ | |||
| public class LiteralArgumentBuilder<S> extends ArgumentBuilder<S, LiteralArgumentBuilder<S>> { | |||
| private final String literal; | |||
|
|
|||
| private boolean isCaseInsensitive = false; | |||
|
|
|||
| protected LiteralArgumentBuilder(final String literal) { | |||
There was a problem hiding this comment.
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.
| @@ -26,9 +28,16 @@ public String getLiteral() { | |||
| return literal; | |||
| } | |||
|
|
|||
| public boolean isCaseInsensitive() { return isCaseInsensitive; } | |||
| @@ -26,9 +28,16 @@ public String getLiteral() { | |||
| return literal; | |||
| } | |||
|
|
|||
| public boolean isCaseInsensitive() { return isCaseInsensitive; } | |||
|
|
|||
| public LiteralArgumentBuilder<S> caseInsensitive(boolean isCaseInsensitive) { | |||
There was a problem hiding this comment.
This method should be removed as ignoreCase should be final.
| @@ -23,11 +23,14 @@ | |||
| public class LiteralCommandNode<S> extends CommandNode<S> { | |||
| private final String literal; | |||
| private final String literalLowerCase; | |||
| private boolean isCaseInsensitive = false; | |||
There was a problem hiding this comment.
This should be final, also rename to ignoreCase as that is convention
| @@ -39,6 +42,12 @@ public String getName() { | |||
| return literal; | |||
| } | |||
|
|
|||
| public boolean isCaseInsensitive() { return isCaseInsensitive; } | |||
| @@ -39,6 +42,12 @@ public String getName() { | |||
| return literal; | |||
| } | |||
|
|
|||
| public boolean isCaseInsensitive() { return isCaseInsensitive; } | |||
|
|
|||
| public void caseInsensitive(boolean isCaseInsensitive) { | |||
There was a problem hiding this comment.
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()
| assertThat(subject.execute(inputWithOffset("/foo", 1), source), is(42)); | ||
| verify(command).run(any(CommandContext.class)); | ||
| } | ||
|
|
There was a problem hiding this comment.
Additional tests for using ignoreCase as a command argument should be added. Currently this only these tests only test the base node.
Literals can now be case insensitive, which can be specified with a flag in LiteralArgumentBuilder.
if the flag is true, the literal will be put into the literals hash in CommandNode with its key set to all lower case, and it will be gotten from the hash with a lowercased key.
When CommandNode.parse is called, it will check the flag and do a case insensitive check if necessary.