Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finalize eager options before any commands #547

Merged
merged 1 commit into from
Sep 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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`() {
Expand Down Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion clikt/api/clikt.api
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down
Loading