Merge pull request #197 from JEG2/feat_improve_menu

Some improvements to HighLine::Menu

5 files changed, 74 insertions, 51 deletions

diff --git a/Changelog.md b/Changelog.md
index 96a91df..8e3fab3 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -2,6 +2,10 @@
 
 Below is a complete listing of changes for each revision of HighLine.
 
+* PR #197 - Some HighLine::Menu improvements
+  * Move Menu::MenuItem to Menu::Item with its own file
+  * Some small refactorings
+
 ### 2.0.0-develop.8 / 2016-06-03
 
 * PR #195 - Add PRONTO to development group at Gemfile by Abinoam Jr. (@abinoam)
diff --git a/Gemfile b/Gemfile
index be5b329..ca1d209 100644
--- a/Gemfile
+++ b/Gemfile
@@ -22,4 +22,5 @@ group :development do
   # is not merged
   gem 'flay', '2.7.0'
   gem 'pronto-flay', '0.6.1', require: false
+  gem 'flog'
 end
diff --git a/lib/highline/menu.rb b/lib/highline/menu.rb
index 2adc244..8dc9247 100644
--- a/lib/highline/menu.rb
+++ b/lib/highline/menu.rb
@@ -9,6 +9,7 @@
 #  This is Free Software.  See LICENSE and COPYING for details.
 
 require "highline/question"
+require "highline/menu/item"
 
 class HighLine
   #
@@ -186,7 +187,7 @@ class HighLine
     #   end
 
     def choice( name, help = nil, text = nil, &action )
-      item = MenuItem.new(name, text: text, help: help, action: action)
+      item = Menu::Item.new(name, text: text, help: help, action: action)
       @items << item
       @help.merge!(item.item_help)
       update_responses  # rebuild responses based on our settings
@@ -194,21 +195,21 @@ class HighLine
 
     #
     # This method helps reduce the namespaces in the original call, which would look
-    # like this: HighLine::Menu::MenuItem.new(...)
+    # like this: HighLine::Menu::Item.new(...)
     # With #build_item, it looks like this: menu.build_item(...)
     # @param *args splat args, the same args you would pass to an initialization of
-    #        HighLine::Menu::MenuItem
-    # @return [HighLine::Menu::MenuItem] the menu item
+    #        HighLine::Menu::Item
+    # @return [HighLine::Menu::Item] the menu item
 
     def build_item(*args)
-      MenuItem.new(*args)
+      Menu::Item.new(*args)
     end
 
     #
     # Adds an item directly to the menu. If you want more configuraiton or options,
     # use this method
     #
-    # @param item [Menu::MenuItem] item containing choice fields and more
+    # @param item [Menu::Item] item containing choice fields and more
     # @return [void]
 
     def add_item(item)
@@ -242,7 +243,7 @@ class HighLine
     # @return (see #choice)
 
     def hidden( name, help = nil, &action )
-      item = MenuItem.new(name, text: name, help: help, action: action)
+      item = Menu::Item.new(name, text: name, help: help, action: action)
       @hidden_items << item
       @help.merge!(item.item_help)
     end
@@ -353,29 +354,30 @@ class HighLine
     # This method returns all possible options for auto-completion, based
     # on the settings of _index_ and _select_by_.
     #
-    def options(  )
-      # add in any hidden menu commands
-      items = all_items
-
+    def options
       case @select_by
-      when :index then
-        map_items_by_index(items, @index)
+      when :index
+        map_items_by_index
       when :name
-        items.map(&:name)
+        map_items_by_name
       else
-        map_items_by_index(items, @index) + items.map(&:name)
+        map_items_by_index + map_items_by_name
       end
     end
 
-    def map_items_by_index(items, index = nil)
-      if index == :letter
+    def map_items_by_index
+      if @index == :letter
         l_index = "`"
-        items.map { "#{l_index.succ!}" }
+        all_items.map { l_index.succ!.dup }
       else
-        (1 .. items.size).map(&:to_s)
+        (1..all_items.size).map(&:to_s)
       end
     end
 
+    def map_items_by_name
+      all_items.map(&:name)
+    end
+
     def all_items
       @items + @hidden_items
     end
@@ -484,17 +486,25 @@ class HighLine
     # This method returns all menu items to be displayed, complete with
     # indexes.
     #
-    def to_ary(  )
+    def to_ary
+      @items.map.with_index { |item, ix| decorate_item(item.text.to_s, ix) }
+    end
+
+    def decorate_item(text, ix)
+      decorated, non_decorated = mark_for_decoration(text, ix)
+      decorate_index(decorated) + non_decorated
+    end
+
+    def mark_for_decoration(text, ix)
       case @index
       when :number
-        @items.map { |i| decorate_index("#{@items.index(i) + 1}#{@index_suffix}") + "#{i.text}" }
+        ["#{ix + 1}#{@index_suffix}", text]
       when :letter
-        l_index = "`"
-        @items.map { |i| decorate_index("#{l_index.succ!}#{@index_suffix}") + "#{i.text}" }
+        ["#{('a'.ord + ix).chr}#{@index_suffix}", text]
       when :none
-        @items.map { |i| decorate_index("#{i.text}") }
+        [text, ""]
       else
-        @items.map { |i| decorate_index("#{index}#{@index_suffix}") + "#{i.text}" }
+        ["#{index}#{@index_suffix}", text]
       end
     end
 
@@ -532,27 +542,5 @@ class HighLine
     def update_responses
       build_responses(options)
     end
-
-    class MenuItem
-      attr_reader :name, :text, :help, :action
-
-      #
-      # @param name [String] The name that is matched against the user input
-      # @param text: [String] The text that displays for that choice (defaults to name)
-      # @param help: [String] help, see above (not sure how it works)
-      # @param action: [Block] a block that gets called when choice is selected
-      #
-      def initialize(name, attributes)
-        @name = name
-        @text = attributes[:text] || @name
-        @help = attributes[:help]
-        @action = attributes[:action]
-      end
-
-      def item_help
-        return {} unless help
-        { name.to_s.downcase => help }
-      end
-    end
   end
 end
diff --git a/lib/highline/menu/item.rb b/lib/highline/menu/item.rb
new file mode 100644
index 0000000..76e1e56
--- /dev/null
+++ b/lib/highline/menu/item.rb
@@ -0,0 +1,30 @@
+class HighLine
+  class Menu < Question
+    # Represents an Item of a HighLine::Menu.
+    #
+    class Item
+      attr_reader :name, :text, :help, :action
+
+      #
+      # @param name [String] The name that is matched against the user input
+      # @param attributes [Hash] options Hash to tailor menu item to your needs
+      # @option attributes text: [String] The text that displays for that
+      #   choice (defaults to name)
+      # @option attributes help: [String] help/hint string to be displayed.
+      # @option attributes action: [Block] a block that gets called when choice
+      #   is selected
+      #
+      def initialize(name, attributes)
+        @name = name
+        @text = attributes[:text] || @name
+        @help = attributes[:help]
+        @action = attributes[:action]
+      end
+
+      def item_help
+        return {} unless help
+        { name.to_s.downcase => help }
+      end
+    end
+  end
+end
diff --git a/test/test_menu.rb b/test/test_menu.rb
index bf4bcb5..b654ef3 100644
--- a/test/test_menu.rb
+++ b/test/test_menu.rb
@@ -114,8 +114,8 @@ class TestMenu < Minitest::Test
     @input.rewind
 
     selected = @terminal.choose do |menu|
-      menu.add_item(HighLine::Menu::MenuItem.new("Sample1", text: "Sample2"))
-      menu.add_item(HighLine::Menu::MenuItem.new("Sample2", text: "Sample1"))
+      menu.add_item(HighLine::Menu::Item.new("Sample1", text: "Sample2"))
+      menu.add_item(HighLine::Menu::Item.new("Sample2", text: "Sample1"))
     end
     assert_equal(selected, "Sample1")
     assert_equal("1. Sample2\n" +
@@ -128,8 +128,8 @@ class TestMenu < Minitest::Test
     @input.rewind
 
     selected = @terminal.choose do |menu|
-      menu.add_item(HighLine::Menu::MenuItem.new("Sample1", text: "Sample2"))
-      menu.add_item(HighLine::Menu::MenuItem.new("Sample2", text: "Sample1"))
+      menu.add_item(HighLine::Menu::Item.new("Sample1", text: "Sample2"))
+      menu.add_item(HighLine::Menu::Item.new("Sample2", text: "Sample1"))
     end
     assert_equal(selected, "Sample2")
     assert_equal("1. Sample2\n" +