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

Fix last argument always being treated as greedy in suggestions #428

Merged
merged 2 commits into from
Feb 22, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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