From e914dc144bdae58fd404a74d6bc1ced1f3778a25 Mon Sep 17 00:00:00 2001 From: AJ Alt Date: Tue, 27 Dec 2022 11:38:33 -0800 Subject: [PATCH] Support environment variables for options in a mutually exclusive group (#385) --- CHANGELOG.md | 3 ++- .../groups/CoOccurringOptionGroup.kt | 8 ++---- .../groups/MutuallyExclusiveOption.kt | 15 ++++++++--- .../ajalt/clikt/parameters/options/Option.kt | 26 +++++++++++++++++-- .../clikt/parameters/EnvvarOptionsTest.kt | 26 ++++++++++++++++++- .../parameters/groups/OptionGroupsTest.kt | 4 +-- 6 files changed, 67 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af50bb3e3..d8f47b93a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,11 @@ ## Unreleased ### Changed -- Updated Kotlin to 1.6.20 +- Updated Kotlin to 1.7.20 ### Fixed - Support unicode in environment variable values on Native Windows. ([#362](https://github.com/ajalt/clikt/issues/362)) +- Support environment variables for options in a mutually exclusive options group. ([#384](https://github.com/ajalt/clikt/issues/384)) ## 3.5.0 ### Added diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/CoOccurringOptionGroup.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/CoOccurringOptionGroup.kt index f668b2eb3..2e5e0450e 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/CoOccurringOptionGroup.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/CoOccurringOptionGroup.kt @@ -4,10 +4,7 @@ import com.github.ajalt.clikt.core.CliktCommand import com.github.ajalt.clikt.core.Context import com.github.ajalt.clikt.output.HelpFormatter import com.github.ajalt.clikt.parameters.internal.NullableLateinit -import com.github.ajalt.clikt.parameters.options.FinalValue -import com.github.ajalt.clikt.parameters.options.Option -import com.github.ajalt.clikt.parameters.options.OptionWithValues -import com.github.ajalt.clikt.parameters.options.getFinalValue +import com.github.ajalt.clikt.parameters.options.* import com.github.ajalt.clikt.parsers.OptionParser import kotlin.properties.ReadOnlyProperty import kotlin.reflect.KProperty @@ -37,8 +34,7 @@ class CoOccurringOptionGroup internal constructor( override fun finalize(context: Context, invocationsByOption: Map>) { occurred = invocationsByOption.isNotEmpty() || group.options.any { - // Also trigger the group if any of the options have values from envvars or value sources - it is OptionWithValues<*, *, *> && it.getFinalValue(context, emptyList(), it.envvar) !is FinalValue.Parsed + it.hasEnvvarOrSourcedValue(context, invocationsByOption[it] ?: emptyList()) } if (occurred) group.finalize(context, invocationsByOption) value = transform(occurred, group, context) diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/MutuallyExclusiveOption.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/MutuallyExclusiveOption.kt index 69da79d7a..fc3db8604 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/MutuallyExclusiveOption.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/MutuallyExclusiveOption.kt @@ -41,9 +41,14 @@ class MutuallyExclusiveOptions internal constructor( override fun getValue(thisRef: CliktCommand, property: KProperty<*>): OutT = value - override fun finalize(context: Context, invocationsByOption: Map>) { + override fun finalize( + context: Context, + invocationsByOption: Map>, + ) { finalizeOptions(context, options, invocationsByOption) - val values = options.filter { it in invocationsByOption }.mapNotNull { it.value } + val values = options.filter { + it in invocationsByOption || it.hasEnvvarOrSourcedValue(context, emptyList()) + }.mapNotNull { it.value } value = MutuallyExclusiveOptionTransformContext(context).transformAll(values) } @@ -108,7 +113,11 @@ fun ParameterHolder.mutuallyExclusiveOptions( name: String? = null, help: String? = null, ): MutuallyExclusiveOptions { - return MutuallyExclusiveOptions(listOf(option1, option2) + options, name, help) { it.lastOrNull() } + return MutuallyExclusiveOptions( + listOf(option1, option2) + options, + name, + help + ) { it.lastOrNull() } } /** diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt index 1285d5e72..b667efbde 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt @@ -48,7 +48,8 @@ interface Option { /** Information about this option for the help output. */ fun parameterHelp(context: Context): HelpFormatter.ParameterHelp.Option? = when { hidden -> null - else -> HelpFormatter.ParameterHelp.Option(names, + else -> HelpFormatter.ParameterHelp.Option( + names, secondaryNames, metavar(context), optionHelp, @@ -86,7 +87,10 @@ interface OptionDelegate : GroupableOption, ReadOnlyProperty): ReadOnlyProperty + operator fun provideDelegate( + thisRef: ParameterHolder, + prop: KProperty<*>, + ): ReadOnlyProperty override fun getValue(thisRef: ParameterHolder, property: KProperty<*>): T = value } @@ -157,12 +161,30 @@ internal fun Option.getFinalValue( context.readEnvvarBeforeValueSource -> { readEnvVar(context, envvar) ?: readValueSource(context) } + else -> { readValueSource(context) ?: readEnvVar(context, envvar) } } ?: FinalValue.Parsed(emptyList()) } +// This is a pretty ugly hack: option groups need to enforce their contraints, including on options +// from envvars/value sources, but not including default values. Unfortunately, we don't know +// whether an option's value is from a default or envvar. So we do some ugly casts and read the +// final value again to check for values from other sources. +internal fun Option.hasEnvvarOrSourcedValue( + context: Context, + invocations: List, +): Boolean { + val envvar = when (this) { + is OptionWithValues<*, *, *> -> envvar + is FlagOption<*> -> envvar + else -> null + } + val final = this.getFinalValue(context, invocations, envvar) + return final !is FinalValue.Parsed +} + private fun Option.readValueSource(context: Context): FinalValue? { return context.valueSource?.getValues(context, this)?.ifEmpty { null } ?.let { FinalValue.Sourced(it) } diff --git a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/EnvvarOptionsTest.kt b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/EnvvarOptionsTest.kt index 97e799c6e..05ebea48b 100644 --- a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/EnvvarOptionsTest.kt +++ b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/EnvvarOptionsTest.kt @@ -1,15 +1,19 @@ package com.github.ajalt.clikt.parameters import com.github.ajalt.clikt.core.CliktCommand +import com.github.ajalt.clikt.core.MutuallyExclusiveGroupException import com.github.ajalt.clikt.core.context import com.github.ajalt.clikt.core.subcommands import com.github.ajalt.clikt.parameters.groups.OptionGroup import com.github.ajalt.clikt.parameters.groups.cooccurring +import com.github.ajalt.clikt.parameters.groups.mutuallyExclusiveOptions +import com.github.ajalt.clikt.parameters.groups.single import com.github.ajalt.clikt.parameters.options.* import com.github.ajalt.clikt.parameters.types.int import com.github.ajalt.clikt.testing.TestCommand import com.github.ajalt.clikt.testing.TestSource import com.github.ajalt.clikt.testing.parse +import io.kotest.assertions.throwables.shouldThrow import io.kotest.data.blocking.forAll import io.kotest.data.row import io.kotest.matchers.shouldBe @@ -180,7 +184,7 @@ class EnvvarOptionsTest { } @Test - @JsName("option_group_envvar") + @JsName("cooccurring_option_group_envvar") fun `cooccurring option group envvar`() = forAll( row("", "xx", "yy"), row("--x=z", "z", "yy"), @@ -205,4 +209,24 @@ class EnvvarOptionsTest { C().withEnv().parse(argv) } + + @Test + @JsName("mutually_exclusive_option_group_envvar") + fun `mutually exclusive option group envvar`() { + class C: TestCommand() { + val opt by mutuallyExclusiveOptions( + option("--foo", envvar = "FOO"), + option("--bar", envvar = "BAR"), + ).single() + } + C().withEnv().parse("--bar=x").opt shouldBe "x" + + env["FOO"] = "y" + C().withEnv().parse("").opt shouldBe "y" + + env["BAR"] = "z" + shouldThrow { + C().withEnv().parse("") + } + } } diff --git a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/groups/OptionGroupsTest.kt b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/groups/OptionGroupsTest.kt index a967e6b94..b80bd4dcc 100644 --- a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/groups/OptionGroupsTest.kt +++ b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/groups/OptionGroupsTest.kt @@ -211,8 +211,8 @@ class OptionGroupsTest { @Test @JsName("mutually_exclusive_group_default_flag_single") fun `mutually exclusive group flag single`() = forAll( -// row("", false), -// row("--x", true), + row("", null), + row("--x", true), row("--y", true) ) { argv, eg -> class C : TestCommand() {