Skip to content

Commit

Permalink
CLI - Rework how missing commands are detected
Browse files Browse the repository at this point in the history
We had a problem here as most options are only valid in the context of
the root command (for instance -D) so trying to parse them in the
context of a subcommand or on their own will most probably fail.

Also parsing recursively the command is non-efficient as we do a lot of
parsing, especially since it was done twice (this was fixed too).

The new approach uses the ParseResult generated once and get the
knownledge from there. From my tests, it provides similar results and
avoid false positives and running the parsing too many times.

This should fix CliHelpTest#testCommandHelp() being flaky as not being
able to interpret the options ended up triggering the missing command
branch and then the JBang support.

(cherry picked from commit fb4a379)
  • Loading branch information
gsmet committed Dec 18, 2023
1 parent 768e2a2 commit 27bf076
Showing 1 changed file with 29 additions and 16 deletions.
45 changes: 29 additions & 16 deletions devtools/cli/src/main/java/io/quarkus/cli/QuarkusCli.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import jakarta.inject.Inject;

Expand Down Expand Up @@ -96,7 +97,9 @@ public int run(String... args) throws Exception {
boolean pluginCommand = args.length >= 1 && (args[0].equals("plug") || args[0].equals("plugin"));

try {
boolean existingCommand = checkMissingCommand(cmd, args).isEmpty();
Optional<String> missingCommand = checkMissingCommand(cmd, args);

boolean existingCommand = missingCommand.isEmpty();
// If the command already exists and is not a help command (that lists subcommands) or plugin command, then just execute
// without dealing with plugins.
// The reason that we check if its a plugin command is that plugin commands need PluginManager initialization.
Expand All @@ -108,8 +111,7 @@ public int run(String... args) throws Exception {
pluginManager.syncIfNeeded();
Map<String, Plugin> plugins = new HashMap<>(pluginManager.getInstalledPlugins());
pluginCommandFactory.populateCommands(cmd, plugins);
Optional<String> missing = checkMissingCommand(cmd, args);
missing.ifPresent(m -> {
missingCommand.ifPresent(m -> {
try {
Map<String, Plugin> installable = pluginManager.getInstallablePlugins();
if (installable.containsKey(m)) {
Expand All @@ -119,11 +121,13 @@ public int run(String... args) throws Exception {
output.info("Command %s not installed but the following plugin is available:\n%s", m,
table.getContent());
if (interactiveMode && Prompt.yesOrNo(true,
"Would you like to install it now ?",
"Would you like to install it now?",
args)) {
pluginManager.addPlugin(m).ifPresent(added -> plugins.put(added.getName(), added));
pluginCommandFactory.populateCommands(cmd, plugins);
}
} else {
output.error("Command %s is missing and can't be installed.", m);
}
} catch (Exception e) {
output.error("Command %s is missing and can't be installed.", m);
Expand All @@ -136,7 +140,7 @@ public int run(String... args) throws Exception {
}

/**
* Recursivelly processes the arguments passed to the command and checks wether a subcommand is missing.
* Process the arguments passed and return an identifier of the potentially missing subcommand if any.
*
* @param root the root command
* @param args the arguments passed to the root command
Expand All @@ -148,17 +152,26 @@ public Optional<String> checkMissingCommand(CommandLine root, String[] args) {
}

try {
ParseResult result = root.parseArgs(args);
if (args.length == 1) {
return Optional.empty();
}
CommandLine next = root.getSubcommands().get(args[0]);
if (next == null) {
return Optional.of(args[0]);
}
String[] remaining = new String[args.length - 1];
System.arraycopy(args, 1, remaining, 0, remaining.length);
return checkMissingCommand(next, remaining).map(nextMissing -> root.getCommandName() + "-" + nextMissing);
ParseResult currentParseResult = root.parseArgs(args);
StringBuilder missingCommand = new StringBuilder();

do {
if (missingCommand.length() > 0) {
missingCommand.append("-");
}
missingCommand.append(currentParseResult.commandSpec().name());

List<String> unmatchedSubcommands = currentParseResult.unmatched().stream()
.filter(u -> !u.startsWith("-")).collect(Collectors.toList());
if (!unmatchedSubcommands.isEmpty()) {
missingCommand.append("-").append(unmatchedSubcommands.get(0));
return Optional.of(missingCommand.toString());
}

currentParseResult = currentParseResult.subcommand();
} while (currentParseResult != null);

return Optional.empty();
} catch (UnmatchedArgumentException e) {
return Optional.of(args[0]);
}
Expand Down

0 comments on commit 27bf076

Please sign in to comment.