Skip to content

Commit c11969f

Browse files
jpenillaCitymonstret
jpenilla
and
Citymonstret
authored
Fix multi-word flag suggestions (#736)
* Half-baked fix for multi-word flag suggestions * Don't ignore result future for flag parsing * fix jd * restore old cursor restoration logic * Keep suggesting after successfully parsed flag value if there is no following space We could better support greedy flags by using similar logic to flag yielding strings here. However, for now this solves the multi-word suggestion issue. * Flag yielding parsers will consume a single trailing dash --------- Co-authored-by: Alexander Söderberg <[email protected]>
1 parent 5d31f7d commit c11969f

File tree

3 files changed

+51
-31
lines changed

3 files changed

+51
-31
lines changed

‎cloud-core/src/main/java/org/incendo/cloud/CommandTree.java‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ private boolean matchesLiteral(final @NonNull List<@NonNull CommandNode<C>> chil
727727
continue;
728728
}
729729
suggestionFuture = suggestionFuture
730-
.thenCompose(ctx -> this.addSuggestionsForDynamicArgument(context, commandInput, child, executor));
730+
.thenCompose(ctx -> this.addSuggestionsForDynamicArgument(context, commandInput, child, executor, false));
731731
}
732732

733733
return suggestionFuture;
@@ -769,28 +769,33 @@ private boolean matchesLiteral(final @NonNull List<@NonNull CommandNode<C>> chil
769769
final @NonNull SuggestionContext<C, ?> context,
770770
final @NonNull CommandInput commandInput,
771771
final @NonNull CommandNode<C> child,
772-
final @NonNull Executor executor
772+
final @NonNull Executor executor,
773+
final boolean inFlag
773774
) {
774775
final CommandComponent<C> component = child.component();
775776
if (component == null) {
776777
return CompletableFuture.completedFuture(context);
777778
}
778779

779-
if (component.parser() instanceof CommandFlagParser) {
780+
if (!inFlag && component.parser() instanceof CommandFlagParser) {
780781
// Use the flag argument parser to deduce what flag is being suggested right now
781782
// If empty, then no flag value is being typed, and the different flag options should
782783
// be suggested instead.
783784
final CommandFlagParser<C> parser = (CommandFlagParser<C>) component.parser();
784-
final Optional<String> lastFlag = parser.parseCurrentFlag(context.commandContext(), commandInput);
785-
if (lastFlag.isPresent()) {
786-
context.commandContext().store(CommandFlagParser.FLAG_META_KEY, lastFlag.get());
787-
} else {
788-
context.commandContext().remove(CommandFlagParser.FLAG_META_KEY);
789-
}
785+
786+
return parser.parseCurrentFlag(context.commandContext(), commandInput, executor).thenCompose(lastFlag -> {
787+
if (lastFlag.isPresent()) {
788+
context.commandContext().store(CommandFlagParser.FLAG_META_KEY, lastFlag.get());
789+
} else {
790+
context.commandContext().remove(CommandFlagParser.FLAG_META_KEY);
791+
}
792+
return this.addSuggestionsForDynamicArgument(context, commandInput, child, executor, true);
793+
});
790794
}
791795

792796
if (commandInput.isEmpty() || commandInput.remainingTokens() == 1
793-
|| (child.isLeaf() && child.component().parser() instanceof AggregateParser)) {
797+
|| (child.isLeaf() && child.component().parser() instanceof AggregateParser)
798+
|| (child.isLeaf() && child.component().parser() instanceof CommandFlagParser)) {
794799
return this.addArgumentSuggestions(context, child, commandInput, executor);
795800
}
796801

‎cloud-core/src/main/java/org/incendo/cloud/parser/flag/CommandFlagParser.java‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.Optional;
3535
import java.util.Set;
3636
import java.util.concurrent.CompletableFuture;
37+
import java.util.concurrent.Executor;
3738
import java.util.regex.Matcher;
3839
import java.util.regex.Pattern;
3940
import org.apiguardian.api.API;
@@ -63,6 +64,10 @@ public final class CommandFlagParser<C> implements ArgumentParser.FutureArgument
6364
* Metadata for the last argument that was suggested
6465
*/
6566
public static final CloudKey<String> FLAG_META_KEY = CloudKey.of("__last_flag__", TypeToken.get(String.class));
67+
/**
68+
* Metadata for the position of the cursor before parsing the last flag's value
69+
*/
70+
public static final CloudKey<Integer> FLAG_CURSOR_KEY = CloudKey.of("__flag_cursor__", TypeToken.get(Integer.class));
6671
/**
6772
* Metadata for the set of parsed flags, used to detect duplicates.
6873
*/
@@ -110,35 +115,35 @@ public CommandFlagParser(final @NonNull Collection<@NonNull CommandFlag<?>> flag
110115
*
111116
* @param commandContext Command context
112117
* @param commandInput The input arguments
118+
* @param completionExecutor The completion executor
113119
* @return current flag being typed, or {@code empty()} if none is
114120
*/
115121
@API(status = API.Status.STABLE)
116-
public @NonNull Optional<String> parseCurrentFlag(
122+
public @NonNull CompletableFuture<Optional<String>> parseCurrentFlag(
117123
final @NonNull CommandContext<@NonNull C> commandContext,
118-
final @NonNull CommandInput commandInput
124+
final @NonNull CommandInput commandInput,
125+
final @NonNull Executor completionExecutor
119126
) {
120127
/* If empty, nothing to do */
121128
if (commandInput.isEmpty()) {
122-
return Optional.empty();
129+
return CompletableFuture.completedFuture(Optional.empty());
123130
}
124131

125-
/* Before parsing, retrieve the last known input of the queue */
126132
final String lastInputValue = commandInput.lastRemainingToken();
127133

128134
/* Parse, but ignore the result of parsing */
129135
final FlagParser parser = new FlagParser();
130-
parser.parse(commandContext, commandInput);
131-
132-
/*
133-
* If the parser parsed the entire queue, restore the last typed
134-
* input obtained earlier.
135-
*/
136-
if (commandInput.isEmpty()) {
137-
final int count = lastInputValue.length();
138-
commandInput.moveCursor(-count);
139-
}
140-
141-
return Optional.ofNullable(parser.lastParsedFlag());
136+
final CompletableFuture<@NonNull ArgumentParseResult<Object>> result = parser.parse(commandContext, commandInput);
137+
138+
return result.thenApplyAsync(parseResult -> {
139+
if (commandContext.contains(FLAG_CURSOR_KEY)) {
140+
commandInput.cursor(commandContext.get(FLAG_CURSOR_KEY));
141+
} else if (parser.lastParsedFlag() == null && commandInput.isEmpty()) {
142+
final int count = lastInputValue.length();
143+
commandInput.moveCursor(-count);
144+
}
145+
return Optional.ofNullable(parser.lastParsedFlag());
146+
}, completionExecutor);
142147
}
143148

144149
@Override
@@ -485,6 +490,7 @@ private final class FlagParser {
485490

486491
// The flag has no argument, so we're done.
487492
if (flag.commandComponent() == null) {
493+
commandContext.remove(FLAG_CURSOR_KEY);
488494
commandContext.flags().addPresenceFlag(flag);
489495
parsedFlags.add(flag);
490496
return CompletableFuture.completedFuture(null);
@@ -515,12 +521,17 @@ private final class FlagParser {
515521

516522
// We then attempt to parse the flag.
517523
final CommandFlag parsingFlag = flag;
524+
final CommandInput commandInputCopy = commandInput.copy();
518525
return ((CommandComponent<C>) flag.commandComponent())
519526
.parser()
520527
.parseFuture(
521528
commandContext,
522529
commandInput
523530
).thenApply(parsedValue -> {
531+
if (parsedValue.failure().isPresent() || commandInput.isEmpty() || commandInput.peek() != ' ') {
532+
commandContext.store(FLAG_CURSOR_KEY, commandInputCopy.cursor());
533+
}
534+
524535
// Forward parsing errors.
525536
if (parsedValue.failure().isPresent()) {
526537
return (ArgumentParseResult<Object>) parsedValue;
@@ -531,8 +542,12 @@ private final class FlagParser {
531542
// At this point we know the flag parsed successfully.
532543
parsedFlags.add(parsingFlag);
533544

534-
// We're no longer parsing a flag.
535-
this.lastParsedFlag = null;
545+
if (!commandInput.isEmpty(false)) {
546+
if (commandInput.peek() == ' ') {
547+
// We're no longer parsing a flag.
548+
this.lastParsedFlag = null;
549+
}
550+
}
536551

537552
return null;
538553
});
@@ -548,7 +563,7 @@ private final class FlagParser {
548563
}
549564

550565
private @NonNull CompletableFuture<ArgumentParseResult<Object>> fail(final @NonNull Throwable exception) {
551-
return CompletableFuture.completedFuture(ArgumentParseResult.failure(exception));
566+
return ArgumentParseResult.failureFuture(exception);
552567
}
553568
}
554569
}

‎cloud-core/src/test/java/org/incendo/cloud/CommandSuggestionsTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,11 +404,11 @@ void testCompoundFlags() {
404404

405405
final String input5 = "flags3 --compound 22 ";
406406
final List<? extends Suggestion> suggestions5 = this.manager.suggestionFactory().suggestImmediately(new TestCommandSender(), input5).list();
407-
Assertions.assertEquals(suggestionList("0", "1", "2", "3", "4", "5", "6", "7", "8", "9"), suggestions5);
407+
Assertions.assertEquals(suggestionList("22 0", "22 1", "22 2", "22 3", "22 4", "22 5", "22 6", "22 7", "22 8", "22 9"), suggestions5);
408408

409409
final String input6 = "flags3 --compound 22 1";
410410
final List<? extends Suggestion> suggestions6 = this.manager.suggestionFactory().suggestImmediately(new TestCommandSender(), input6).list();
411-
Assertions.assertEquals(suggestionList("1", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19"), suggestions6);
411+
Assertions.assertEquals(suggestionList("22 1", "22 10", "22 11", "22 12", "22 13", "22 14", "22 15", "22 16", "22 17", "22 18", "22 19"), suggestions6);
412412

413413
/* We've typed compound already, so that flag should be omitted from the suggestions */
414414
final String input7 = "flags3 --compound 22 33 44 ";

0 commit comments

Comments
 (0)