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.

Sign up for GitHub

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

Added setting to disable implicit short options #155

Merged
merged 2 commits into from
May 10, 2024
Merged

Added setting to disable implicit short options #155

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45b4725
added settings to only create short options explicitly
nanobowers May 5, 2024
60f2f2e
Flip polarity of explicit_short_opts: false to implicit_short_opts: true
nanobowers May 10, 2024
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
9 changes: 7 additions & 2 deletions lib/optimist.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ def self.registry_getopttype(type)
## ignore options that it does not recognize.
attr_accessor :ignore_invalid_options

DEFAULT_SETTINGS = { suggestions: true, exact_match: true }
DEFAULT_SETTINGS = {
exact_match: true,
explicit_short_opts: false,
Copy link
Member

@Fryguy Fryguy May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm finding the "negative" option text, a default of false, and then using unless in the code with it very confusing (like a triple negative in my mind). Perhaps it's better to flip the logic to the positive with:

Suggested change
explicit_short_opts: false,
implicit_short_opts: true,

Then later, for example, this reads better to me:

-     resolve_default_short_options! unless @settings[:explicit_short_opts]
+     resolve_default_short_options! if @settings[:implicit_short_opts]

Copy link
Member

Choose a reason for hiding this comment

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

This would also set all the default settings to "true" and making them all "opt-out", so it's consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll switch the polarity from explicit_short_opts: false to implicit_short_opts: true and push it up

suggestions: true
}

## Initializes the parser, and instance-evaluates any block given.
def initialize(*a, &b)
Expand Down Expand Up @@ -313,7 +317,7 @@ def parse(cmdline = ARGV)
vals[sym] = [] if opts.multi && !opts.default # multi arguments default to [], not nil
end

resolve_default_short_options!
resolve_default_short_options! unless @settings[:explicit_short_opts]

## resolve symbols
given_args = {}
Expand Down Expand Up @@ -1027,6 +1031,7 @@ def multi_arg? ; true ; end
## +settings+ include:
## * :exact_match : (default=true) Allow minimum unambigous number of characters to match a long option
## * :suggestions : (default=true) Enables suggestions when unknown arguments are given and DidYouMean is installed. DidYouMean comes standard with Ruby 2.3+
## * :explicit_short_opts : (default=false) Short options will only be created where explicitly defined. If you do not like short-options, this will prevent having to define :short=> :none for all of your options.
## Because Optimist::options uses a default argument for +args+, you must pass that argument when using the settings feature.
##
## See more examples at https://www.manageiq.org/optimist
Expand Down
33 changes: 33 additions & 0 deletions test/optimist/parser_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,39 @@ def test_default_shorts_assigned_only_after_user_shorts
assert opts[:ccd]
end

def test_short_opts_not_implicitly_created
newp = Parser.new(explicit_short_opts: true)
newp.opt :user1, "user1"
newp.opt :bag, "bag", :short => 'b'
assert_raises(CommandlineError) do
newp.parse %w(-u)
end
opts = newp.parse %w(--user1)
assert opts[:user1]
opts = newp.parse %w(-b)
assert opts[:bag]
end

def test_short_opts_not_implicit_help_ver
# When explicit_short_opts is enabled this extends
# to the built-in help/version also.
newp = Parser.new(explicit_short_opts: true)
newp.opt :abc, "abc"
newp.version "3.4.5"
assert_raises(CommandlineError) do
newp.parse %w(-h)
end
assert_raises(CommandlineError) do
newp.parse %w(-v)
end
assert_raises(HelpNeeded) do
newp.parse %w(--help)
end
assert_raises(VersionNeeded) do
newp.parse %w(--version)
end
end

def test_inexact_match
newp = Parser.new(exact_match: false)
newp.opt :liberation, "liberate something", :type => :int
Expand Down
Loading