Skip to content

Commit

Permalink
Style error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
ajalt committed Apr 22, 2023
1 parent b0f6733 commit fb8dddf
Show file tree
Hide file tree
Showing 27 changed files with 192 additions and 130 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,8 @@ abstract class CliktCommand(
return ctx.helpFormatter.formatHelp(
ctx,
err,
commandHelp,
commandHelpEpilog,
cmd.commandHelp,
cmd.commandHelpEpilog,
cmd.allHelpParams(),
programName
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.github.ajalt.clikt.core

import com.github.ajalt.clikt.output.Localization
import com.github.ajalt.clikt.output.ParameterFormatter
import com.github.ajalt.clikt.parameters.arguments.Argument
import com.github.ajalt.clikt.parameters.options.Option
import com.github.ajalt.clikt.parameters.options.longestName
Expand Down Expand Up @@ -28,7 +29,8 @@ open class CliktError(
*
* @property error If true, execution should halt with an error. Otherwise, execution halt with no error code.
*/
class PrintHelpMessage(val command: CliktCommand, val error: Boolean = false) : CliktError(printError = false)
class PrintHelpMessage(val command: CliktCommand, val error: Boolean = false) :
CliktError(printError = false)

/**
* An exception that indicates that a message should be printed.
Expand Down Expand Up @@ -83,7 +85,9 @@ open class UsageError(
constructor(option: Option, statusCode: Int = 1)
: this(null, option.longestName(), statusCode)

open fun formatMessage(localization: Localization): String = message ?: ""
open fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return message ?: ""
}

/**
* The context of the command that raised this error.
Expand All @@ -107,12 +111,14 @@ class MultiUsageError(
fun buildOrNull(errors: List<UsageError>): UsageError? = when (errors.size) {
0 -> null
1 -> errors[0]
else -> MultiUsageError(errors.flatMap { (it as? MultiUsageError)?.errors ?: listOf(it) })
else -> MultiUsageError(errors.flatMap {
(it as? MultiUsageError)?.errors ?: listOf(it)
})
}
}

override fun formatMessage(localization: Localization): String {
return errors.joinToString("\n") { it.formatMessage(localization) }
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return errors.joinToString("\n") { it.formatMessage(localization, formatter) }
}
}

Expand All @@ -125,9 +131,9 @@ class BadParameterValue : UsageError {
constructor(message: String, argument: Argument) : super(message, argument)
constructor(message: String, option: Option) : super(message, option)

override fun formatMessage(localization: Localization): String {
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
val m = message.takeUnless { it.isNullOrBlank() }
val p = paramName?.takeIf { it.isNotBlank() }
val p = paramName?.takeIf { it.isNotBlank() }?.let(formatter)

return when {
m == null && p == null -> localization.badParameter()
Expand All @@ -141,21 +147,28 @@ class BadParameterValue : UsageError {

/** A required option was not provided */
class MissingOption(option: Option) : UsageError(option) {
override fun formatMessage(localization: Localization) = localization.missingOption(paramName ?: "")
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.missingOption(paramName?.let(formatter) ?: "")
}
}

/** A required argument was not provided */
class MissingArgument(argument: Argument) : UsageError(argument) {
override fun formatMessage(localization: Localization) = localization.missingArgument(paramName ?: "")
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.missingArgument(paramName?.let(formatter) ?: "")
}
}

/** A subcommand was provided that does not exist. */
class NoSuchSubcommand(
paramName: String,
private val possibilities: List<String> = emptyList(),
) : UsageError(null, paramName) {
override fun formatMessage(localization: Localization): String {
return localization.noSuchSubcommand(paramName ?: "", possibilities)
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.noSuchSubcommand(
paramName?.let(formatter) ?: "",
possibilities.map(formatter)
)
}
}

Expand All @@ -165,8 +178,11 @@ class NoSuchOption(
paramName: String,
private val possibilities: List<String> = emptyList(),
) : UsageError(null, paramName) {
override fun formatMessage(localization: Localization): String {
return localization.noSuchOption(paramName ?: "", possibilities)
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.noSuchOption(
paramName?.let(formatter) ?: "",
possibilities.map(formatter)
)
}
}

Expand All @@ -178,8 +194,8 @@ class IncorrectOptionValueCount(
) : UsageError(null, paramName) {
constructor(option: Option, paramName: String) : this(option.nvalues.first, paramName)

override fun formatMessage(localization: Localization): String {
return localization.incorrectOptionValueCount(paramName ?: "", minValues)
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.incorrectOptionValueCount(paramName?.let(formatter) ?: "", minValues)
}
}

Expand All @@ -190,8 +206,8 @@ class IncorrectArgumentValueCount(
) : UsageError(argument) {
constructor(argument: Argument) : this(argument.nvalues, argument)

override fun formatMessage(localization: Localization): String {
return localization.incorrectArgumentValueCount(paramName ?: "", nvalues)
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.incorrectArgumentValueCount(paramName?.let(formatter) ?: "", nvalues)
}
}

Expand All @@ -202,17 +218,20 @@ class MutuallyExclusiveGroupException(
require(names.size > 1) { "must provide at least two names" }
}

override fun formatMessage(localization: Localization): String {
return localization.mutexGroupException(names.first(), names.drop(1))
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.mutexGroupException(
names.first().let(formatter),
names.drop(1).map(formatter)
)
}
}

/** A required configuration file was not found. */
class FileNotFound(
val filename: String,
) : UsageError(null) {
override fun formatMessage(localization: Localization): String {
return localization.fileNotFound(filename)
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return localization.fileNotFound(filename.let(formatter))
}
}

Expand All @@ -222,10 +241,10 @@ class InvalidFileFormat(
message: String,
private val lineno: Int? = null,
) : UsageError(message) {
override fun formatMessage(localization: Localization): String {
override fun formatMessage(localization: Localization, formatter: ParameterFormatter): String {
return when (lineno) {
null -> localization.invalidFileFormat(filename, message!!)
else -> localization.invalidFileFormat(filename, lineno, message!!)
null -> localization.invalidFileFormat(filename.let(formatter), message!!)
else -> localization.invalidFileFormat(filename.let(formatter), lineno, message!!)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,5 @@ interface HelpFormatter {
}
}

/** A function that formats a parameter name within an error message */
typealias ParameterFormatter = (String) -> String
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,32 @@ interface Localization {
fun badParameterWithMessage(message: String) = "Invalid value: $message"

/** Message for [BadParameterValue] */
fun badParameterWithParam(paramName: String) = "Invalid value for \"$paramName\""
fun badParameterWithParam(paramName: String) = "Invalid value for $paramName"

/** Message for [BadParameterValue] */
fun badParameterWithMessageAndParam(paramName: String, message: String) =
"Invalid value for \"$paramName\": $message"
"Invalid value for $paramName: $message"

/** Message for [MissingOption] */
fun missingOption(paramName: String) = "Missing option \"$paramName\""
fun missingOption(paramName: String) = "Missing option $paramName"

/** Message for [MissingArgument] */
fun missingArgument(paramName: String) = "Missing argument \"$paramName\""
fun missingArgument(paramName: String) = "Missing argument $paramName"

/** Message for [NoSuchSubcommand] */
fun noSuchSubcommand(name: String, possibilities: List<String>): String {
return "no such subcommand: \"$name\"" + when (possibilities.size) {
return "no such subcommand: $name" + when (possibilities.size) {
0 -> ""
1 -> ". Did you mean \"${possibilities[0]}\"?"
1 -> ". Did you mean ${possibilities[0]}?"
else -> possibilities.joinToString(prefix = ". (Possible subcommands: ", postfix = ")")
}
}

/** Message for [NoSuchOption] */
fun noSuchOption(name: String, possibilities: List<String>): String {
return "no such option: \"$name\"" + when (possibilities.size) {
return "no such option: $name" + when (possibilities.size) {
0 -> ""
1 -> ". Did you mean \"${possibilities[0]}\"?"
1 -> ". Did you mean ${possibilities[0]}?"
else -> possibilities.joinToString(prefix = ". (Possible options: ", postfix = ")")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,22 @@ open class MordantHelpFormatter(
if (parameters.isNotEmpty()) add(renderParameters(context, parameters))
if (epilog.isNotEmpty()) add(renderEpilog(context, epilog))
} else {
add(renderError(context, error))
add(renderError(context, parameters, error))
}
}

protected open fun renderError(context: Context, error: UsageError): Widget {
protected open fun renderError(
context: Context,
parameters: List<ParameterHelp>,
error: UsageError,
): Widget {
return Text(buildString {
val errors = (error as? MultiUsageError)?.errors ?: listOf(error)
for ((i, e) in errors.withIndex()) {
if (i > 0) appendLine()
append(context.localization.usageError()).append(" ")
append(e.formatMessage(context.localization))
append(styleError(context, context.localization.usageError()))
append(" ")
append(e.formatMessage(context.localization, parameterFormatter(context)))
}
})
}
Expand Down Expand Up @@ -165,7 +170,10 @@ open class MordantHelpFormatter(
if (it.secondaryNames.isNotEmpty()) names += joinNamesForOption(
context, it.secondaryNames
)
DefinitionRow(col1 = names.joinToString(" / ", postfix = renderOptionValue(context, it)),
DefinitionRow(col1 = names.joinToString(
" / ",
postfix = renderOptionValue(context, it)
),
col2 = renderParameterHelpText(context, it.help, it.tags),
marker = when (HelpFormatter.Tags.REQUIRED) {
in it.tags -> requiredOptionMarker?.let { m ->
Expand Down Expand Up @@ -285,11 +293,18 @@ open class MordantHelpFormatter(
protected open fun styleUsageTitle(context: Context, title: String): String =
context.terminal.theme.style("warning")(title)

protected open fun styleError(context: Context, title: String): String =
context.terminal.theme.style("danger")(title)

protected open fun styleMetavar(context: Context, metavar: String): String {
val style = context.terminal.theme.style("warning") + context.terminal.theme.style("muted")
return style(metavar)
}

protected open fun parameterFormatter(context: Context): ParameterFormatter {
return { styleOptionName(context, it) }
}

protected open fun renderOptionValue(context: Context, option: ParameterHelp.Option): String {
if (option.metavar == null) return ""
var prefix = "="
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,27 @@ fun <T : Any> NullableOption<T, T>.prompt(
) { input ->
val ctx = OptionCallTransformContext("", this@transformAll, context)
try {
val v = transformAll(listOf(transformEach(ctx, listOf(transformValue(ctx, input)))))
val v =
transformAll(listOf(transformEach(ctx, listOf(transformValue(ctx, input)))))
if (v != null) {
@Suppress("UNCHECKED_CAST")
(option as? OptionWithValues<T, T, T>)?.transformValidator?.invoke(this@transformAll, v)
(option as? OptionWithValues<T, T, T>)?.transformValidator?.invoke(
this@transformAll,
v
)
}
ConversionResult.Valid(v)
} catch (e: UsageError) {
e.context = e.context ?: context
ConversionResult.Invalid(e.formatMessage(context.localization))
ConversionResult.Invalid(e.formatMessage(context.localization) { it })
}
}
}

else -> provided
} ?: throw Abort()
}

// the stdlib capitalize was deprecated without a replacement
private fun String.capitalize2(): String = replaceFirstChar { if (it.isLowerCase()) it.titlecase() else it.toString() }
private fun String.capitalize2(): String =
replaceFirstChar { if (it.isLowerCase()) it.titlecase() else it.toString() }
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ class CliktCommandTest {
@Test
@JsName("invokeWithoutSubcommand_true")
fun `invokeWithoutSubcommand=true`() {
TestCommand(called = true, invokeWithoutSubcommand = true).subcommands(TestCommand(called = false)).apply {
TestCommand(
called = true,
invokeWithoutSubcommand = true
).subcommands(TestCommand(called = false)).apply {
parse("")
currentContext.invokedSubcommand shouldBe null
}
Expand Down Expand Up @@ -137,10 +140,27 @@ class CliktCommandTest {
}.let { p.getFormattedHelp(it) } shouldBe """
|Usage: parent [OPTIONS] ARG
|
|Error: Missing argument "ARG"
|Error: Missing argument ARG
""".trimMargin()
}

@Test
@JsName("command_usage_exit_code")
fun `command usage exit code`() {
class C : TestCommand() {
override fun run_() {
throw ProgramResult(-1)
}
}

val p = C()
val e = shouldThrow<ProgramResult> {
p.parse("")
}
e.statusCode shouldBe -1
p.getFormattedHelp(e) shouldBe null
}

@Test
@JsName("command_toString")
fun `command toString`() {
Expand Down Expand Up @@ -272,6 +292,6 @@ class CliktCommandTest {
}
shouldThrow<NoSuchOption> {
C(false).parse("-fgi")
}.formattedMessage shouldBe "no such option: \"-g\""
}.formattedMessage shouldBe "no such option: -g"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -697,8 +697,8 @@ class MordantHelpFormatterTest {
TestCommand().test("--foo --bar").stderr shouldBe """
|Usage: test [OPTIONS]
|
|Error: no such option: "--foo"
|Error: no such option: "--bar"
|Error: no such option: --foo
|Error: no such option: --bar
|
""".trimMargin()
}
Expand Down

0 comments on commit fb8dddf

Please sign in to comment.