Finalize eager options before any commands (#547)

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.

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<PrintHelpMessage> {
+            p.subcommands(Child().subcommands(Grandchild()))
+                .parse("child grandchild --help")
+        }.let { p.getFormattedHelp(it) } shouldBe """
+            |Usage: parent child grandchild [<options>]
+            |
+            |Options:
+            |  --foo=<text>
+            |  -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"

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;

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 <T : BaseCliktCommand<T>> parse(command: T, argv: List<String>): CommandLineParseResult<T> {
         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() {

clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Invocation.kt

@@ -95,8 +95,8 @@ class CommandLineParseResult<T : BaseCliktCommand<T>>(
  * ```
  *
  * @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 <T : BaseCliktCommand<T>> CommandInvocation<T>.flatten(finalize: Boolean = true): FlatInvocations<T> {
     return FlatInvocations(this, finalize)
@@ -116,9 +116,21 @@ class FlatInvocations<T : BaseCliktCommand<T>> internal constructor(
             closables.removeLast().close()
         }
         yieldSubs(root)
-    }.onEach { if (finalize) CommandLineParser.finalize(it) }
+    }
+
 
-    override fun iterator(): Iterator<CommandInvocation<T>> = seq.iterator()
+    override fun iterator(): Iterator<CommandInvocation<T>> {
+        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.