From 88d4dcb99fb4343aeb091880860ce594d0b487ca Mon Sep 17 00:00:00 2001 From: AJ Date: Sat, 30 Mar 2024 11:18:56 -0700 Subject: [PATCH 01/43] Add NoSuchArgument --- CHANGELOG.md | 2 ++ .../com/github/ajalt/clikt/core/exceptions.kt | 33 ++++++++++++++++--- .../com/github/ajalt/clikt/parsers/Parser.kt | 14 +++----- .../ajalt/clikt/parameters/ArgumentTest.kt | 6 ++-- .../ajalt/clikt/parameters/OptionTest.kt | 2 +- 5 files changed, 39 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8197eae..8cc0d9e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog ## Unreleased +### Added +- Added `NoSuchArgument` exception that is thrown when too many arguments were given on the command line. Previously, a less specific `UsageError` was thrown instead. ## 4.4.0 ### Added - Publish `linuxArm64` and `wasmJs` targets. diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/exceptions.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/exceptions.kt index 73e6b1e9..18d93a91 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/exceptions.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/exceptions.kt @@ -147,9 +147,16 @@ class MultiUsageError( fun buildOrNull(errors: List): UsageError? = when (errors.size) { 0 -> null 1 -> errors[0] - else -> MultiUsageError(errors.flatMap { - (it as? MultiUsageError)?.errors ?: listOf(it) - }) + else -> { + val flatErrors = errors.flatMap { (it as? MultiUsageError)?.errors ?: listOf(it) } + var encounteredUnknownOpt = false + // Don't report argument errors after unknown options since they might be the values + // for that option + MultiUsageError(flatErrors.filter { + if (it is NoSuchOption) encounteredUnknownOpt = true + !encounteredUnknownOpt || it !is NoSuchArgument + }) + } } } @@ -230,7 +237,6 @@ class NoSuchSubcommand( } } - /** An option was provided that does not exist. */ class NoSuchOption( paramName: String, @@ -244,6 +250,25 @@ class NoSuchOption( } } +/** + * One or more arguments were provided that do not exist. + */ +class NoSuchArgument( + private val excessArguments: List, +) : UsageError(null) { + init { + require(excessArguments.isNotEmpty()) { "must provide at least one excess argument" } + } + + override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String { + val actual = excessArguments.joinToString(" ", limit = 3, prefix = "(", postfix = ")") + return when (excessArguments.size) { + 1 -> localization.extraArgumentOne(actual) + else -> localization.extraArgumentMany(actual, excessArguments.size) + } + } +} + /** An option was supplied but the number of values supplied to the option was incorrect. */ class IncorrectOptionValueCount( diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Parser.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Parser.kt index 8bd60c21..050faa97 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Parser.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parsers/Parser.kt @@ -8,7 +8,7 @@ import com.github.ajalt.clikt.parameters.options.Option import com.github.ajalt.clikt.parameters.options.splitOptionPrefix /** [i] is the argv index of the token that caused the error */ -private data class Err(val e: UsageError, val i: Int, val includeInMulti: Boolean = true) +private data class Err(val e: UsageError, val i: Int) private data class ArgsParseResult( val excessCount: Int, @@ -302,7 +302,6 @@ internal object Parser { } val usageErrors = errors - .filter { it.includeInMulti }.ifEmpty { errors } .sortedBy { it.i }.mapTo(mutableListOf()) { it.e } nextArgvI = excessResult.first @@ -369,7 +368,7 @@ internal object Parser { ) } - else -> -1 to Err(excessArgsError(positionalArgs, excess, context), i, false) + else -> -1 to Err(excessArgsError(positionalArgs, excess, context), i) } } return i to null @@ -558,13 +557,8 @@ internal object Parser { excess: Int, context: Context, ): UsageError { - val actual = positionalArgs.takeLast(excess) - .joinToString(" ", limit = 3, prefix = "(", postfix = ")") { it.second } - val message = when (excess) { - 1 -> context.localization.extraArgumentOne(actual) - else -> context.localization.extraArgumentMany(actual, excess) - } - return UsageError(message).also { it.context = context } + val actual = positionalArgs.takeLast(excess).map { it.second } + return NoSuchArgument(actual).also { it.context = context } } } diff --git a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/ArgumentTest.kt b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/ArgumentTest.kt index 6dda101c..8af41685 100644 --- a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/ArgumentTest.kt +++ b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/ArgumentTest.kt @@ -161,9 +161,9 @@ class ArgumentTest { } shouldThrow { C().parse("foo") } .formattedMessage shouldBe "argument X requires 2 values" - shouldThrow { C().parse("foo bar baz") } + shouldThrow { C().parse("foo bar baz") } .formattedMessage shouldBe "got unexpected extra argument (baz)" - shouldThrow { C().parse("foo bar baz qux") } + shouldThrow { C().parse("foo bar baz qux") } .formattedMessage shouldBe "got unexpected extra arguments (baz qux)" } @@ -176,7 +176,7 @@ class ArgumentTest { shouldThrow { C().parse("foo bar") } .formattedMessage shouldBe "argument X requires 3 values" - shouldThrow { C().parse("foo bar baz qux") } + shouldThrow { C().parse("foo bar baz qux") } .formattedMessage shouldBe "got unexpected extra argument (qux)" } diff --git a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/OptionTest.kt b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/OptionTest.kt index 51d6a584..27d3514f 100644 --- a/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/OptionTest.kt +++ b/clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/OptionTest.kt @@ -233,7 +233,7 @@ class OptionTest { } shouldThrow { C().parse("-x") }.formattedMessage shouldBe "option -x requires 2 values" - shouldThrow { C().parse("--yy foo bar baz") }.formattedMessage shouldBe + shouldThrow { C().parse("--yy foo bar baz") }.formattedMessage shouldBe "got unexpected extra argument (baz)" } From 50b8cf1d95d39d74762764c72d4202806db32cf0 Mon Sep 17 00:00:00 2001 From: AJ Date: Sat, 30 Mar 2024 16:05:18 -0700 Subject: [PATCH 02/43] Separate parsing from execution --- .../github/ajalt/clikt/core/CliktCommand.kt | 7 +- .../com/github/ajalt/clikt/core/Context.kt | 36 +- .../clikt/execution/CommandLineRunner.kt | 41 ++ .../ajalt/clikt/internal/Finalization.kt | 23 +- .../github/ajalt/clikt/parsers/Invocation.kt | 20 + .../com/github/ajalt/clikt/parsers/Parser.kt | 19 +- .../com/github/ajalt/clikt/parsers/atfile.kt | 9 +- .../ajalt/clikt/tmp/CommandLineParser.kt | 39 ++ .../com/github/ajalt/clikt/tmp/NewParser.kt | 446 ++++++++++++++++++ .../github/ajalt/clikt/testing/TestCommand.kt | 2 +- 10 files changed, 594 insertions(+), 48 deletions(-) create mode 100644 clikt/src/commonMain/kotlin/com/github/ajalt/clikt/execution/CommandLineRunner.kt create mode 100644 clikt/src/commonMain/kotlin/com/github/ajalt/clikt/tmp/CommandLineParser.kt create mode 100644 clikt/src/commonMain/kotlin/com/github/ajalt/clikt/tmp/NewParser.kt diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/CliktCommand.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/CliktCommand.kt index 6bbcdd2b..7761f44d 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/CliktCommand.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/CliktCommand.kt @@ -52,7 +52,7 @@ abstract class CliktCommand( */ val printHelpOnEmptyArgs: Boolean = false, /** - * Extra information about this option to pass to the help formatter. + * Extra information about this command to pass to the help formatter. */ val helpTags: Map = emptyMap(), /** @@ -132,6 +132,11 @@ abstract class CliktCommand( private fun registeredOptionNames() = _options.flatMapTo(mutableSetOf()) { it.names } + fun resetContext(parent: Context?): Context { + TODO("implement this, doc that each run get a fresh context now") +// _context?.close() +// return createContext(...) + } private fun createContext( argv: List, parent: Context?, diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/Context.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/Context.kt index f3f2d297..7af5a08c 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/Context.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/Context.kt @@ -357,24 +357,24 @@ class Context private constructor( val formatter: ((Context) -> HelpFormatter) = helpFormatter ?: { MordantHelpFormatter(it) } return Context( - parent, - command, - interspersed, - allowGroupedShortOptions, - autoEnvvarPrefix, - printExtraMessages, - helpOptionNames.toSet(), - formatter, - tokenTransformer, - terminal, - argumentFileReader, - readEnvvarBeforeValueSource, - valueSource, - correctionSuggestor, - localization, - envvarReader, - obj, - argv + parent = parent, + command = command, + allowInterspersedArgs = interspersed, + allowGroupedShortOptions = allowGroupedShortOptions, + autoEnvvarPrefix = autoEnvvarPrefix, + printExtraMessages = printExtraMessages, + helpOptionNames = helpOptionNames.toSet(), + helpFormatter = formatter, + tokenTransformer = tokenTransformer, + terminal = terminal, + argumentFileReader = argumentFileReader, + readEnvvarBeforeValueSource = readEnvvarBeforeValueSource, + valueSource = valueSource, + correctionSuggestor = correctionSuggestor, + localization = localization, + readEnvvar = envvarReader, + obj = obj, + originalArgv = argv ) } } diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/execution/CommandLineRunner.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/execution/CommandLineRunner.kt new file mode 100644 index 00000000..2674e9ab --- /dev/null +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/execution/CommandLineRunner.kt @@ -0,0 +1,41 @@ +package com.github.ajalt.clikt.execution +// +//import com.github.ajalt.clikt.core.BaseCliktCommand +//import com.github.ajalt.clikt.core.CliktError +//import com.github.ajalt.clikt.mpp.exitProcessMpp +//import com.github.ajalt.clikt.tmp.CommandLineParser +// +//fun BaseCliktCommand<() -> Unit>.parse(argv: Array) { +// val result = CommandLineParser.parse(this, argv.asList()) +// for (invocation in result.invocations) { +// CommandLineParser.finalize(invocation) +// invocation.command.runner() +// } +//} +// +//fun BaseCliktCommand<() -> Unit>.main(argv: Array) { +// try { +// parse(argv) +// } catch (e: CliktError) { +// echoFormattedHelp(e) +// exitProcessMpp(e.statusCode) +// } +//} +// +//suspend fun BaseCliktCommand Unit>.parse(argv: Array) { +// val result = CommandLineParser.parse(this, argv.asList()) +// for (invocation in result.invocations) { +// CommandLineParser.finalize(invocation) +// invocation.command.runner() +// } +//} +// +//fun BaseCliktCommand<(T) -> T>.parse(initialValue: T, argv: Array): T { +// val result = CommandLineParser.parse(this, argv.asList()) +// var value = initialValue +// for (invocation in result.invocations) { +// CommandLineParser.finalize(invocation) +// value = invocation.command.runner(value) +// } +// return value +//} diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/internal/Finalization.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/internal/Finalization.kt index e165aa9e..dc6b6f87 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/internal/Finalization.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/internal/Finalization.kt @@ -1,12 +1,10 @@ package com.github.ajalt.clikt.internal -import com.github.ajalt.clikt.core.Abort -import com.github.ajalt.clikt.core.Context -import com.github.ajalt.clikt.core.MultiUsageError -import com.github.ajalt.clikt.core.UsageError +import com.github.ajalt.clikt.core.* import com.github.ajalt.clikt.parameters.arguments.Argument import com.github.ajalt.clikt.parameters.groups.ParameterGroup import com.github.ajalt.clikt.parameters.options.Option +import com.github.ajalt.clikt.parsers.ArgumentInvocation import com.github.ajalt.clikt.parsers.Invocation internal fun finalizeOptions( @@ -15,7 +13,7 @@ internal fun finalizeOptions( invocationsByOption: Map>, ) { finalizeParameters( - context, options, emptyList(), mapOf(null to invocationsByOption), emptyList() + context, options, emptyList(), invocationsByOption, emptyList() ) } @@ -28,23 +26,26 @@ internal fun finalizeParameters( context: Context, options: List