Fix menu selection gathering with the various options

String/Regexp/Hash was broken and integer had some issues some times. Also lack of support for symbol was inconvenient at times.
Lots of tests in place.

4 files changed, 177 insertions, 22 deletions

diff --git a/lib/highline.rb b/lib/highline.rb
index 0ef2b57..775a4ac 100755
--- a/lib/highline.rb
+++ b/lib/highline.rb
@@ -251,11 +251,18 @@ class HighLine
     menu.answer_type = menu.shell ? shell_style_lambda(menu) : menu.options
 
     selected = ask(menu)
+    return unless selected
 
     if menu.shell
-      menu.select(self, *selected)
+      selection, details = selected
     else
-      menu.select(self, selected)
+      selection = selected
+    end
+
+    if menu.gather
+      menu.gather_selected(self, selection, details)
+    else
+      menu.select(self, selection, details)
     end
   end
 
diff --git a/lib/highline/menu.rb b/lib/highline/menu.rb
index fbe493d..5d5e6b5 100644
--- a/lib/highline/menu.rb
+++ b/lib/highline/menu.rb
@@ -346,15 +346,39 @@ class HighLine
       items = all_items
 
       # Find the selected action.
-      item = if selection =~ /^\d+$/ # is a number?
+      selected_item = find_item_from_selection(items, selection)
+
+      # Run or return it.
+      @highline = highline_context
+      value_for_selected_item(selected_item, details)
+    end
+
+    def find_item_from_selection(items, selection)
+      if selection =~ /^\d+$/ # is a number?
         get_item_by_number(items, selection)
       else
         get_item_by_letter(items, selection)
       end
+    end
 
-      # Run or return it.
+    # Returns the menu item referenced by its index
+    # @param selection [Integer] menu item's index.
+    def get_item_by_number(items, selection)
+      items[selection.to_i - 1]
+    end
+
+    # Returns the menu item referenced by its title/header/name.
+    # @param selection [String] menu's title/header/name
+    def get_item_by_letter(items, selection)
+      item = items.find { |i| i.name == selection }
+      return item if item
+      l_index = "`" # character before the letter "a"
+      index = items.map { "#{l_index.succ!}" }.index(selection)
+      items[index]
+    end
+
+    def value_for_selected_item(item, details)
       if item.action
-        @highline = highline_context
         if @shell
           result = item.action.call(item.name, details)
         else
@@ -366,18 +390,36 @@ class HighLine
       end
     end
 
-    # Returns the menu item referenced by its index
-    # @param selection [Integer] menu item's index.
-    def get_item_by_number(items, selection)
-      items[selection.to_i - 1]
+    def gather_selected(highline_context, selections, details = nil)
+      @highline = highline_context
+      # add in any hidden menu commands
+      items = all_items
+
+      if selections.is_a?(Array)
+        value_for_array_selections(items, selections, details)
+      elsif selections.is_a?(Hash)
+        value_for_hash_selections(items, selections, details)
+      else
+        fail ArgumentError, 'selections must be either Array or Hash'
+      end
     end
 
-    # Returns the menu item referenced by its title/header/name.
-    # @param selection [String] menu's title/header/name
-    def get_item_by_letter(items, selection)
-      l_index = "`" # character before the letter "a"
-      index = items.map { "#{l_index.succ!}" }.index(selection)
-      items.find { |i| i.name == selection } or items[index]
+    def value_for_array_selections(items, selections, details)
+      # Find the selected items and return values
+      selected_items = selections.map do |selection|
+        find_item_from_selection(items, selection)
+      end
+      selected_items.map do |selected_item|
+        value_for_selected_item(selected_item, details)
+      end
+    end
+
+    def value_for_hash_selections(items, selections, details)
+      # Find the selected items and return in hash form
+      selections.each_with_object({}) do |(key, selection), memo|
+        selected_item = find_item_from_selection(items, selection)
+        memo[key] = value_for_selected_item(selected_item, details)
+      end
     end
 
     #
diff --git a/lib/highline/question_asker.rb b/lib/highline/question_asker.rb
index bb10c9e..06e6647 100644
--- a/lib/highline/question_asker.rb
+++ b/lib/highline/question_asker.rb
@@ -105,13 +105,11 @@ class HighLine
     # with keys provided by the Hash on {Question#gather}
     # @return [Hash]
     def gather_hash
-      answers = {}
-
-      question.gather.keys.sort.each do |key|
+      sorted_keys = question.gather.keys.sort_by(&:to_s)
+      sorted_keys.each_with_object({}) do |key, answers|
         @highline.key = key
         answers[key]  = ask_once
       end
-      answers
     end
 
 
@@ -133,15 +131,18 @@ class HighLine
     end
 
     def answer_matches_regex(answer)
-      (question.gather.is_a?(::String) && answer.to_s == question.gather) ||
-      (question.gather.is_a?(Regexp)   && answer.to_s =~ question.gather)
+      if question.gather.is_a?(::String) || question.gather.is_a?(Symbol)
+        answer.to_s == question.gather.to_s
+      else question.gather.is_a?(Regexp)
+        answer.to_s =~ question.gather
+      end
     end
 
     def gather_answers_based_on_type
       case question.gather
       when Integer
         gather_integer
-      when ::String, Regexp
+      when ::String, Symbol, Regexp
         gather_regexp
       when Hash
         gather_hash
diff --git a/test/test_menu.rb b/test/test_menu.rb
index 2bcd824..5da8331 100644
--- a/test/test_menu.rb
+++ b/test/test_menu.rb
@@ -496,4 +496,109 @@ class TestMenu < Minitest::Test
 
     assert_equal complete_interaction, @output.string
   end
+
+  def test_menu_gather_integer
+    @input << "Sample1\nlast\n"
+    @input.rewind
+
+    selected = @terminal.choose do |menu|
+      menu.gather = 2
+      menu.choice "Sample1"
+      menu.choice "Sample2"
+      menu.choice "last"
+    end
+    assert_equal(["Sample1", "last"], selected)
+
+    assert_equal("1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  ", @output.string)
+  end
+
+  def test_menu_gather_string
+    @input << "Sample1\nlast\n"
+    @input.rewind
+
+    selected = @terminal.choose do |menu|
+      menu.gather = :last
+      menu.choice "Sample1"
+      menu.choice "Sample2"
+      menu.choice :last
+    end
+    assert_equal(["Sample1"], selected)
+
+    assert_equal("1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  ", @output.string)
+  end
+
+  def test_menu_gather_symbol
+    @input << "Sample1\nlast\n"
+    @input.rewind
+
+    selected = @terminal.choose do |menu|
+      menu.gather = "last"
+      menu.choice "Sample1"
+      menu.choice "Sample2"
+      menu.choice "last"
+    end
+    assert_equal(["Sample1"], selected)
+
+    assert_equal("1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  ", @output.string)
+  end
+
+  def test_menu_gather_regexp
+    @input << "Sample1\nlast\n"
+    @input.rewind
+
+    selected = @terminal.choose do |menu|
+      menu.gather = /la/
+      menu.choice "Sample1"
+      menu.choice "Sample2"
+      menu.choice "last"
+    end
+    assert_equal(["Sample1"], selected)
+
+    assert_equal("1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  ", @output.string)
+  end
+
+  def test_menu_gather_hash
+    @input << "Sample1\n3\n"
+    @input.rewind
+
+    selected = @terminal.choose do |menu|
+      menu.gather = { "First" => true, second: true }
+      menu.choice "Sample1"
+      menu.choice "Sample2"
+      menu.choice "last"
+    end
+    assert_equal({ "First" => "Sample1", second: "last" }, selected)
+
+    assert_equal("1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  1. Sample1\n" +
+                 "2. Sample2\n" +
+                 "3. last\n" +
+                 "?  ", @output.string)
+  end
 end