gnodet-bot commented on code in PR #28763:
URL: https://github.com/apache/camel/pull/28763#discussion_r4430915793
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/CamelJBangMain.java:
##########
@@ -152,155 +163,277 @@ public void execute(String... args) {
quit(exitCode);
}
+ /**
+ * The command of the CLI that a command line runs, when the CLI needs to
create only that command: not for the
+ * help, the version, a command line that starts with an option, or the
commands that use the other commands or the
Review Comment:
⚠️ **Inaccurate Javadoc**: the phrase "not for the help, **the version**, a
command line that starts with an option" implies `version` returns null from
`onlyCommand`, but the test explicitly asserts `assertEquals("version",
CamelJBangMain.onlyCommand("version", "get"))` — `camel version get` IS
optimized. "The version" likely meant `--version` (the flag), which is already
covered by "a command line that starts with an option".
```suggestion
* The command of the CLI that a command line runs, when the CLI needs
to create only that command: not for the
* help, a command line that starts with an option ({@code --version},
{@code --help}), or the commands that use the other commands or the
```
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/CamelJBangMain.java:
##########
@@ -86,6 +86,10 @@
" 70:Internal error: an unexpected exception",
"124:Timed out: no reply from the integration in time" })
public class CamelJBangMain implements Callable<Integer> {
+ // the commands that need all the commands of the CLI: the shell and the
completion script use the others, and the
+ // plugins, which may add to any command, are loaded for run, export and
cmd (see PluginHelper.shouldDiscoverPlugins)
+ private static final Set<String> FULL_COMMANDS = Set.of("shell",
"completion", "run", "export", "cmd");
Review Comment:
💡 **Nit**: `FULL_COMMANDS` and `PluginHelper.PLUGIN_CONSUMING_BUILTINS`
encode related but separately-maintained knowledge about which commands need
full initialization. A new command added to one but not the other will silently
break the invariant. Worth a cross-reference comment to prevent drift.
```suggestion
// the commands that need all the commands of the CLI: the shell and the
completion script use the others, and the
// plugins, which may add to any command, are loaded for run, export and
cmd (see PluginHelper.shouldDiscoverPlugins)
// NOTE: overlaps with PluginHelper.PLUGIN_CONSUMING_BUILTINS — keep
both in sync when adding commands
private static final Set<String> FULL_COMMANDS = Set.of("shell",
"completion", "run", "export", "cmd");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
