Improve knife --help with topic-based command groups

lib/chef/application/knife.rb

@@ -212,8 +212,10 @@ def want_version?
   def print_help_and_exit(exitcode = 1, fatal_message = nil)
     Chef::Log.error(fatal_message) if fatal_message
 
+    topic = help_topic
+
     begin
-      parse_options
+      parse_options if topic.nil?
     rescue OptionParser::InvalidOption => e
       puts "#{e}\n"
     end
@@ -226,10 +228,29 @@ def print_help_and_exit(exitcode = 1, fatal_message = nil)
       puts
     end
 
-    puts opt_parser
-    puts
-    Chef::Knife.list_commands
+    if topic.nil? || topic == "all"
+      puts opt_parser
+      puts
+    end
+
+    if want_help?
+      help_result = Chef::Knife.show_help(topic)
+      exitcode = 1 if help_result == :unknown_topic && exitcode == 0
+    else
+      Chef::Knife.list_commands
+    end
+
     exit exitcode
   end
 
+  def help_topic
+    return nil unless want_help?
+
+    raw_topic = ARGV[1]
+    return nil if raw_topic.nil? || raw_topic.empty?
+    return nil if raw_topic.start_with?("--")
+
+    Chef::Knife.normalize_help_topic(raw_topic)
+  end
+
 end

lib/chef/knife.rb

@@ -266,11 +266,42 @@ def self.load_deps
 
     OFFICIAL_PLUGINS = %w{lpar openstack push rackspace vcenter}.freeze
 
+    HELP_TOPIC_ORDER = %w{all infra content security remote cloud repo ops plugins search}.freeze
+
+    HELP_TOPIC_HINTS = {
+      "all" => "full command list",
+      "infra" => "node, role, environment, tag",
+      "content" => "cookbook, data bag, recipe, supermarket, yaml",
+      "security" => "org, acl, group, user, client, vault, ssl",
+      "remote" => "bootstrap, ssh, winrm, wsman, windows",
+      "cloud" => "ec2, google",
+      "repo" => "path-based, raw, serve",
+      "ops" => "config, configure, rehash, license, exec, status",
+      "plugins" => "installed plugin command families",
+      "search" => "search",
+    }.freeze
+
+    HELP_TOPIC_CATEGORIES = {
+      "infra" => %w{node role environment tag},
+      "content" => ["cookbook", "data bag", "recipe", "supermarket", "yaml"],
+      "security" => ["CHEF ORGANIZATION MANAGEMENT", "acl", "group", "user", "client", "vault", "ssl"],
+      "remote" => %w{bootstrap ssh winrm wsman windows},
+      "cloud" => %w{ec2 google},
+      "repo" => %w{path-based raw serve},
+      "ops" => %w{config configure rehash license exec status knife},
+      "search" => %w{search},
+    }.freeze
+
+    CORE_HELP_CATEGORIES = HELP_TOPIC_CATEGORIES.values.flatten.uniq.freeze
+
     class << self
-      def list_commands(preferred_category = nil)
-        category_desc = preferred_category ? preferred_category + " " : ""
+      def list_commands(preferred_category = nil, categories: nil, category_desc: nil)
+        category_desc ||= preferred_category ? preferred_category + " " : ""
         msg "Available #{category_desc}subcommands: (for details, knife SUB-COMMAND --help)\n\n"
-        subcommand_loader.list_commands(preferred_category).sort.each do |category, commands|
+        command_groups = subcommand_loader.list_commands(preferred_category)
+        command_groups = command_groups.select { |category, _| categories.include?(category) } if categories
+
+        command_groups.sort.each do |category, commands|
           next if /deprecated/i.match?(category)
 
           msg "** #{category.upcase} COMMANDS **"
@@ -282,6 +313,67 @@ def list_commands(preferred_category = nil)
         end
       end
 
+      def normalize_help_topic(raw_topic)
+        return nil if raw_topic.nil?
+
+        topic = raw_topic.to_s.strip
+        return nil if topic.empty?
+
+        topic = topic[1..] if topic.start_with?("-")
+        topic = topic.downcase
+        topic = "plugins" if topic == "plugin"
+        topic
+      end
+
+      def show_help(topic = nil)
+        return show_help_topics if topic.nil?
+
+        normalized_topic = normalize_help_topic(topic)
+        return show_unknown_help_topic(topic) if normalized_topic.nil?
+
+        return list_commands if normalized_topic == "all"
+
+        if normalized_topic == "plugins"
+          show_plugin_help
+          return show_help_topics
+        end
+
+        categories = HELP_TOPIC_CATEGORIES[normalized_topic]
+        return show_unknown_help_topic(topic) unless categories
+
+        list_commands(nil, categories: categories, category_desc: normalized_topic + " ")
+        show_help_topics
+      end
+
+      def show_help_topics
+        msg "For help on specific knife commands, see the command groups below."
+        HELP_TOPIC_ORDER.each do |topic|
+          msg format("  knife --help -%-8s (%s)", topic, HELP_TOPIC_HINTS[topic])
+        end
+        msg
+        :ok
+      end
+
+      def show_plugin_help
+        all_categories = subcommand_loader.list_commands.keys
+        plugin_categories = all_categories - CORE_HELP_CATEGORIES
+        plugin_categories.reject! { |category| /deprecated/i.match?(category) }
+
+        if plugin_categories.empty?
+          msg "No plugin command groups detected in this environment."
+          return :ok
+        end
+
+        list_commands(nil, categories: plugin_categories, category_desc: "plugins ")
+      end
+
+      def show_unknown_help_topic(topic)
+        msg "Unknown help topic: #{topic}"
+        msg "Valid topics: #{HELP_TOPIC_ORDER.join(", ")}"
+        msg "Use: knife --help -all"
+        :unknown_topic
+      end
+
       private
 
       # @api private

spec/unit/application/knife_spec.rb

@@ -44,9 +44,41 @@ def run; end
     @knife = Chef::Application::Knife.new
     allow(@knife).to receive(:puts)
     allow(@knife).to receive(:trap)
+    allow(Chef::Knife).to receive(:show_help).and_return(:ok)
+    allow(Chef::Knife).to receive(:normalize_help_topic).and_call_original
     allow(Chef::Knife).to receive(:list_commands)
   end
 
+  it "shows short help topics for --help" do
+    with_argv("--help") do
+      expect(@knife).to receive(:puts).with(@knife.opt_parser)
+      expect(Chef::Knife).to receive(:show_help).with(nil).and_return(:ok)
+      expect { @knife.run }.to raise_error(SystemExit) { |e| expect(e.status).to eq(0) }
+    end
+  end
+
+  it "routes --help -all to the full help topic" do
+    with_argv("--help", "-all") do
+      expect(Chef::Knife).to receive(:show_help).with("all").and_return(:ok)
+      expect { @knife.run }.to raise_error(SystemExit) { |e| expect(e.status).to eq(0) }
+    end
+  end
+
+  it "routes --help TOPIC to topic help" do
+    with_argv("--help", "cloud") do
+      expect(@knife).not_to receive(:puts).with(@knife.opt_parser)
+      expect(Chef::Knife).to receive(:show_help).with("cloud").and_return(:ok)
+      expect { @knife.run }.to raise_error(SystemExit) { |e| expect(e.status).to eq(0) }
+    end
+  end
+
+  it "exits non-zero for unknown help topics" do
+    with_argv("--help", "-unknown-topic") do
+      expect(Chef::Knife).to receive(:show_help).with("unknown-topic").and_return(:unknown_topic)
+      expect { @knife.run }.to raise_error(SystemExit) { |e| expect(e.status).to eq(1) }
+    end
+  end
+
   it "should exit 1 and print the options if no arguments are given at all" do
     with_argv([]) do
       expect { @knife.run }.to raise_error(SystemExit) { |e| expect(e.status).to eq(1) }