From bdcf22e676bbe9e79bad6da4b7c6eb20e8d550c6 Mon Sep 17 00:00:00 2001 From: AJ Date: Sat, 14 Sep 2024 15:14:37 -0700 Subject: [PATCH] Finalize eager options before any commands --- CHANGELOG.md | 1 + .../ajalt/clikt/parameters/SubcommandTest.kt | 34 ++++++++++-- clikt/api/clikt.api | 3 +- .../ajalt/clikt/parsers/CommandLineParser.kt | 53 +++++++++++++------ .../github/ajalt/clikt/parsers/Invocation.kt | 20 +++++-- 5 files changed, 86 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ed1bc45..ec07c7b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ - **Breaking Change:** `Context.obj` and `Context.terminal`, and `OptionTransformContext.terminal` are now extension functions rather than properties. - **Breaking Change:** The `RenderedSection` and `DefinitionRow` classes have moved to `AbstractHelpFormatter`. - Updated Kotlin to 2.0.0 +- Support calling `--help` on subcommands when parents have required parameters. ### Fixed - Fixed excess arguments not being reported when `allowMultipleSubcommands=true` and a subcommand has excess arguments followed by another subcommand. diff --git a/clikt-mordant/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/SubcommandTest.kt b/clikt-mordant/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/SubcommandTest.kt index c54f45af..66df42e3 100644 --- a/clikt-mordant/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/SubcommandTest.kt +++ b/clikt-mordant/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/SubcommandTest.kt @@ -23,7 +23,7 @@ import io.kotest.matchers.string.shouldContain import kotlin.js.JsName import kotlin.test.Test -@Suppress("BooleanLiteralArgument") +@Suppress("BooleanLiteralArgument", "unused") class SubcommandTest { @Test fun subcommand() = forAll( @@ -267,6 +267,32 @@ class SubcommandTest { """.trimMargin() } + @Test + @JsName("subcommand_help_with_required_parent") + fun `subcommand help with required parent`() { + class Parent : TestCommand() { + val o by option().required() + } + class Child : TestCommand() { + val o by option().required() + } + class Grandchild : TestCommand(called = false) { + val foo by option() + } + + val p = Parent() + shouldThrow { + p.subcommands(Child().subcommands(Grandchild())) + .parse("child grandchild --help") + }.let { p.getFormattedHelp(it) } shouldBe """ + |Usage: parent child grandchild [] + | + |Options: + | --foo= + | -h, --help Show this message and exit + """.trimMargin() + } + @Test @JsName("subcommandprintHelpOnEmptyArgs__true") fun `subcommand printHelpOnEmptyArgs = true`() { @@ -342,10 +368,12 @@ class SubcommandTest { @Test @JsName("multiple_subcommands_optional_sub_arg") fun `multiple subcommands optional sub arg`() { - class Sub: TestCommand(count = 2) { + class Sub : TestCommand(count = 2) { val a by argument().optional() } - class C: TestCommand(allowMultipleSubcommands = true) + + class C : TestCommand(allowMultipleSubcommands = true) + val sub = Sub() C().subcommands(sub).parse("sub sub b") sub.a shouldBe "b" diff --git a/clikt/api/clikt.api b/clikt/api/clikt.api index feb8d996..a7206d0b 100644 --- a/clikt/api/clikt.api +++ b/clikt/api/clikt.api @@ -1349,7 +1349,8 @@ public final class com/github/ajalt/clikt/parsers/CommandLineParseResult { public final class com/github/ajalt/clikt/parsers/CommandLineParser { public static final field INSTANCE Lcom/github/ajalt/clikt/parsers/CommandLineParser; - public final fun finalize (Lcom/github/ajalt/clikt/parsers/CommandInvocation;)V + public final fun finalizeCommand (Lcom/github/ajalt/clikt/parsers/CommandInvocation;)V + public final fun finalizeEagerOptions (Lcom/github/ajalt/clikt/parsers/CommandInvocation;)V public final fun main (Lcom/github/ajalt/clikt/core/BaseCliktCommand;Lkotlin/jvm/functions/Function1;)V public final fun mainReturningValue (Lcom/github/ajalt/clikt/core/BaseCliktCommand;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; public final fun parse (Lcom/github/ajalt/clikt/core/BaseCliktCommand;Ljava/util/List;)Lcom/github/ajalt/clikt/parsers/CommandLineParseResult; diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/CommandLineParser.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/CommandLineParser.kt index 0445a95e..0523e32f 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/CommandLineParser.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/CommandLineParser.kt @@ -78,7 +78,7 @@ object CommandLineParser { } /** - * [Finalize][finalize] and [run][runCommand] all invoked commands. + * [Finalize][finalizeCommand] and [run][runCommand] all invoked commands. * * @throws CliktError if an error occurred while parsing or of any occur while finalizing or * running the commands. @@ -100,38 +100,48 @@ object CommandLineParser { * This function does not throw exceptions. If parsing errors occur, they will be in the returned * result. * - * This function does not [run] the command or [finalize] the invocations. + * This function does not [run] the command or [finalizeCommand] the invocations. */ fun > parse(command: T, argv: List): CommandLineParseResult { return parseArgv(command, argv) } - /** - * Finalize a command invocation, converting and setting the values for all options and other - * parameters. This function does not [run] the command. + * Finalize eager options for a command invocation, running them if they were invoked. * - * @throws CliktError If the [invocation] had any errors or if any parameters fail to finalize, - * such as if a required option is missing or a value could not be converted. + * This does not finalize any other parameters. + * + * @throws CliktError If any of the eager options were invoked and throw an error like + * [PrintHelpMessage]. */ - fun finalize(invocation: CommandInvocation<*>) { + fun finalizeEagerOptions(invocation: CommandInvocation<*>) { val command = invocation.command val context = command.currentContext - val groups = command.registeredParameterGroups() - val arguments = command.registeredArguments() - throwCompletionMessageIfRequested(context, command) - val (eagerOpts, nonEagerOpts) = command.registeredOptions() - .partition { it.eager } - - val (eagerInvs, nonEagerInvs) = invocation.optionInvocations.entries - .partition { it.key.eager } - .toList().map { it.associate { (k, v) -> k to v } } + val (eagerOpts, _) = getOpts(command) + val (eagerInvs, _) = getInvs(invocation) // finalize and validate eager options first; unlike other options, eager options only get // validated if they're invoked finalizeOptions(context, eagerOpts, eagerInvs) validateParameters(context, eagerInvs.keys).throwErrors() + } + + /** + * Finalize a command invocation, converting and setting the values for all options and other + * parameters. This function does not [finalizeEagerOptions] or [run] the command. + * + * @throws CliktError If the [invocation] had any errors or if any parameters fail to finalize, + * such as if a required option is missing or a value could not be converted. + */ + fun finalizeCommand(invocation: CommandInvocation<*>) { + val command = invocation.command + val context = command.currentContext + val groups = command.registeredParameterGroups() + val arguments = command.registeredArguments() + + val (_, nonEagerOpts) = getOpts(command) + val (_, nonEagerInvs) = getInvs(invocation) // throw any parse errors after the eager options are finalized invocation.throwErrors() @@ -154,6 +164,15 @@ object CommandLineParser { context.invokedSubcommands += invocation.subcommandInvocations.map { it.command } } + + private fun getInvs(invocation: CommandInvocation<*>) = + invocation.optionInvocations.entries + .partition { it.key.eager } + .toList().map { it.associate { (k, v) -> k to v } } + + private fun getOpts(command: BaseCliktCommand<*>) = + command.registeredOptions() + .partition { it.eager } } private fun CommandInvocation<*>.throwErrors() { diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Invocation.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Invocation.kt index 68591785..23fd6187 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Invocation.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Invocation.kt @@ -95,8 +95,8 @@ class CommandLineParseResult>( * ``` * * @param finalize If true (the default), finalize all commands as they are emitted in the sequence. - * If false, you must call [CommandLineParser.finalize] on each invocation yourself before running - * the command. + * If false, you must call [CommandLineParser.finalizeEagerOptions] and + * [CommandLineParser.finalizeCommand] on each invocation yourself before running the command. */ fun > CommandInvocation.flatten(finalize: Boolean = true): FlatInvocations { return FlatInvocations(this, finalize) @@ -116,9 +116,21 @@ class FlatInvocations> internal constructor( closables.removeLast().close() } yieldSubs(root) - }.onEach { if (finalize) CommandLineParser.finalize(it) } + } + - override fun iterator(): Iterator> = seq.iterator() + override fun iterator(): Iterator> { + return when { + finalize -> sequence { + // Finalize eager options of all commands first so that you can call --help on + // subcommands even if the parent has required parameters + seq.iterator().forEach(CommandLineParser::finalizeEagerOptions) + yieldAll(seq.onEach { CommandLineParser.finalizeCommand(it) }) + } + + else -> seq + }.iterator() + } /** * [Close][Context.close] all open contexts of invoked commands.