Merge pull request #6989 from Homebrew/revert-6988-revert-6965-external-help-cli-parser

help: print cli/parser help message if used

Library/Homebrew/brew.rb

@@ -63,12 +63,12 @@ begin
 
   ENV["PATH"] = path
 
-  if cmd
-    internal_cmd = require? HOMEBREW_LIBRARY_PATH/"cmd"/cmd
+  require "commands"
 
-    unless internal_cmd
-      internal_dev_cmd = require? HOMEBREW_LIBRARY_PATH/"dev-cmd"/cmd
-      internal_cmd = internal_dev_cmd
+  if cmd
+    internal_cmd = Commands.valid_internal_cmd?(cmd)
+    internal_cmd ||= begin
+      internal_dev_cmd = Commands.valid_internal_dev_cmd?(cmd)
       if internal_dev_cmd && !ARGV.homebrew_developer?
         if (HOMEBREW_REPOSITORY/".git/config").exist?
           system "git", "config", "--file=#{HOMEBREW_REPOSITORY}/.git/config",
@@ -76,6 +76,7 @@ begin
         end
         ENV["HOMEBREW_DEV_CMD_RUN"] = "1"
       end
+      internal_dev_cmd
     end
   end
 
@@ -95,18 +96,19 @@ begin
   if (empty_argv || help_flag) && cmd != "cask"
     require "help"
     Homebrew::Help.help cmd, empty_argv: empty_argv
-    # `Homebrew.help` never returns, except for external/unknown commands.
+    # `Homebrew.help` never returns, except for unknown commands.
   end
 
-  if internal_cmd
-    Homebrew.send cmd.to_s.tr("-", "_").downcase
-  elsif which "brew-#{cmd}"
+  if internal_cmd || Commands.external_ruby_v2_cmd_path(cmd)
+    Homebrew.send Commands.method_name(cmd)
+  elsif (path = Commands.external_ruby_cmd_path(cmd))
+    require?(path)
+    exit Homebrew.failed? ? 1 : 0
+  elsif Commands.external_cmd_path(cmd)
     %w[CACHE LIBRARY_PATH].each do |env|
       ENV["HOMEBREW_#{env}"] = Object.const_get("HOMEBREW_#{env}").to_s
     end
     exec "brew-#{cmd}", *ARGV
-  elsif (path = which("brew-#{cmd}.rb")) && require?(path)
-    exit Homebrew.failed? ? 1 : 0
   else
     possible_tap = OFFICIAL_CMD_TAPS.find { |_, cmds| cmds.include?(cmd) }
     possible_tap = Tap.fetch(possible_tap.first) if possible_tap

Library/Homebrew/cli/parser.rb

@@ -16,6 +16,18 @@ module Homebrew
         new(args, &block).parse(args)
       end
 
+      def self.from_cmd_path(cmd_path)
+        cmd_args_method_name = Commands.args_method_name(cmd_path)
+
+        begin
+          Homebrew.send(cmd_args_method_name) if require?(cmd_path)
+        rescue NoMethodError => e
+          raise if e.name != cmd_args_method_name
+
+          nil
+        end
+      end
+
       def self.global_options
         {
           quiet:   [["-q", "--quiet"], :quiet, "Suppress any warnings."],
@@ -159,7 +171,8 @@ module Homebrew
       end
 
       def generate_help_text
-        @parser.to_s.sub(/^/, "#{Tty.bold}Usage: brew#{Tty.reset} ")
+        @parser.to_s
+               .sub(/^/, "#{Tty.bold}Usage: brew#{Tty.reset} ")
                .gsub(/`(.*?)`/m, "#{Tty.bold}\\1#{Tty.reset}")
                .gsub(%r{<([^\s]+?://[^\s]+?)>}) { |url| Formatter.url(url) }
                .gsub(/<(.*?)>/m, "#{Tty.underline}\\1#{Tty.reset}")

Library/Homebrew/cmd/command.rb

@@ -23,13 +23,8 @@ module Homebrew
 
     raise UsageError, "This command requires a command argument" if args.remaining.empty?
 
-    args.remaining.each do |c|
-      cmd = HOMEBREW_INTERNAL_COMMAND_ALIASES.fetch(c, c)
+    args.remaining.each do |cmd|
       path = Commands.path(cmd)
-      cmd_paths = PATH.new(ENV["PATH"]).append(Tap.cmd_directories) unless path
-      path ||= which("brew-#{cmd}", cmd_paths)
-      path ||= which("brew-#{cmd}.rb", cmd_paths)
-
       odie "Unknown command: #{cmd}" unless path
       puts path
     end

Library/Homebrew/cmd/commands.rb

@@ -27,54 +27,18 @@ module Homebrew
     commands_args.parse
 
     if args.quiet?
-      cmds = internal_commands
-      cmds += external_commands
-      cmds += internal_developer_commands
-      cmds += HOMEBREW_INTERNAL_COMMAND_ALIASES.keys if args.include_aliases?
-      puts Formatter.columns(cmds.sort)
+      puts Formatter.columns(Commands.commands(aliases: args.include_aliases?))
       return
     end
 
-    # Find commands in Homebrew/cmd
-    ohai "Built-in commands", Formatter.columns(internal_commands.sort)
-
-    # Find commands in Homebrew/dev-cmd
+    ohai "Built-in commands", Formatter.columns(Commands.internal_commands)
     puts
-    ohai "Built-in developer commands", Formatter.columns(internal_developer_commands.sort)
+    ohai "Built-in developer commands", Formatter.columns(Commands.internal_developer_commands)
 
-    exts = external_commands
-    return if exts.empty?
+    external_commands = Commands.external_commands
+    return if external_commands.blank?
 
-    # Find commands in the PATH
     puts
-    ohai "External commands", Formatter.columns(exts)
-  end
-
-  def internal_commands
-    find_internal_commands HOMEBREW_LIBRARY_PATH/"cmd"
-  end
-
-  def internal_developer_commands
-    find_internal_commands HOMEBREW_LIBRARY_PATH/"dev-cmd"
-  end
-
-  def external_commands
-    cmd_paths = PATH.new(ENV["PATH"]).append(Tap.cmd_directories)
-    cmd_paths.each_with_object([]) do |path, cmds|
-      Dir["#{path}/brew-*"].each do |file|
-        next unless File.executable?(file)
-
-        cmd = File.basename(file, ".rb")[5..-1]
-        next if cmd.include?(".")
-
-        cmds << cmd
-      end
-    end.sort
-  end
-
-  def find_internal_commands(directory)
-    Pathname.glob(directory/"*")
-            .select(&:file?)
-            .map { |f| f.basename.to_s.sub(/\.(?:rb|sh)$/, "") }
+    ohai "External commands", Formatter.columns(external_commands)
   end
 end

Library/Homebrew/commands.rb

 # frozen_string_literal: true
 
 module Commands
-  def self.path(cmd)
+  module_function
+
+  HOMEBREW_CMD_PATH = (HOMEBREW_LIBRARY_PATH/"cmd").freeze
+  HOMEBREW_DEV_CMD_PATH = (HOMEBREW_LIBRARY_PATH/"dev-cmd").freeze
+  HOMEBREW_INTERNAL_COMMAND_ALIASES = {
+    "ls"          => "list",
+    "homepage"    => "home",
+    "-S"          => "search",
+    "up"          => "update",
+    "ln"          => "link",
+    "instal"      => "install", # gem does the same
+    "uninstal"    => "uninstall",
+    "rm"          => "uninstall",
+    "remove"      => "uninstall",
+    "configure"   => "diy",
+    "abv"         => "info",
+    "dr"          => "doctor",
+    "--repo"      => "--repository",
+    "environment" => "--env",
+    "--config"    => "config",
+    "-v"          => "--version",
+  }.freeze
+
+  def valid_internal_cmd?(cmd)
+    require?(HOMEBREW_CMD_PATH/cmd)
+  end
+
+  def valid_internal_dev_cmd?(cmd)
+    require?(HOMEBREW_DEV_CMD_PATH/cmd)
+  end
+
+  def method_name(cmd)
+    cmd.to_s
+       .tr("-", "_")
+       .downcase
+       .to_sym
+  end
+
+  def args_method_name(cmd_path)
+    cmd_path_basename = basename_without_extension(cmd_path)
+    cmd_method_prefix = method_name(cmd_path_basename)
+    "#{cmd_method_prefix}_args".to_sym
+  end
+
+  def internal_cmd_path(cmd)
     [
-      HOMEBREW_LIBRARY_PATH/"cmd/#{cmd}.sh",
-      HOMEBREW_LIBRARY_PATH/"dev-cmd/#{cmd}.sh",
-      HOMEBREW_LIBRARY_PATH/"cmd/#{cmd}.rb",
-      HOMEBREW_LIBRARY_PATH/"dev-cmd/#{cmd}.rb",
+      HOMEBREW_CMD_PATH/"#{cmd}.rb",
+      HOMEBREW_CMD_PATH/"#{cmd}.sh",
     ].find(&:exist?)
   end
+
+  def internal_dev_cmd_path(cmd)
+    [
+      HOMEBREW_DEV_CMD_PATH/"#{cmd}.rb",
+      HOMEBREW_DEV_CMD_PATH/"#{cmd}.sh",
+    ].find(&:exist?)
+  end
+
+  # Ruby commands which can be `require`d without being run.
+  def external_ruby_v2_cmd_path(cmd)
+    path = which("#{cmd}.rb", Tap.cmd_directories)
+    path if require?(path)
+  end
+
+  # Ruby commands which are run by being `require`d.
+  def external_ruby_cmd_path(cmd)
+    which("brew-#{cmd}.rb", PATH.new(ENV["PATH"]).append(Tap.cmd_directories))
+  end
+
+  def external_cmd_path(cmd)
+    which("brew-#{cmd}", PATH.new(ENV["PATH"]).append(Tap.cmd_directories))
+  end
+
+  def path(cmd)
+    internal_cmd = HOMEBREW_INTERNAL_COMMAND_ALIASES.fetch(cmd, cmd)
+    path ||= internal_cmd_path(internal_cmd)
+    path ||= internal_dev_cmd_path(internal_cmd)
+    path ||= external_ruby_v2_cmd_path(cmd)
+    path ||= external_ruby_cmd_path(cmd)
+    path ||= external_cmd_path(cmd)
+    path
+  end
+
+  def commands(aliases: false)
+    cmds = internal_commands
+    cmds += internal_developer_commands
+    cmds += external_commands
+    cmds += internal_commands_aliases if aliases
+    cmds.sort
+  end
+
+  def internal_commands_paths
+    find_commands HOMEBREW_CMD_PATH
+  end
+
+  def internal_developer_commands_paths
+    find_commands HOMEBREW_DEV_CMD_PATH
+  end
+
+  def internal_commands
+    find_internal_commands(HOMEBREW_CMD_PATH).map(&:to_s)
+  end
+
+  def internal_developer_commands
+    find_internal_commands(HOMEBREW_DEV_CMD_PATH).map(&:to_s)
+  end
+
+  def internal_commands_aliases
+    HOMEBREW_INTERNAL_COMMAND_ALIASES.keys
+  end
+
+  def find_internal_commands(path)
+    find_commands(path).map(&:basename)
+                       .map(&method(:basename_without_extension))
+  end
+
+  def external_commands
+    Tap.cmd_directories.flat_map do |path|
+      find_commands(path).select(&:executable?)
+                         .map(&method(:basename_without_extension))
+                         .map { |p| p.to_s.sub(/^brew(cask)?-/, '\1 ').strip }
+    end.map(&:to_s)
+       .sort
+  end
+
+  def basename_without_extension(path)
+    path.basename(path.extname)
+  end
+
+  def find_commands(path)
+    Pathname.glob("#{path}/*")
+            .select(&:file?)
+            .sort
+  end
 end

Library/Homebrew/dev-cmd/man.rb

@@ -4,8 +4,6 @@ require "formula"
 require "erb"
 require "ostruct"
 require "cli/parser"
-# Require all commands
-Dir.glob("#{HOMEBREW_LIBRARY_PATH}/{dev-,}cmd/*.rb").sort.each { |cmd| require cmd }
 
 module Homebrew
   module_function
@@ -61,8 +59,8 @@ module Homebrew
     template = (SOURCE_PATH/"brew.1.md.erb").read
     variables = OpenStruct.new
 
-    variables[:commands] = generate_cmd_manpages("#{HOMEBREW_LIBRARY_PATH}/cmd/*.{rb,sh}")
-    variables[:developer_commands] = generate_cmd_manpages("#{HOMEBREW_LIBRARY_PATH}/dev-cmd/{*.rb,sh}")
+    variables[:commands] = generate_cmd_manpages(Commands.internal_commands_paths)
+    variables[:developer_commands] = generate_cmd_manpages(Commands.internal_developer_commands_paths)
     variables[:global_options] = global_options_manpage
 
     readme = HOMEBREW_REPOSITORY/"README.md"
@@ -144,26 +142,19 @@ module Homebrew
     end
   end
 
-  def generate_cmd_manpages(glob)
-    cmd_paths = Pathname.glob(glob).sort
+  def generate_cmd_manpages(cmd_paths)
     man_page_lines = []
     man_args = Homebrew.args
     # preserve existing manpage order
     cmd_paths.sort_by(&method(:sort_key_for_path))
              .each do |cmd_path|
-      cmd_args_method_name = cmd_arg_parser(cmd_path)
-
-      cmd_man_page_lines = begin
-        cmd_parser = Homebrew.send(cmd_args_method_name)
+      cmd_man_page_lines = if cmd_parser = CLI::Parser.from_cmd_path(cmd_path)
         next if cmd_parser.hide_from_man_page
 
         cmd_parser_manpage_lines(cmd_parser).join
-      rescue NoMethodError => e
-        raise if e.name != cmd_args_method_name
-
-        nil
+      else
+        cmd_comment_manpage_lines(cmd_path)
       end
-      cmd_man_page_lines ||= cmd_comment_manpage_lines(cmd_path)
 
       man_page_lines << cmd_man_page_lines
     end
@@ -171,10 +162,6 @@ module Homebrew
     man_page_lines.compact.join("\n")
   end
 
-  def cmd_arg_parser(cmd_path)
-    "#{cmd_path.basename.to_s.gsub(".rb", "").tr("-", "_")}_args".to_sym
-  end
-
   def cmd_parser_manpage_lines(cmd_parser)
     lines = [format_usage_banner(cmd_parser.usage_banner_text)]
     lines += cmd_parser.processed_options.map do |short, long, _, desc|

Library/Homebrew/global.rb

@@ -115,25 +115,6 @@ rescue
   nil
 end.compact.freeze
 
-HOMEBREW_INTERNAL_COMMAND_ALIASES = {
-  "ls"          => "list",
-  "homepage"    => "home",
-  "-S"          => "search",
-  "up"          => "update",
-  "ln"          => "link",
-  "instal"      => "install", # gem does the same
-  "uninstal"    => "uninstall",
-  "rm"          => "uninstall",
-  "remove"      => "uninstall",
-  "configure"   => "diy",
-  "abv"         => "info",
-  "dr"          => "doctor",
-  "--repo"      => "--repository",
-  "environment" => "--env",
-  "--config"    => "config",
-  "-v"          => "--version",
-}.freeze
-
 require "set"
 
 require "extend/pathname"

Library/Homebrew/help.rb

@@ -42,16 +42,11 @@ module Homebrew
 
     def help(cmd = nil, flags = {})
       # Resolve command aliases and find file containing the implementation.
-      if cmd
-        cmd = HOMEBREW_INTERNAL_COMMAND_ALIASES.fetch(cmd, cmd)
-        path = Commands.path(cmd)
-        path ||= which("brew-#{cmd}")
-        path ||= which("brew-#{cmd}.rb")
-      end
+      path = Commands.path(cmd) if cmd
 
       # Display command-specific (or generic) help in response to `UsageError`.
       if (error_message = flags[:usage_error])
-        $stderr.puts path ? command_help(path) : HOMEBREW_HELP
+        $stderr.puts path ? command_help(cmd, path) : HOMEBREW_HELP
         $stderr.puts
         onoe error_message
         exit 1
@@ -73,36 +68,48 @@ module Homebrew
       return if path.nil?
 
       # Display help for internal command (or generic help if undocumented).
-      puts command_help(path)
+      puts command_help(cmd, path)
       exit 0
     end
 
-    def command_help(path)
-      # Let OptionParser generate help text for commands which have a parser defined
-      cmd = path.basename(path.extname)
-      cmd_args_method_name = "#{cmd.to_s.tr("-", "_")}_args".to_sym
-      begin
-        return Homebrew.send(cmd_args_method_name)
-                       .generate_help_text
-      rescue NoMethodError => e
-        raise if e.name != cmd_args_method_name
-
-        nil
+    def command_help(cmd, path)
+      # Only some types of commands can have a parser.
+      output = if Commands.valid_internal_cmd?(cmd) ||
+                  Commands.valid_internal_dev_cmd?(cmd) ||
+                  Commands.external_ruby_v2_cmd_path(cmd)
+        parser_help(path)
       end
 
-      help_lines = command_help_lines(path)
-      if help_lines.empty?
+      output ||= comment_help(path)
+
+      output ||= if output.blank?
         opoo "No help text in: #{path}" if ARGV.homebrew_developer?
         HOMEBREW_HELP
-      else
-        Formatter.wrap(help_lines.join.gsub(/^  /, ""), COMMAND_DESC_WIDTH)
-                 .sub("@hide_from_man_page ", "")
-                 .sub(/^\* /, "#{Tty.bold}Usage: brew#{Tty.reset} ")
-                 .gsub(/`(.*?)`/m, "#{Tty.bold}\\1#{Tty.reset}")
-                 .gsub(%r{<([^\s]+?://[^\s]+?)>}) { |url| Formatter.url(url) }
-                 .gsub(/<(.*?)>/m, "#{Tty.underline}\\1#{Tty.reset}")
-                 .gsub(/\*(.*?)\*/m, "#{Tty.underline}\\1#{Tty.reset}")
       end
+
+      output
+    end
+
+    def parser_help(path)
+      # Let OptionParser generate help text for commands which have a parser.
+      cmd_parser = CLI::Parser.from_cmd_path(path)
+      return unless cmd_parser
+
+      cmd_parser.generate_help_text
+    end
+
+    def comment_help(path)
+      # Otherwise read #: lines from the file.
+      help_lines = command_help_lines(path)
+      return if help_lines.blank?
+
+      Formatter.wrap(help_lines.join.gsub(/^  /, ""), COMMAND_DESC_WIDTH)
+               .sub("@hide_from_man_page ", "")
+               .sub(/^\* /, "#{Tty.bold}Usage: brew#{Tty.reset} ")
+               .gsub(/`(.*?)`/m, "#{Tty.bold}\\1#{Tty.reset}")
+               .gsub(%r{<([^\s]+?://[^\s]+?)>}) { |url| Formatter.url(url) }
+               .gsub(/<(.*?)>/m, "#{Tty.underline}\\1#{Tty.reset}")
+               .gsub(/\*(.*?)\*/m, "#{Tty.underline}\\1#{Tty.reset}")
     end
   end
 end

Library/Homebrew/test/cmd/commands_spec.rb

 # frozen_string_literal: true
 
-require "cmd/command"
 require "cmd/commands"
 require "fileutils"
 
@@ -18,82 +17,3 @@ describe "brew commands", :integration_test do
       .and be_a_success
   end
 end
-
-RSpec.shared_context "custom internal commands" do
-  let(:cmds) do
-    [
-      # internal commands
-      HOMEBREW_LIBRARY_PATH/"cmd/rbcmd.rb",
-      HOMEBREW_LIBRARY_PATH/"cmd/shcmd.sh",
-
-      # internal developer-commands
-      HOMEBREW_LIBRARY_PATH/"dev-cmd/rbdevcmd.rb",
-      HOMEBREW_LIBRARY_PATH/"dev-cmd/shdevcmd.sh",
-    ]
-  end
-
-  around do |example|
-    cmds.each do |f|
-      FileUtils.touch f
-    end
-
-    example.run
-  ensure
-    FileUtils.rm_f cmds
-  end
-end
-
-describe Homebrew do
-  include_context "custom internal commands"
-
-  specify "::internal_commands" do
-    cmds = described_class.internal_commands
-    expect(cmds).to include("rbcmd"), "Ruby commands files should be recognized"
-    expect(cmds).to include("shcmd"), "Shell commands files should be recognized"
-    expect(cmds).not_to include("rbdevcmd"), "Dev commands shouldn't be included"
-  end
-
-  specify "::internal_developer_commands" do
-    cmds = described_class.internal_developer_commands
-    expect(cmds).to include("rbdevcmd"), "Ruby commands files should be recognized"
-    expect(cmds).to include("shdevcmd"), "Shell commands files should be recognized"
-    expect(cmds).not_to include("rbcmd"), "Non-dev commands shouldn't be included"
-  end
-
-  specify "::external_commands" do
-    mktmpdir do |dir|
-      %w[brew-t1 brew-t2.rb brew-t3.py].each do |file|
-        path = "#{dir}/#{file}"
-        FileUtils.touch path
-        FileUtils.chmod 0755, path
-      end
-
-      FileUtils.touch "#{dir}/brew-t4"
-
-      ENV["PATH"] += "#{File::PATH_SEPARATOR}#{dir}"
-      cmds = described_class.external_commands
-
-      expect(cmds).to include("t1"), "Executable files should be included"
-      expect(cmds).to include("t2"), "Executable Ruby files should be included"
-      expect(cmds).not_to include("t3"), "Executable files with a non Ruby extension shouldn't be included"
-      expect(cmds).not_to include("t4"), "Non-executable files shouldn't be included"
-    end
-  end
-end
-
-describe Commands do
-  include_context "custom internal commands"
-
-  describe "::path" do
-    specify "returns the path for an internal command" do
-      expect(described_class.path("rbcmd")).to eq(HOMEBREW_LIBRARY_PATH/"cmd/rbcmd.rb")
-      expect(described_class.path("shcmd")).to eq(HOMEBREW_LIBRARY_PATH/"cmd/shcmd.sh")
-      expect(described_class.path("idontexist1234")).to be nil
-    end
-
-    specify "returns the path for an internal developer-command" do
-      expect(described_class.path("rbdevcmd")).to eq(HOMEBREW_LIBRARY_PATH/"dev-cmd/rbdevcmd.rb")
-      expect(described_class.path("shdevcmd")).to eq(HOMEBREW_LIBRARY_PATH/"dev-cmd/shdevcmd.sh")
-    end
-  end
-end

Library/Homebrew/test/commands_spec.rb

+# frozen_string_literal: true
+
+require "commands"
+
+RSpec.shared_context "custom internal commands" do
+  let(:cmds) do
+    [
+      # internal commands
+      Commands::HOMEBREW_CMD_PATH/"rbcmd.rb",
+      Commands::HOMEBREW_CMD_PATH/"shcmd.sh",
+
+      # internal developer-commands
+      Commands::HOMEBREW_DEV_CMD_PATH/"rbdevcmd.rb",
+      Commands::HOMEBREW_DEV_CMD_PATH/"shdevcmd.sh",
+    ]
+  end
+
+  around do |example|
+    cmds.each do |f|
+      FileUtils.touch f
+    end
+
+    example.run
+  ensure
+    FileUtils.rm_f cmds
+  end
+end
+
+describe Commands do
+  include_context "custom internal commands"
+