Skip to content

Commit

Permalink
Improve typo correction suggestions (#150)
Browse files Browse the repository at this point in the history
  • Loading branch information
ajalt authored Mar 11, 2020
1 parent ba6b66d commit 4543cb7
Show file tree
Hide file tree
Showing 15 changed files with 263 additions and 40 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- `registeredSubcommands`, `registeredOptions`, `registeredArguments`, and `registeredParameterGroups` methods on `CliktCommand`.
- Ability to [read default option values](https://ajalt.github.io/clikt/api/clikt/com.github.ajalt.clikt.sources/-value-source/index.md) from configuration files and other sources. Support for Java property files is built in on JVM, and other formats can be added easily.
- `allowMultipleSubcommands` parameter to `CliktCommand` that allows you to pass multiple subcommands in the same call. ([docs](docs/commands.md#chaining-and-repeating-subcommands))
- Errors from typos in subcommand names will now include suggested corrections. Corrections for options and subcommands are now based on a Jaro-Winkler similarity metric.

### Changed
- Update Kotlin to 1.3.70
Expand All @@ -13,7 +14,6 @@
- `CliktCommand.toString` now includes the class name

## [2.5.0] - 2020-02-22

### Added
- Clikt is now available as a Kotlin Multiplatform Project, supporting JVM, NodeJS, and native Windows, Linux, and macOS.
- `eagerOption {}` function to more easily register eager options.
Expand Down
24 changes: 22 additions & 2 deletions clikt/src/commonMain/kotlin/com/github/ajalt/clikt/core/Context.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import com.github.ajalt.clikt.sources.ValueSource
import kotlin.properties.ReadOnlyProperty
import kotlin.reflect.KProperty

typealias TypoSuggestor = (enteredValue: String, possibleValues: List<String>) -> List<String>

/**
* A object used to control command line parsing and pass data between commands.
*
Expand All @@ -34,6 +36,9 @@ import kotlin.reflect.KProperty
* @property console The console to use to print messages.
* @property expandArgumentFiles If true, arguments starting with `@` will be expanded as argument
* files. If false, they will be treated as normal arguments.
* @property correctionSuggestor A callback called when the command line contains an invalid option or
* subcommand name. It takes the entered name and a list of all registered names option/subcommand
* names and filters the list down to values to suggest to the user.
*/
@OptIn(ExperimentalValueSourceApi::class)
class Context(
Expand All @@ -49,7 +54,8 @@ class Context(
val console: CliktConsole,
val expandArgumentFiles: Boolean,
val readEnvvarBeforeValueSource: Boolean,
val valueSource: ValueSource?
val valueSource: ValueSource?,
val correctionSuggestor: TypoSuggestor
) {
var invokedSubcommand: CliktCommand? = null
internal set
Expand Down Expand Up @@ -177,6 +183,13 @@ class Context(
fun valueSources(vararg sources: ValueSource) {
valueSource = ChainedValueSource(sources.toList())
}

/**
* A callback called when the command line contains an invalid option or
* subcommand name. It takes the entered name and a list of all registered names option/subcommand
* names and filters the list down to values to suggest to the user.
*/
var correctionSuggestor: TypoSuggestor = DEFAULT_CORRECTION_SUGGESTOR
}

companion object {
Expand All @@ -188,7 +201,7 @@ class Context(
return Context(
parent, command, interspersed, autoEnvvarPrefix, printExtraMessages,
helpOptionNames, helpOptionMessage, helpFormatter, tokenTransformer, console,
expandArgumentFiles, readEnvvarBeforeValueSource, valueSource
expandArgumentFiles, readEnvvarBeforeValueSource, valueSource, correctionSuggestor
)
}
}
Expand Down Expand Up @@ -230,3 +243,10 @@ inline fun <reified T : Any> CliktCommand.findOrSetObject(crossinline default: (
}
}
}

private val DEFAULT_CORRECTION_SUGGESTOR : TypoSuggestor = { enteredValue, possibleValues ->
possibleValues.map { it to jaroWinklerSimilarity(enteredValue, it) }
.filter { it.second > 0.8 }
.sortedByDescending { it.second }
.map { it.first }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.github.ajalt.clikt.core

import kotlin.math.max
import kotlin.math.min


private fun jaroSimilarity(s1: String, s2: String): Double {
if (s1.isEmpty() && s2.isEmpty()) return 1.0
else if (s1.isEmpty() || s2.isEmpty()) return 0.0
else if (s1.length == 1 && s2.length == 1) return if (s1[0] == s2[0]) 1.0 else 0.0

val searchRange: Int = max(s1.length, s2.length) / 2 - 1
val s2Consumed = BooleanArray(s2.length)
var matches = 0.0
var transpositions = 0
var s2MatchIndex = 0

for ((i, c1) in s1.withIndex()) {
val start = max(0, i - searchRange)
val end = min(s2.lastIndex, i + searchRange)
for (j in start..end) {
val c2 = s2[j]
if (c1 != c2 || s2Consumed[j]) continue
s2Consumed[j] = true
matches += 1
if (j < s2MatchIndex) transpositions += 1
s2MatchIndex = j
break
}
}

return when (matches) {
0.0 -> 0.0
else -> (matches / s1.length +
matches / s2.length +
(matches - transpositions) / matches) / 3.0
}
}

internal fun jaroWinklerSimilarity(s1: String, s2: String): Double {
// Unlike classic Jaro-Winkler, we don't set a limit on the prefix length
val prefixLength = s1.commonPrefixWith(s2).length
val jaro = jaroSimilarity(s1, s2)
val winkler = jaro + (0.1 * prefixLength * (1 - jaro))
return min(winkler, 1.0)
}
Original file line number Diff line number Diff line change
Expand Up @@ -123,22 +123,38 @@ open class MissingParameter : UsageError {
}
}

/** An option was provided that does not exist. */
open class NoSuchOption(
/** A parameter was provided that does not exist. */
open class NoSuchParameter(
protected val parameterType: String,
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 {
return "no such ${parameterType}: \"$givenName\"." + when {
possibilities.size == 1 -> " Did you mean \"${possibilities[0]}\"?"
possibilities.size > 1 -> possibilities.joinToString(
prefix = " (Possible options: ", postfix = ")")
prefix = " (Possible ${parameterType}s: ", postfix = ")")
else -> ""
}
}
}

/** A subcommand was provided that does not exist. */
open class NoSuchSubcommand(
givenName: String,
possibilities: List<String> = emptyList(),
context: Context? = null
) : NoSuchParameter("subcommand", givenName, possibilities, context)


/** An option was provided that does not exist. */
open class NoSuchOption(
givenName: String,
possibilities: List<String> = emptyList(),
context: Context? = null
) : NoSuchParameter("option", givenName, possibilities, context)

/** An option was supplied but the number of values supplied to the option was incorrect. */
open class IncorrectOptionValueCount(
option: Option,
Expand Down Expand Up @@ -178,9 +194,9 @@ open class MutuallyExclusiveGroupException(
}

/** A required configuration file was not found. */
class FileNotFoundError(filename: String) : UsageError("$filename not found")
class FileNotFound(filename: String) : UsageError("$filename not found")

/** A configuration file failed to parse correctly */
class FileFormatError(filename: String, message: String, lineno: Int? = null) : UsageError(
class InvalidFileFormat(filename: String, message: String, lineno: Int? = null) : UsageError(
"incorrect format in file $filename${lineno?.let { " line $it" } ?: ""}}: $message"
)
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ internal object Parser {
if (excess > 0) {
if (hasMultipleSubAncestor) {
i = tokens.size - excess
} else if (excess == 1 && subcommands.isNotEmpty()) {
val actual = positionalArgs.last()
throw NoSuchSubcommand(actual, context.correctionSuggestor(actual, subcommands.keys.toList()))
} else {
val actual = positionalArgs.takeLast(excess).joinToString(" ", limit = 3, prefix = "(", postfix = ")")
throw UsageError("Got unexpected extra argument${if (excess == 1) "" else "s"} $actual")
Expand Down Expand Up @@ -180,8 +183,10 @@ internal object Parser {
tok to null
}
name = context.tokenTransformer(context, name)
val option = optionsByName[name] ?: throw NoSuchOption(name,
possibilities = optionsByName.keys.filter { it.startsWith(name) })
val option = optionsByName[name] ?: throw NoSuchOption(
givenName = name,
possibilities = context.correctionSuggestor(name, optionsByName.keys.toList())
)
val result = option.parser.parseLongOpt(option, name, tokens, index, value)
return option to result
}
Expand Down Expand Up @@ -241,13 +246,13 @@ internal object Parser {
}

private fun loadArgFile(filename: String): List<String> {
val text = readFileIfExists(filename) ?: throw FileNotFoundError(filename)
val text = readFileIfExists(filename) ?: throw FileNotFound(filename)
val toks = mutableListOf<String>()
var inQuote: Char? = null
val sb = StringBuilder()
var i = 0
fun err(msg: String): Nothing {
throw FileFormatError(filename, msg, text.take(i).count { it == '\n' })
throw InvalidFileFormat(filename, msg, text.take(i).count { it == '\n' })
}
loop@ while (i < text.length) {
val c = text[i]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.github.ajalt.clikt.core

import io.kotest.data.forall
import io.kotest.matchers.doubles.plusOrMinus
import io.kotest.matchers.shouldBe
import io.kotest.tables.row
import kotlin.test.Test


class JaroWinkerSimilarityTest {
@Test
fun jaroWinklerSimilarity() = forall(
row("", "", 1.0),
row("", "a", 0.0),
row("a", "", 0.0),
row("a", "a", 1.0),
row("aa", "aa", 1.0),
row("aaapppp", "", 0.0),
row("fly", "ant", 0.0),
row("cheeseburger", "cheese fries", 0.91),
row("frog", "fog", 0.93),
row("elephant", "hippo", 0.44),
row("hippo", "elephant", 0.44),
row("hippo", "zzzzzzzz", 0.0),
row("hello", "hallo", 0.88)
) { s1, s2, expected ->
jaroWinklerSimilarity(s1, s2) shouldBe (expected plusOrMinus 0.01)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,22 @@ class OptionTest {

@Test
@JsName("no_such_option")
fun `no such option`() {
fun `no such option`() = forall(
row("--qux", "no such option: \"--qux\"."),
row("--fo", "no such option: \"--fo\". Did you mean \"--foo\"?"),
row("--fop", "no such option: \"--fop\". Did you mean \"--foo\"?"),
row("--car", "no such option: \"--car\". Did you mean \"--bar\"?"),
row("--ba", "no such option: \"--ba\". (Possible options: --bar, --baz)")
) { argv, message ->
class C : TestCommand(called = false) {
val foo by option()
val bar by option()
val baz by option()
}

shouldThrow<NoSuchOption> {
C().parse("--qux")
}.message shouldBe "no such option: \"--qux\"."

shouldThrow<NoSuchOption> {
C().parse("--fo")
}.message shouldBe "no such option: \"--fo\". Did you mean \"--foo\"?"

shouldThrow<NoSuchOption> {
C().parse("--ba")
}.message shouldBe "no such option: \"--ba\". (Possible options: --bar, --baz)"
C().parse(argv)
}.message shouldBe message
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.github.ajalt.clikt.parameters

import com.github.ajalt.clikt.core.NoSuchSubcommand
import com.github.ajalt.clikt.core.UsageError
import com.github.ajalt.clikt.core.context
import com.github.ajalt.clikt.core.subcommands
Expand Down Expand Up @@ -254,4 +255,19 @@ class SubcommandTest {
|Error: Missing argument "ARG".
""".trimMargin()
}

@Test
fun noSuchSubcommand() = forall(
row("qux", "no such subcommand: \"qux\"."),
row("fo", "no such subcommand: \"fo\". Did you mean \"foo\"?"),
row("fop", "no such subcommand: \"fop\". Did you mean \"foo\"?"),
row("bart", "no such subcommand: \"bart\". Did you mean \"bar\"?"),
row("ba", "no such subcommand: \"ba\". (Possible subcommands: bar, baz)")
) { argv, message ->
shouldThrow<NoSuchSubcommand> {
TestCommand()
.subcommands(TestCommand(name = "foo"), TestCommand(name = "bar"), TestCommand(name = "baz"))
.parse(argv)
}.message shouldBe message
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
Copyright 2016 sksamuel
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package io.kotest.matchers.doubles

import io.kotest.matchers.Matcher
import io.kotest.matchers.MatcherResult
import kotlin.math.abs

/**
* Creates a matcher for the interval [[this] - [tolerance] , [this] + [tolerance]]
*
*
* ```
* 0.1 shouldBe (0.4 plusOrMinus 0.5) // Assertion passes
* 0.1 shouldBe (0.4 plusOrMinus 0.2) // Assertion fails
* ```
*/
infix fun Double.plusOrMinus(tolerance: Double): ToleranceMatcher = ToleranceMatcher(
this,
tolerance)

class ToleranceMatcher(private val expected: Double?, private val tolerance: Double) : Matcher<Double?> {
override fun test(value: Double?): MatcherResult {
return if(value == null || expected == null) {
MatcherResult(value == expected, "$value should be equal to $expected", "$value should not be equal to $expected")
} else if (expected.isNaN() && value.isNaN()) {
println("[WARN] By design, Double.Nan != Double.Nan; see https://stackoverflow.com/questions/8819738/why-does-double-nan-double-nan-return-false/8819776#8819776")
MatcherResult(false,
"By design, Double.Nan != Double.Nan; see https://stackoverflow.com/questions/8819738/why-does-double-nan-double-nan-return-false/8819776#8819776",
"By design, Double.Nan != Double.Nan; see https://stackoverflow.com/questions/8819738/why-does-double-nan-double-nan-return-false/8819776#8819776"
)
} else {
if (tolerance == 0.0)
println("[WARN] When comparing doubles consider using tolerance, eg: a shouldBe (b plusOrMinus c)")
val diff = abs(value - expected)
MatcherResult(diff <= tolerance, "$value should be equal to $expected", "$value should not be equal to $expected")
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.github.ajalt.clikt.sources

import com.github.ajalt.clikt.core.Context
import com.github.ajalt.clikt.core.FileFormatError
import com.github.ajalt.clikt.core.InvalidFileFormat
import com.github.ajalt.clikt.parameters.options.Option
import com.github.ajalt.clikt.sources.MapValueSource.Companion.defaultKey
import java.io.File
Expand All @@ -18,7 +18,7 @@ object PropertiesValueSource {
* If the [file] does not exist, an empty value source will be returned.
*
* @param file The file to read from.
* @param requireValid If true, a [FileFormatError] will be thrown if the file doesn't parse correctly.
* @param requireValid If true, a [InvalidFileFormat] will be thrown if the file doesn't parse correctly.
* @param getKey A function that will return the property key for a given option.
*/
fun from(
Expand All @@ -31,7 +31,7 @@ object PropertiesValueSource {
try {
file.bufferedReader().use { properties.load(it) }
} catch (e: Throwable) {
if (requireValid) throw FileFormatError(file.name, e.message ?: "could not read file")
if (requireValid) throw InvalidFileFormat(file.name, e.message ?: "could not read file")
}
}

Expand Down
Loading

0 comments on commit 4543cb7

Please sign in to comment.