Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding support for DidYouMean when long options are spelled incorrectly #150

Merged
merged 4 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions examples/didyoumean.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/usr/bin/env ruby
require_relative '../lib/optimist'

opts = Optimist::options do
opt :cone, "Ice cream cone"
opt :zippy, "It zips"
opt :zapzy, "It zapz"
opt :big_bug, "Madagascar cockroach"
end
p opts

# $ ./didyoumean.rb --one
# Error: unknown argument '--one'. Did you mean: [--cone] ?.
# Try --help for help.

# $ ./didyoumean.rb --zappy
# Error: unknown argument '--zappy'. Did you mean: [--zapzy, --zippy] ?.
# Try --help for help.

# $ ./didyoumean.rb --big_bug
# Error: unknown argument '--big_bug'. Did you mean: [--big-bug] ?.
# Try --help for help.

# $ ./didyoumean.rb --bigbug
# Error: unknown argument '--bigbug'. Did you mean: [--big-bug] ?.
# Try --help for help.
53 changes: 51 additions & 2 deletions lib/optimist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
# optimist is licensed under the MIT license.

require 'date'
require 'did_you_mean/levenshtein'
require 'did_you_mean/jaro_winkler'

module Optimist
VERSION = "3.1.0"
Expand Down Expand Up @@ -82,6 +84,8 @@ def self.registry_getopttype(type)
## ignore options that it does not recognize.
attr_accessor :ignore_invalid_options

DEFAULT_SETTINGS = { suggestions: true }

## Initializes the parser, and instance-evaluates any block given.
def initialize(*a, &b)
@version = nil
Expand All @@ -97,8 +101,19 @@ def initialize(*a, &b)
@synopsis = nil
@usage = nil

## allow passing settings through Parser.new as an optional hash.
## but keep compatibility with non-hashy args, though.
begin
settings_hash = Hash[*a]
@settings = DEFAULT_SETTINGS.merge(settings_hash)
a=[] ## clear out args if using as settings-hash
rescue ArgumentError
@settings = DEFAULT_SETTINGS
end

# instance_eval(&b) if b # can't take arguments
cloaker(&b).bind(self).call(*a) if b
#cloaker(&b).bind(self).call(*a) if b
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this line just be deleted? I'm curious why you stopped using the cloaker.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, should be deleted, I just removed the call to it and the cloaker method.
AFAICT the cloaker was a super clever trick used before instance_exec was in ruby, since instance_eval didnt support passing a block, but i could be wrong about that...

In any event, instance_exec seems to work such that all tests pass and doesnt require weird trickery, so felt like the cloaker could be safely removed.

self.instance_exec(*a, &b) if block_given?
end

## Define an option. +name+ is the option name, a unique identifier
Expand Down Expand Up @@ -231,6 +246,39 @@ def educate_on_error
@educate_on_error = true
end

def handle_unknown_argument(arg, candidates, suggestions)
errstring = "unknown argument '#{arg}'"
errstring += " for command '#{subcommand_name}'" if self.respond_to?(:subcommand_name)
if suggestions
input = arg.sub(/^[-]*/,'')

# Code borrowed from did_you_mean gem
jw_threshold = 0.75
seed = candidates.select {|candidate| DidYouMean::JaroWinkler.distance(candidate, input) >= jw_threshold } \
.sort_by! {|candidate| DidYouMean::JaroWinkler.distance(candidate.to_s, input) } \
.reverse!
# Correct mistypes
threshold = (input.length * 0.25).ceil
has_mistype = seed.rindex {|c| DidYouMean::Levenshtein.distance(c, input) <= threshold }
corrections = if has_mistype
seed.take(has_mistype + 1)
else
# Correct misspells
seed.select do |candidate|
length = input.length < candidate.length ? input.length : candidate.length

DidYouMean::Levenshtein.distance(candidate, input) < length
end.first(1)
end
unless corrections.empty?
dashdash_corrections = corrections.map{|s| "--#{s}" }
errstring << ". Did you mean: [#{dashdash_corrections.join(', ')}] ?"
end
end
raise CommandlineError, errstring
end
private :handle_unknown_argument

## Parses the commandline. Typically called by Optimist::options,
## but you can call it directly if you need more control.
##
Expand Down Expand Up @@ -269,7 +317,8 @@ def parse(cmdline = ARGV)
sym = nil if arg =~ /--no-/ # explicitly invalidate --no-no- arguments

next nil if ignore_invalid_options && !sym
raise CommandlineError, "unknown argument '#{arg}'" unless sym

handle_unknown_argument(arg, @long.keys, @settings[:suggestions]) unless sym

if given_args.include?(sym) && !@specs[sym].multi?
raise CommandlineError, "option '#{arg}' specified multiple times"
Expand Down
40 changes: 40 additions & 0 deletions test/optimist/parser_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,46 @@ def test_unknown_arguments
assert_raises(CommandlineError) { @p.parse(%w(--arg2)) }
end

def test_unknown_arguments_with_suggestions
sugp = Parser.new(:suggestions => true)
err = assert_raises(CommandlineError) { sugp.parse(%w(--bone)) }
assert_match(/unknown argument '--bone'$/, err.message)

sugp.opt "cone"
sugp.parse(%w(--cone))

# single letter mismatch
err = assert_raises(CommandlineError) { sugp.parse(%w(--bone)) }
assert_match(/unknown argument '--bone'. Did you mean: \[--cone\] \?$/, err.message)

# transposition
err = assert_raises(CommandlineError) { sugp.parse(%w(--ocne)) }
assert_match(/unknown argument '--ocne'. Did you mean: \[--cone\] \?$/, err.message)

# extra letter at end
err = assert_raises(CommandlineError) { sugp.parse(%w(--cones)) }
assert_match(/unknown argument '--cones'. Did you mean: \[--cone\] \?$/, err.message)

# too big of a mismatch to suggest (extra letters in front)
err = assert_raises(CommandlineError) { sugp.parse(%w(--snowcone)) }
assert_match(/unknown argument '--snowcone'$/, err.message)

# too big of a mismatch to suggest (nothing close)
err = assert_raises(CommandlineError) { sugp.parse(%w(--clown-nose)) }
assert_match(/unknown argument '--clown-nose'$/, err.message)

sugp.opt "zippy"
sugp.opt "zapzy"
# single letter mismatch, matches two
err = assert_raises(CommandlineError) { sugp.parse(%w(--zipzy)) }
assert_match(/unknown argument '--zipzy'. Did you mean: \[--zippy, --zapzy\] \?$/, err.message)

sugp.opt "big_bug"
# suggest common case of dash versus underscore in argnames
err = assert_raises(CommandlineError) { sugp.parse(%w(--big_bug)) }
assert_match(/unknown argument '--big_bug'. Did you mean: \[--big-bug\] \?$/, err.message)
end

def test_syntax_check
@p.opt "arg"

Expand Down
Loading