clikt icon indicating copy to clipboard operation
clikt copied to clipboard

Include parameter name when ValueSource value is invalid

Open RafeArnold opened this issue 4 months ago • 1 comments

When an invalid value is provided via the CLI or environment variables, the name of the parameter is provided in the UsageError. This results in rendered errors like Error: invalid value for --the-integer: foo is not a valid integer or Error: invalid value for MY_COMMAND_THE_INTEGER: foo is not a valid integer. However, when the value is provided by a ValueSource, the parameter name is omitted from the error, so the errors look like Error: invalid value: foo is not a valid integer. This is not very helpful for the user of the CLI tool to diagnose which parameter is the problem and where it's located. A simple example to reproduce this behaviour:

import com.github.ajalt.clikt.core.NoOpCliktCommand
import com.github.ajalt.clikt.core.main
import com.github.ajalt.clikt.parameters.options.option
import com.github.ajalt.clikt.parameters.types.int
import com.github.ajalt.clikt.sources.PropertiesValueSource
import kotlin.io.path.createTempFile
import kotlin.io.path.writeText

fun main() {
  class C : NoOpCliktCommand() {
    val theInteger by option().int()
  }
  val propertiesFile = createTempFile()
  propertiesFile.writeText("""the-integer=foo""")
  val envVars = emptyMap<String, String>()
//  val envVars = mapOf("MY_COMMAND_THE_INTEGER" to "foo")
  val argv = emptyList<String>()
//  val argv = listOf("--the-integer", "foo")
  C()
    .apply {
      configureContext {
        readEnvvar = { envVars[it] }
        autoEnvvarPrefix = "MY_COMMAND"
        valueSources(PropertiesValueSource.from(propertiesFile))
      }
    }
    .main(argv)
}

This behaviour is due to fun Option.readValueSource(context: Context): List<OptionInvocation>? providing an empty name argument to the OptionInvocation constructor. I suggest adding a property to ValueSource.Invocation alongside values that would be used as the name argument. This property could default to an empty string or null for backwards compatibility but would allow ValueSources to opt in to more expressive errors if desired.

Happy to submit a PR for this if deemed a good addition.

RafeArnold avatar Oct 14 '25 10:10 RafeArnold

Good idea!

ajalt avatar Oct 14 '25 15:10 ajalt