Print subcommand names in usage messages.

Fixes #47
ajalt · Jan 21, 2019 · 0a1589a · 0a1589a
1 parent 6b41c88
commit 0a1589a
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,8 @@
 
 ## [Unreleased]
 ### Fixed
-- Arguments with `multiple(required=true)` now report an error if no argument is given on the command line.
+- Usage errors now correctly print subcommand names. ([#47](https://github.com/ajalt/clikt/issues/47))
+- Arguments with `multiple(required=true)` now report an error if no argument is given on the command line. ([#36](https://github.com/ajalt/clikt/issues/36))
 
 ## [1.6.0] - 2018-12-02
 ### Added

diff --git a/clikt/src/main/kotlin/com/github/ajalt/clikt/core/CliktCommand.kt b/clikt/src/main/kotlin/com/github/ajalt/clikt/core/CliktCommand.kt
@@ -61,6 +61,13 @@ abstract class CliktCommand(
             _arguments.mapNotNull { it.parameterHelp } +
             _subcommands.map { ParameterHelp.Subcommand(it.commandName, it.shortHelp()) }
 
+    private fun getCommandNameWithParents(): String {
+        if (_context == null) createContext()
+        return generateSequence(context) { it.parent }.toList()
+                .asReversed()
+                .joinToString(" ") { it.command.commandName }
+    }
+
     /**
      * This command's context.
      *
@@ -108,15 +115,15 @@ abstract class CliktCommand(
 
     /** Return the usage string for this command. */
     open fun getFormattedUsage(): String {
-        if (_context == null) createContext()
-        return context.helpFormatter.formatUsage(allHelpParams(), programName = commandName)
+        val programName = getCommandNameWithParents()
+        return context.helpFormatter.formatUsage(allHelpParams(), programName = programName)
     }
 
     /** Return the full help string for this command. */
     open fun getFormattedHelp(): String {
-        if (_context == null) createContext()
+        val programName = getCommandNameWithParents()
         return context.helpFormatter.formatHelp(commandHelp, commandHelpEpilog,
-                allHelpParams(), programName = commandName)
+                allHelpParams(), programName = programName)
     }
 
     /**
@@ -173,7 +180,7 @@ abstract class CliktCommand(
             echo(e.message)
             exitProcess(0)
         } catch (e: UsageError) {
-            echo(e.helpMessage(context), err = true)
+            echo(e.helpMessage(), err = true)
             exitProcess(1)
         } catch (e: CliktError) {
             echo(e.message, err = true)

diff --git a/clikt/src/main/kotlin/com/github/ajalt/clikt/core/exceptions.kt b/clikt/src/main/kotlin/com/github/ajalt/clikt/core/exceptions.kt
@@ -49,17 +49,18 @@ open class UsageError private constructor(
         val text: String? = null,
         var paramName: String? = null,
         var option: Option? = null,
-        var argument: Argument? = null) : CliktError() {
-    constructor(text: String, paramName: String? = null)
-            : this(text, paramName, null, null)
+        var argument: Argument? = null,
+        var context: Context? = null) : CliktError() {
+    constructor(text: String, paramName: String? = null, context: Context? = null)
+            : this(text, paramName, null, null, context)
 
-    constructor(text: String, argument: Argument)
-            : this(text, null, null, argument)
+    constructor(text: String, argument: Argument, context: Context? = null)
+            : this(text, null, null, argument, context)
 
-    constructor(text: String, option: Option)
-            : this(text, null, option, null)
+    constructor(text: String, option: Option, context: Context? = null)
+            : this(text, null, option, null, context)
 
-    fun helpMessage(context: Context? = null): String = buildString {
+    fun helpMessage(): String = buildString {
         context?.let { append(it.command.getFormattedUsage()).append("\n\n") }
         append("Error: ").append(formatMessage())
     }
@@ -80,10 +81,10 @@ open class UsageError private constructor(
  * A parameter was given the correct number of values, but of invalid format or type.
  */
 open class BadParameterValue : UsageError {
-    constructor(text: String) : super(text)
-    constructor(text: String, paramName: String) : super(text, paramName)
-    constructor(text: String, argument: Argument) : super(text, argument)
-    constructor(text: String, option: Option) : super(text, option)
+    constructor(text: String, context: Context? = null) : super(text, null, context)
+    constructor(text: String, paramName: String, context: Context? = null) : super(text, paramName, context)
+    constructor(text: String, argument: Argument, context: Context? = null) : super(text, argument, context)
+    constructor(text: String, option: Option, context: Context? = null) : super(text, option, context)
 
     override fun formatMessage(): String {
         if (inferParamName().isEmpty()) return "Invalid value: $text"
@@ -93,20 +94,11 @@ open class BadParameterValue : UsageError {
 
 /** A required parameter was not provided */
 open class MissingParameter : UsageError {
-    /**
-     * @param paramName The name of the parameter that caused the error
-     * @param text Extra text to display in the message
-     * @param paramType A string indicating the type of parameter.
-     */
-    constructor(paramName: String, paramType: String = "parameter") : super("", paramName) {
-        this.paramType = paramType
-    }
-
-    constructor(argument: Argument) : super("", argument) {
+    constructor(argument: Argument, context: Context? = null) : super("", argument, context) {
         this.paramType = "argument"
     }
 
-    constructor(option: Option) : super("", option) {
+    constructor(option: Option, context: Context? = null) : super("", option, context) {
         this.paramType = "option"
     }
 
@@ -118,8 +110,11 @@ open class MissingParameter : UsageError {
 }
 
 /** An option was provided that does not exist. */
-open class NoSuchOption(protected val givenName: String,
-                        protected val possibilities: List<String> = emptyList()) : UsageError("") {
+open class NoSuchOption(
+        protected val givenName: String,
+        protected val possibilities: List<String> = emptyList(),
+        context: Context? = null
+) : UsageError("", context = context) {
     override fun formatMessage(): String {
         return "no such option: \"$givenName\"." + when {
             possibilities.size == 1 -> " Did you mean \"${possibilities[0]}\"?"
@@ -131,8 +126,11 @@ open class NoSuchOption(protected val givenName: String,
 }
 
 /** An option was supplied but the number of values supplied to the option was incorrect. */
-open class IncorrectOptionValueCount(option: Option,
-                                     private val givenName: String) : UsageError("", option) {
+open class IncorrectOptionValueCount(
+        option: Option,
+        private val givenName: String,
+        context: Context? = null
+) : UsageError("", option, context) {
     override fun formatMessage(): String {
         return when (option!!.nvalues) {
             0 -> "$givenName option does not take a value"
@@ -143,7 +141,10 @@ open class IncorrectOptionValueCount(option: Option,
 }
 
 /** An argument was supplied but the number of values supplied was incorrect. */
-open class IncorrectArgumentValueCount(argument: Argument) : UsageError("", argument) {
+open class IncorrectArgumentValueCount(
+        argument: Argument,
+        context: Context? = null
+) : UsageError("", argument, context) {
     override fun formatMessage(): String {
         return "argument ${inferParamName()} takes ${argument!!.nvalues} values"
     }

diff --git a/clikt/src/main/kotlin/com/github/ajalt/clikt/output/TermUi.kt b/clikt/src/main/kotlin/com/github/ajalt/clikt/output/TermUi.kt
@@ -100,7 +100,7 @@ object TermUi {
                 val result = try {
                     convert.invoke(value)
                 } catch (err: UsageError) {
-                    echo(err.helpMessage(null), console=console)
+                    echo(err.helpMessage(), console=console)
                     continue
                 }
 

diff --git a/clikt/src/main/kotlin/com/github/ajalt/clikt/parsers/Parser.kt b/clikt/src/main/kotlin/com/github/ajalt/clikt/parsers/Parser.kt
@@ -1,6 +1,12 @@
 package com.github.ajalt.clikt.parsers
 
-import com.github.ajalt.clikt.core.*
+import com.github.ajalt.clikt.core.CliktCommand
+import com.github.ajalt.clikt.core.Context
+import com.github.ajalt.clikt.core.IncorrectArgumentValueCount
+import com.github.ajalt.clikt.core.MissingParameter
+import com.github.ajalt.clikt.core.NoSuchOption
+import com.github.ajalt.clikt.core.PrintHelpMessage
+import com.github.ajalt.clikt.core.UsageError
 import com.github.ajalt.clikt.parameters.arguments.Argument
 import com.github.ajalt.clikt.parameters.options.EagerOption
 import com.github.ajalt.clikt.parameters.options.Option
@@ -80,31 +86,44 @@ internal object Parser {
 
         val invocationsByOption = invocations.groupBy({ it.first }, { it.second })
 
-        invocationsByOption.forEach { (o, inv) -> if (o is EagerOption) o.finalize(context, inv) }
-        invocationsByOption.forEach { (o, inv) -> if (o !is EagerOption) o.finalize(context, inv) }
+        try {
+            // Finalize eager options
+            invocationsByOption.forEach { (o, inv) -> if (o is EagerOption) o.finalize(context, inv) }
 
-        // Finalize the options with values so that they can apply default values etc.
-        for (o in command._options) {
-            if (o !is EagerOption && o !in invocationsByOption) o.finalize(context, emptyList())
-        }
+            // Finalize remaining options that occurred on the command line
+            invocationsByOption.forEach { (o, inv) -> if (o !is EagerOption) o.finalize(context, inv) }
 
-        parseArguments(positionalArgs, arguments).forEach { (it, v) -> it.finalize(context, v) }
+            // Finalize options not provided on the command line so that they can apply default values etc.
+            command._options.forEach { o ->
+                if (o !is EagerOption && o !in invocationsByOption) o.finalize(context, emptyList())
+            }
 
-        if (subcommand == null && subcommands.isNotEmpty() && !command.invokeWithoutSubcommand) {
-            throw PrintHelpMessage(command)
-        }
+            parseArguments(positionalArgs, arguments).forEach { (it, v) -> it.finalize(context, v) }
 
-        command.context.invokedSubcommand = subcommand
-        command.run()
+            if (subcommand == null && subcommands.isNotEmpty() && !command.invokeWithoutSubcommand) {
+                throw PrintHelpMessage(command)
+            }
+
+            command.context.invokedSubcommand = subcommand
+            command.run()
+        } catch (e: UsageError) {
+            // Augment usage errors with the current context if they don't have one
+            if (e.context == null) e.context = context
+            throw e
+        }
 
         if (subcommand != null) {
             parse(args, subcommand.context, i + 1)
         }
     }
 
-    private fun parseLongOpt(context: Context, argv: List<String>, arg: String,
-                             index: Int,
-                             optionsByName: Map<String, Option>): Pair<Option, ParseResult> {
+    private fun parseLongOpt(
+            context: Context,
+            argv: List<String>,
+            arg: String,
+            index: Int,
+            optionsByName: Map<String, Option>
+    ): Pair<Option, ParseResult> {
         val equalsIndex = arg.indexOf('=')
         var (name, value) = if (equalsIndex >= 0) {
             arg.substring(0, equalsIndex) to arg.substring(equalsIndex + 1)
@@ -118,9 +137,13 @@ internal object Parser {
         return option to result
     }
 
-    private fun parseShortOpt(context: Context, argv: List<String>, arg: String,
-                              index: Int,
-                              optionsByName: Map<String, Option>): Pair<Int, List<Pair<Option, Invocation>>> {
+    private fun parseShortOpt(
+            context: Context,
+            argv: List<String>,
+            arg: String,
+            index: Int,
+            optionsByName: Map<String, Option>
+    ): Pair<Int, List<Pair<Option, Invocation>>> {
         val prefix = arg[0].toString()
         val invocations = mutableListOf<Pair<Option, Invocation>>()
         for ((i, opt) in arg.withIndex()) {
@@ -136,7 +159,10 @@ internal object Parser {
                 "Error parsing short option ${argv[index]}: no parser consumed value.")
     }
 
-    private fun parseArguments(positionalArgs: List<String>, arguments: List<Argument>): Map<Argument, List<String>> {
+    private fun parseArguments(
+            positionalArgs: List<String>,
+            arguments: List<Argument>
+    ): Map<Argument, List<String>> {
         val out = linkedMapOf<Argument, List<String>>().withDefault { listOf() }
         // The number of fixed size arguments that occur after an unlimited size argument. This
         // includes optional single value args, so it might be bigger than the number of provided

diff --git a/clikt/src/test/kotlin/com/github/ajalt/clikt/parameters/SubcommandTest.kt b/clikt/src/test/kotlin/com/github/ajalt/clikt/parameters/SubcommandTest.kt
@@ -1,6 +1,8 @@
 package com.github.ajalt.clikt.parameters
 
 import com.github.ajalt.clikt.core.CliktCommand
+import com.github.ajalt.clikt.core.NoRunCliktCommand
+import com.github.ajalt.clikt.core.UsageError
 import com.github.ajalt.clikt.core.context
 import com.github.ajalt.clikt.core.subcommands
 import com.github.ajalt.clikt.parameters.arguments.argument
@@ -11,6 +13,7 @@ import com.github.ajalt.clikt.testing.splitArgv
 import io.kotlintest.data.forall
 import io.kotlintest.shouldBe
 import io.kotlintest.shouldNotBe
+import io.kotlintest.shouldThrow
 import io.kotlintest.tables.row
 import org.junit.Test
 
@@ -238,4 +241,37 @@ class SubcommandTest {
         C().subcommands(Sub())
                 .parse(splitArgv(argv))
     }
+
+    @Test
+    fun `command usage`() {
+        class Parent : NoRunCliktCommand() {
+            val arg by argument()
+        }
+
+        shouldThrow<UsageError> {
+            Parent().parse(splitArgv(""))
+        }.helpMessage() shouldBe """
+            |Usage: parent [OPTIONS] ARG
+            |
+            |Error: Missing argument "ARG".
+            """.trimMargin()
+    }
+
+    @Test
+    fun `subcommand usage`() {
+        class Parent : NoRunCliktCommand()
+        class Child : NoRunCliktCommand()
+        class Grandchild : NoRunCliktCommand() {
+            val arg by argument()
+        }
+
+        shouldThrow<UsageError> {
+            Parent().subcommands(Child().subcommands(Grandchild()))
+                    .parse(splitArgv("child grandchild"))
+        }.helpMessage() shouldBe """
+            |Usage: parent child grandchild [OPTIONS] ARG
+            |
+            |Error: Missing argument "ARG".
+            """.trimMargin()
+    }
 }