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

Add alt longname and multiple char support #151

Merged
merged 2 commits into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 20 additions & 0 deletions examples/alt_names.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env ruby
require_relative '../lib/optimist'

opts = Optimist::options do
# 'short:' can now take more than one short-option character
# you can specify 'short:' as a string/symbol or an array of strings/symbols
# 'alt:' adds additional long-opt choices (over the original name or the long: name)
# you can specify 'alt:' as a string/symbol or an array of strings/symbols.
#
opt :concat, 'concatenate flag', short: ['-C', 'A'], alt: ['cat', '--append']
opt :array_len, 'set Array length', long: 'size', alt: 'length', type: Integer
end

p opts

# $ ./alt_names.rb -h
# Options:
# -C, -A, --concat, --cat, --append concatenate flag
# -s, --size, --length=<i> set Array length
# -h, --help Show this message
135 changes: 96 additions & 39 deletions lib/optimist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ def self.registry_getopttype(type)
return @registry[lookup].new
end

INVALID_SHORT_ARG_REGEX = /[\d-]/ #:nodoc:

## The values from the commandline that were not interpreted by #parse.
attr_reader :leftovers

Expand Down Expand Up @@ -166,11 +164,19 @@ def opt(name, desc = "", opts = {}, &b)
o = Option.create(name, desc, opts)

raise ArgumentError, "you already have an argument named '#{name}'" if @specs.member? o.name
raise ArgumentError, "long option name #{o.long.inspect} is already taken; please specify a (different) :long" if @long[o.long]
raise ArgumentError, "short option name #{o.short.inspect} is already taken; please specify a (different) :short" if @short[o.short]

o.long.names.each do |lng|
raise ArgumentError, "long option name #{lng.inspect} is already taken; please specify a (different) :long/:alt" if @long[lng]
@long[lng] = o.name
end

o.short.chars.each do |short|
raise ArgumentError, "short option name #{short.inspect} is already taken; please specify a (different) :short" if @short[short]
@short[short] = o.name
end

raise ArgumentError, "permitted values for option #{o.long.inspect} must be either nil or an array;" unless o.permitted.nil? or o.permitted.is_a? Array
@long[o.long] = o.name
@short[o.short] = o.name if o.short?

@specs[o.name] = o
@order << [:opt, o.name]
end
Expand Down Expand Up @@ -619,11 +625,10 @@ def collect_argument_parameters(args, start_at)
def resolve_default_short_options!
@order.each do |type, name|
opts = @specs[name]
next if type != :opt || opts.short

c = opts.long.chars.find { |d| d !~ INVALID_SHORT_ARG_REGEX && !@short.member?(d) }
next if type != :opt || opts.doesnt_need_autogen_short
c = opts.long.long.split(//).find { |d| d !~ Optimist::ShortNames::INVALID_ARG_REGEX && !@short.member?(d) }
if c # found a character to use
opts.short = c
opts.short.add c
@short[c] = name
end
end
Expand Down Expand Up @@ -651,14 +656,86 @@ def wrap_line(str, opts = {})

end

class LongNames
def initialize
@truename = nil
@long = nil
@alts = []
end

def make_valid(lopt)
return nil if lopt.nil?
case lopt.to_s
when /^--([^-].*)$/ then $1
when /^[^-]/ then lopt.to_s
else raise ArgumentError, "invalid long option name #{lopt.inspect}"
end
end

def set(name, lopt, alts)
@truename = name
lopt = lopt ? lopt.to_s : name.to_s.gsub("_", "-")
@long = make_valid(lopt)
alts = [alts] unless alts.is_a?(Array) # box the value
@alts = alts.map { |alt| make_valid(alt) }.compact
end

# long specified with :long has precedence over the true-name
def long ; @long || @truename ; end

# all valid names, including alts
def names
[long] + @alts
end

end

class ShortNames

INVALID_ARG_REGEX = /[\d-]/ #:nodoc:

def initialize
@chars = []
@auto = true
end

attr_reader :chars, :auto

def add(values)
values = [values] unless values.is_a?(Array) # box the value
values = values.compact
if values.include?(:none)
if values.size == 1
@auto = false
return
end
raise ArgumentError, "Cannot use :none with any other values in short option: #{values.inspect}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated to prevent :none from being used with any other value (including itself)

end
values.each do |val|
strval = val.to_s
sopt = case strval
when /^-(.)$/ then $1
when /^.$/ then strval
else raise ArgumentError, "invalid short option name '#{val.inspect}'"
end

if sopt =~ INVALID_ARG_REGEX
raise ArgumentError, "short option name '#{sopt}' can't be a number or a dash"
end
Comment on lines +722 to +724
Copy link
Member

Choose a reason for hiding this comment

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

numeric options are interesting though. I'm reminded of git log -1 - would be cool to support a singular pure number as some sort of new type. 🤔

For this piece of code though I agree that the short opt can't be a number

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a reformat of this

raise ArgumentError, "a short option name can't be a number or a dash" if sopt =~ ::Optimist::Parser::INVALID_SHORT_ARG_REGEX

but yeah I agree. Implementing commands like ls (e.g. ls -1 or ls -d1) with optimist is impractical as-is due to the numerical short-opt restriction. I found this out trying to implement a version of colorls some time back. OTOH, there are unintended consequences to supporting this, bc it's harder to tell what's a negative integer vs. an option, especially since optmist allows ambiguous integer lists.

@chars << sopt
end
end

end

class Option

attr_accessor :name, :short, :long, :default, :permitted
attr_writer :multi_given

def initialize
@long = nil
@short = nil
@long = LongNames.new
@short = ShortNames.new # can be an Array of one-char strings, a one-char String, nil or :none
@name = nil
@multi_given = false
@hidden = false
Expand Down Expand Up @@ -690,7 +767,7 @@ def multi_arg? ; false ; end

def array_default? ; self.default.kind_of?(Array) ; end

def short? ; short && short != :none ; end
def doesnt_need_autogen_short ; !short.auto || short.chars.any? ; end

def callback ; opts(:callback) ; end
def desc ; opts(:desc) ; end
Expand All @@ -705,7 +782,10 @@ def parse(_paramlist, _neg_given)
def type_format ; "" ; end

def educate
(short? ? "-#{short}, " : " ") + "--#{long}" + type_format + (flag? && default ? ", --no-#{long}" : "")
optionlist = []
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noting that this is an undo of the short-argument padding/alignment.
If there can be multiple short args, then the padding doesnt make sense anymore or it would be a bit more complicated to align.

optionlist.concat(short.chars.map { |o| "-#{o}" })
optionlist.concat(long.names.map { |o| "--#{o}" })
optionlist.compact.join(', ') + type_format + (flag? && default ? ", --no-#{long}" : "")
end

## Format the educate-line description including the default and permitted value(s)
Expand Down Expand Up @@ -770,10 +850,10 @@ def self.create(name, desc="", opts={}, settings={})
opt_inst = (opttype || opttype_from_default || Optimist::BooleanOption.new)

## fill in :long
opt_inst.long = handle_long_opt(opts[:long], name)
opt_inst.long.set(name, opts[:long], opts[:alt])

## fill in :short
opt_inst.short = handle_short_opt(opts[:short])
opt_inst.short.add opts[:short]

## fill in :multi
multi_given = opts[:multi] || false
Expand Down Expand Up @@ -826,29 +906,6 @@ def self.get_klass_from_default(opts, opttype)
return Optimist::Parser.registry_getopttype(type_from_default)
end

def self.handle_long_opt(lopt, name)
lopt = lopt ? lopt.to_s : name.to_s.gsub("_", "-")
lopt = case lopt
when /^--([^-].*)$/ then $1
when /^[^-]/ then lopt
else raise ArgumentError, "invalid long option name #{lopt.inspect}"
end
end

def self.handle_short_opt(sopt)
sopt = sopt.to_s if sopt && sopt != :none
sopt = case sopt
when /^-(.)$/ then $1
when nil, :none, /^.$/ then sopt
else raise ArgumentError, "invalid short option name '#{sopt.inspect}'"
end

if sopt
raise ArgumentError, "a short option name can't be a number or a dash" if sopt =~ ::Optimist::Parser::INVALID_SHORT_ARG_REGEX
end
return sopt
end

end

# Flag option. Has no arguments. Can be negated with "no-".
Expand Down
159 changes: 159 additions & 0 deletions test/optimist/alt_names_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
require 'test_helper'

module Optimist

class AlternateNamesTest < ::Minitest::Test

def setup
@p = Parser.new
end

def get_help_string
err = assert_raises(Optimist::HelpNeeded) do
@p.parse(%w(--help))
end
sio = StringIO.new "w"
@p.educate sio
sio.string
end

def test_altshort
@p.opt :catarg, "desc", :short => ["c", "-C"]
opts = @p.parse %w(-c)
assert_equal true, opts[:catarg]
opts = @p.parse %w(-C)
assert_equal true, opts[:catarg]
assert_raises(CommandlineError) { @p.parse %w(-c -C) }
assert_raises(CommandlineError) { @p.parse %w(-cC) }
end

def test_altshort_invalid_none
assert_raises(ArgumentError) {
@p.opt :something, "some opt", :short => [:s, :none]
}
assert_raises(ArgumentError) {
@p.opt :something, "some opt", :short => [:none, :s]
}
assert_raises(ArgumentError) {
@p.opt :zumthing, "some opt", :short => [:none, :none]
}
end

def test_altshort_with_multi
@p.opt :flag, "desc", :short => ["-c", "C", :x], :multi => true
@p.opt :num, "desc", :short => ["-n", "N"], :multi => true, type: Integer
@p.parse %w(-c)
@p.parse %w(-C -c -x)
@p.parse %w(-c -C)
@p.parse %w(-c -C -c -C)
opts = @p.parse %w(-ccCx)
assert_equal true, opts[:flag]
@p.parse %w(-c)
@p.parse %w(-N 1 -n 3)
@p.parse %w(-n 2 -N 4)
opts = @p.parse %w(-n 4 -N 3 -n 2 -N 1)
assert_equal [4, 3, 2, 1], opts[:num]
end

def test_altlong
@p.opt "goodarg0", "desc", :alt => "zero"
@p.opt "goodarg1", "desc", :long => "newone", :alt => "one"
@p.opt "goodarg2", "desc", :alt => "--two"
@p.opt "goodarg3", "desc", :alt => ["three", "--four", :five]

[%w[--goodarg0], %w[--zero]].each do |a|
opts = @p.parse(a)
assert opts.goodarg0
end

[%w[--newone], %w[-n], %w[--one]].each do |a|
opts = @p.parse(a)
assert opts.goodarg1
end

[%w[--two]].each do |a|
opts = @p.parse(a)
assert opts.goodarg2
end

[%w[--three], %w[--four], %w[--five]].each do |a|
opts = @p.parse(a)
assert opts.goodarg3
end

[%w[--goodarg1], %w[--missing], %w[-a]].each do |a|
assert_raises(Optimist::CommandlineError) { @p.parse(a) }
end

["", '--', '-bad', '---threedash'].each do |altitem|
assert_raises(ArgumentError) { @p.opt "badarg", "desc", :alt => altitem }
end
end

def test_altshort_help
@p.opt :cat, 'cat', short: ['c','C','a','T']
outstring = get_help_string
# expect mutliple short-opts to be in the help
assert_match(/-c, -C, -a, -T, --cat/, outstring)
end


def test_altlong_help
@p.opt :cat, 'a cat', alt: :feline
@p.opt :dog, 'a dog', alt: ['Pooch', :canine]
@p.opt :fruit, 'a fruit', long: :fig, alt: ['peach', :pear, "--apple"], short: :none
@p.opt :veg, "gemuse", long: :gemuse, alt: [:groente]
outstring = get_help_string

assert_match(/^\s*-c, --cat, --feline/, outstring)
assert_match(/^\s*-d, --dog, --Pooch, --canine/, outstring)

# expect long-opt to shadow the actual name
assert_match(/^\s*--fig, --peach, --pear, --apple/, outstring)
assert_match(/^\s*-g, --gemuse, --groente/, outstring)

end

def test_alt_duplicates
# alt duplicates named option
assert_raises(ArgumentError) {
@p.opt :cat, 'desc', :alt => :cat
}
# alt duplicates :long
assert_raises(ArgumentError) {
@p.opt :cat, 'desc', :long => :feline, :alt => [:feline]
}
# alt duplicates itself
assert_raises(ArgumentError) {
@p.opt :abc, 'desc', :alt => [:aaa, :aaa]
}
end

def test_altlong_collisions
@p.opt :fat, 'desc'
@p.opt :raton, 'desc', :long => :rat
@p.opt :bat, 'desc', :alt => [:baton, :twirl]

# :alt collision with named option
assert_raises(ArgumentError) {
@p.opt :cat, 'desc', :alt => :fat
}

# :alt collision with :long option
assert_raises(ArgumentError) {
@p.opt :cat, 'desc', :alt => :rat
}

# :named option collision with existing :alt option
assert_raises(ArgumentError) {
@p.opt :baton, 'desc'
}

# :long option collision with existing :alt option
assert_raises(ArgumentError) {
@p.opt :whirl, 'desc', :long => 'twirl'
}

end
end
end
Loading
Loading