From 46639a22ba9aca022e034b9349e5581172ea0ace Mon Sep 17 00:00:00 2001
From: Ben Bowers <nanobowers@gmail.com>
Date: Sat, 11 May 2024 08:29:28 -0400
Subject: [PATCH 1/2] rebase merge from master

---
 examples/alt_names.rb           |  20 +++++
 lib/optimist.rb                 | 132 +++++++++++++++++++---------
 test/optimist/alt_names_test.rb | 147 ++++++++++++++++++++++++++++++++
 test/optimist/parser_test.rb    |  12 ++-
 4 files changed, 268 insertions(+), 43 deletions(-)
 create mode 100755 examples/alt_names.rb
 create mode 100644 test/optimist/alt_names_test.rb

diff --git a/examples/alt_names.rb b/examples/alt_names.rb
new file mode 100755
index 0000000..b209541
--- /dev/null
+++ b/examples/alt_names.rb
@@ -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
diff --git a/lib/optimist.rb b/lib/optimist.rb
index 8ac4f6c..f3a6af2 100644
--- a/lib/optimist.rb
+++ b/lib/optimist.rb
@@ -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
 
@@ -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
@@ -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
@@ -651,14 +656,83 @@ 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.compact.each do |val|
+      if val == :none
+        @auto = false
+        raise "Cannot set short to :none if short-chars have been defined '#{@chars}'" unless chars.empty?
+        next
+      end
+      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
+      @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
@@ -690,7 +764,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
@@ -705,7 +779,10 @@ def parse(_paramlist, _neg_given)
   def type_format ; "" ; end
 
   def educate
-    (short? ? "-#{short}, " : "    ") + "--#{long}" + type_format + (flag? && default ? ", --no-#{long}" : "")
+    optionlist = []
+    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)
@@ -770,10 +847,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
@@ -826,29 +903,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-".
diff --git a/test/optimist/alt_names_test.rb b/test/optimist/alt_names_test.rb
new file mode 100644
index 0000000..c4d99ce
--- /dev/null
+++ b/test/optimist/alt_names_test.rb
@@ -0,0 +1,147 @@
+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_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
diff --git a/test/optimist/parser_test.rb b/test/optimist/parser_test.rb
index dce588f..dbadc84 100644
--- a/test/optimist/parser_test.rb
+++ b/test/optimist/parser_test.rb
@@ -528,8 +528,8 @@ def test_multi_line_description
     @p.educate(out)
     assert_equal <<-EOM, out.string
 Options:
-      --arg=<i>    This is an arg
-                   with a multi-line description
+  --arg=<i>    This is an arg
+               with a multi-line description
     EOM
   end
 
@@ -592,7 +592,11 @@ def test_short_options_cant_be_numeric
     assert_raises(ArgumentError) { @p.opt :arg, "desc", :short => "-1" }
     @p.opt :a1b, "desc"
     @p.opt :a2b, "desc"
-    assert @p.specs[:a2b].short.to_i == 0
+    @p.parse []
+    # testing private interface to ensure default
+    # short options did not become numeric
+    assert_equal @p.specs[:a1b].short.chars.first, 'a'
+    assert_equal @p.specs[:a2b].short.chars.first, 'b'
   end
 
   def test_short_options_can_be_weird
@@ -769,7 +773,7 @@ def test_long_options_also_take_equals
 
   def test_auto_generated_long_names_convert_underscores_to_hyphens
     @p.opt :hello_there
-    assert_equal "hello-there", @p.specs[:hello_there].long
+    assert_equal "hello-there", @p.specs[:hello_there].long.long
   end
 
   def test_arguments_passed_through_block

From 66628edcab44637e859cbee3d2ca52822650885c Mon Sep 17 00:00:00 2001
From: Ben Bowers <nanobowers@gmail.com>
Date: Fri, 10 May 2024 08:17:30 -0400
Subject: [PATCH 2/2] fixing alt-short with :none ordering issue

---
 lib/optimist.rb                 | 11 +++++++----
 test/optimist/alt_names_test.rb | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/lib/optimist.rb b/lib/optimist.rb
index f3a6af2..d3f21da 100644
--- a/lib/optimist.rb
+++ b/lib/optimist.rb
@@ -703,12 +703,15 @@ def initialize
 
   def add(values)
     values = [values] unless values.is_a?(Array) # box the value
-    values.compact.each do |val|
-      if val == :none
+    values = values.compact
+    if values.include?(:none)
+      if values.size == 1
         @auto = false
-        raise "Cannot set short to :none if short-chars have been defined '#{@chars}'" unless chars.empty?
-        next
+        return
       end
+      raise ArgumentError, "Cannot use :none with any other values in short option: #{values.inspect}"
+    end
+    values.each do |val|
       strval = val.to_s
       sopt = case strval
              when /^-(.)$/ then $1
diff --git a/test/optimist/alt_names_test.rb b/test/optimist/alt_names_test.rb
index c4d99ce..0e004df 100644
--- a/test/optimist/alt_names_test.rb
+++ b/test/optimist/alt_names_test.rb
@@ -18,7 +18,7 @@ def get_help_string
     end
 
     def test_altshort
-       @p.opt :catarg, "desc", :short => ["c", "-C"]
+      @p.opt :catarg, "desc", :short => ["c", "-C"]
        opts = @p.parse %w(-c)
        assert_equal true, opts[:catarg]
        opts = @p.parse %w(-C)
@@ -27,6 +27,18 @@ def test_altshort
        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