Skip to content

Commit

Permalink
Fix last argument always being treated as greedy in suggestions (#428)
Browse files Browse the repository at this point in the history
  • Loading branch information
Pablete1234 authored Feb 22, 2023
1 parent b1e55c5 commit 1c8552a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 11 deletions.
31 changes: 20 additions & 11 deletions cloud-core/src/main/java/cloud/commandframework/CommandTree.java
Original file line number Diff line number Diff line change
Expand Up @@ -630,16 +630,13 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
if (commandQueue.isEmpty()) {
return Collections.emptyList();
} else if (child.isLeaf()) {
final String input;
// Handles only simple cases, others will attempt to parse and then decide based on what gets consumed.
if (commandQueue.size() == 1) {
input = commandQueue.peek();
} else {
input = child.getValue() instanceof CompoundArgument
? ((LinkedList<String>) commandQueue).getLast()
: String.join(" ", commandQueue);
return this.directSuggestions(commandContext, child, commandQueue.peek());
} else if (child.getValue() instanceof CompoundArgument) {
return this.directSuggestions(commandContext, child, ((LinkedList<String>) commandQueue).getLast());
}
return this.directSuggestions(commandContext, child, input);
} else if (commandQueue.peek().isEmpty()) {
} else if (commandQueue.size() == 1 && commandQueue.peek().isEmpty()) {
return this.directSuggestions(commandContext, child, commandQueue.peek());
}

Expand All @@ -662,6 +659,18 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
final Optional<?> parsedValue = result.getParsedValue();
final boolean parseSuccess = parsedValue.isPresent();

// It's the last node, we don't care for success or not as we don't need to delegate to a child
if (child.isLeaf()) {
if (commandQueue.isEmpty()) {
// Greedy parser took all the input, we can restore and just ask for suggestions
commandQueue.addAll(commandQueueOriginal);
return this.directSuggestions(commandContext, child, String.join(" ", commandQueue));
} else {
// It's a leaf and there's leftover stuff, no possible suggestions!
return Collections.emptyList();
}
}

if (parseSuccess && !commandQueue.isEmpty()) {
// the current argument at the position is parsable and there are more arguments following
commandContext.store(child.getValue().getName(), parsedValue.get());
Expand All @@ -688,10 +697,10 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
// Therefore we shouldn't list the suggestions of the current argument, as clearly the suggestions of
// one of the following arguments is requested
return Collections.emptyList();
} else {
// Fallback: use suggestion provider of argument
return this.directSuggestions(commandContext, child, commandQueue.peek());
}

// Fallback: use suggestion provider of argument
return this.directSuggestions(commandContext, child, commandQueue.peek());
}

private @NonNull String stringOrEmpty(final @Nullable String string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import cloud.commandframework.arguments.compound.ArgumentTriplet;
import cloud.commandframework.arguments.parser.ArgumentParseResult;
import cloud.commandframework.arguments.standard.BooleanArgument;
import cloud.commandframework.arguments.standard.DurationArgument;
import cloud.commandframework.arguments.standard.EnumArgument;
import cloud.commandframework.arguments.standard.IntegerArgument;
import cloud.commandframework.arguments.standard.StringArgument;
Expand Down Expand Up @@ -113,6 +114,8 @@ static void setupManager() {
manager.command(manager.commandBuilder("numberswithmin")
.argument(IntegerArgument.<TestCommandSender>builder("num").withMin(5).withMax(100)));

manager.command(manager.commandBuilder("duration").argument(DurationArgument.of("duration")));

manager.command(manager.commandBuilder("partial")
.argument(
StringArgument.<TestCommandSender>builder("arg")
Expand Down Expand Up @@ -316,6 +319,10 @@ void testNumbers() {
final String input5 = "numberswithmin ";
final List<String> suggestions5 = manager.suggest(new TestCommandSender(), input5);
Assertions.assertEquals(Arrays.asList("5", "6", "7", "8", "9"), suggestions5);

final String input6 = "numbers 1 ";
final List<String> suggestions6 = manager.suggest(new TestCommandSender(), input6);
Assertions.assertEquals(Collections.emptyList(), suggestions6);
}

@Test
Expand All @@ -337,6 +344,22 @@ void testNumbersWithFollowingArguments() {
);
}

@Test
void testDurations() {
final String input = "duration ";
final List<String> suggestions = manager.suggest(new TestCommandSender(), input);
Assertions.assertEquals(Arrays.asList("1", "2", "3", "4", "5", "6", "7", "8", "9"), suggestions);
final String input2 = "duration 5";
final List<String> suggestions2 = manager.suggest(new TestCommandSender(), input2);
Assertions.assertEquals(Arrays.asList("5d", "5h", "5m", "5s"), suggestions2);
final String input3 = "duration 5s";
final List<String> suggestions3 = manager.suggest(new TestCommandSender(), input3);
Assertions.assertEquals(Collections.emptyList(), suggestions3);
final String input4 = "duration 5s ";
final List<String> suggestions4 = manager.suggest(new TestCommandSender(), input4);
Assertions.assertEquals(Collections.emptyList(), suggestions4);
}

@Test
void testInvalidLiteralThenSpace() {
final String input = "test o";
Expand Down

0 comments on commit 1c8552a

Please sign in to comment.