Add exact_match to settings, defaulting to inexact matching #154
Conversation
|
I was on the fence on this one, but this part changed my mind, so overall I'm good with this idea:
|
lib/optimist.rb
Outdated
| ## Otherwise, we raise an error that the partially given option was ambiguous. | ||
| def perform_inexact_match(arg, partial_match) # :nodoc: | ||
| return @long[partial_match] if @long.has_key?(partial_match) | ||
| partially_matched_keys = @long.keys.grep(/^#{partial_match}/) |
There was a problem hiding this comment.
Do we need to escape the partial match? Command-line wise, I'm not sure if weird characters can get in there, but it might make sense to be defensive anyway.
| partially_matched_keys = @long.keys.grep(/^#{partial_match}/) | |
| partially_matched_keys = @long.keys.grep(/^#{Regexp.escape(partial_match)}/) |
Example of the difference:
[1] pry(main)> partial_match = ".o"
[2] pry(main)> "foo".match?(/^#{partial_match}")
=> true
[3] pry(main)> "foo".match?(/^#{Regexp.escape(partial_match)}/)
=> falseThere was a problem hiding this comment.
actually wondering if this should be a select with start_with? regex may be overkill here
lib/optimist.rb
Outdated
| @@ -316,10 +331,14 @@ def parse(cmdline = ARGV) | |||
| else raise CommandlineError, "invalid argument syntax: '#{arg}'" | |||
| end | |||
|
|
|||
| sym = nil if arg =~ /--no-/ # explicitly invalidate --no-no- arguments | |||
| if arg =~ /--no-/ # explicitly invalidate --no-no- arguments | |||
There was a problem hiding this comment.
Do we want to check if the string starts with --no- or just includes --no-? The regex right now is the latter, but I don 't think that's what we want.
Regardless, I don't see us using any regex special chars nor the result from this, so I think we could use .start_with? or .include? respectively, with a string "--no-"
There was a problem hiding this comment.
hrm, interesting. Agree and we can do this as part of this PR or another.
| ## test-only access reader method so that we dont have to | ||
| ## expose settings in the public API. | ||
| class Optimist::Parser | ||
| def get_settings_for_testing ; return @settings ;end | ||
| end |
There was a problem hiding this comment.
Is it a problem to expose settings in the public API? I'm wondering if we should just allow it?
There was a problem hiding this comment.
(Note: it's fine for this PR to keep them private, and I won't hold up merge for that)
| def test_two_arguments_passed_through_block | ||
| newp = Parser.new(:abcd => 123, :efgh => "other" ) do |i| | ||
| end |
There was a problem hiding this comment.
This is interesting - I actually thought this should raise an exception. (We can discuss separately, I won't hold up the PR for this)
|
This is really nice. My only hangup is that I think the default of exact_match should be |
Co-authored-by: Jason Frey <fryguy9@gmail.com>
Co-authored-by: Jason Frey <fryguy9@gmail.com>
Co-authored-by: Jason Frey <fryguy9@gmail.com>
|
note to self: will need to update the tests to handle the flipped default... |
Add support for inexact long-option matches, inspired by Perl's Getopt::Long.
Allows the user to provide a leading substring of characters for a long-option so long as it is not ambiguous which option it references
E.g. if you have options
cakeandcarrot,--car,--carr,--carro,--carrotcan enablecarrot--cak,--cakecan enablecake--cais invalid because it is ambiguous.The behavior is not enabled for
--nooptions due to concerns over confusion.The exact_match behavior is defaulted to true, but can be turned off by passing in settings.
exact_match: falseAdditional tests were added to clarify behavior around the settings hash.
@miq-bot add-label enhancement
@miq-bot add-reviewer @Fryguy