Merge pull request #3423 from MikeMcQuaid/commands-refactoring-fix

Refactor command handling code

Library/Homebrew/brew.rb

@@ -33,7 +33,6 @@ begin
   empty_argv = ARGV.empty?
   help_flag_list = %w[-h --help --usage -?]
   help_flag = !ENV["HOMEBREW_HELP"].nil?
-  internal_cmd = true
   cmd = nil
 
   ARGV.dup.each_with_index do |arg, i|
@@ -60,8 +59,9 @@ begin
     internal_cmd = require? HOMEBREW_LIBRARY_PATH/"cmd"/cmd
 
     unless internal_cmd
-      internal_cmd = require? HOMEBREW_LIBRARY_PATH/"dev-cmd"/cmd
-      if internal_cmd && !ARGV.homebrew_developer?
+      internal_dev_cmd = require? HOMEBREW_LIBRARY_PATH/"dev-cmd"/cmd
+      internal_cmd = internal_dev_cmd
+      if internal_dev_cmd && !ARGV.homebrew_developer?
         system "git", "config", "--file=#{HOMEBREW_REPOSITORY}/.git/config",
                                 "--replace-all", "homebrew.devcmdrun", "true"
         ENV["HOMEBREW_DEV_CMD_RUN"] = "1"
@@ -90,8 +90,7 @@ begin
 
   unless internal_cmd
     # Add contributed commands to PATH before checking.
-    tap_cmds = Pathname.glob(Tap::TAP_DIRECTORY/"*/*/cmd")
-    homebrew_path.append(tap_cmds)
+    homebrew_path.append(Tap.cmd_directories)
 
     # External commands expect a normal PATH
     ENV["PATH"] = homebrew_path
@@ -100,14 +99,13 @@ begin
   if internal_cmd
     Homebrew.send cmd.to_s.tr("-", "_").downcase
   elsif which "brew-#{cmd}"
-    %w[CACHE LIBRARY_PATH].each do |e|
-      ENV["HOMEBREW_#{e}"] = Object.const_get("HOMEBREW_#{e}").to_s
+    %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
-    require "tap"
     possible_tap = OFFICIAL_CMD_TAPS.find { |_, cmds| cmds.include?(cmd) }
     possible_tap = Tap.fetch(possible_tap.first) if possible_tap
 

Library/Homebrew/cmd/command.rb

@@ -8,15 +8,16 @@ module Homebrew
 
   def command
     abort "This command requires a command argument" if ARGV.empty?
-    cmd = ARGV.first
-    cmd = HOMEBREW_INTERNAL_COMMAND_ALIASES.fetch(cmd, cmd)
 
-    if (path = Commands.path(cmd))
-      puts path
-    elsif (path = which("brew-#{cmd}") || which("brew-#{cmd}.rb"))
-      puts path
-    else
-      odie "Unknown command: #{cmd}"
-    end
+    cmd = HOMEBREW_INTERNAL_COMMAND_ALIASES.fetch(ARGV.first, ARGV.first)
+
+    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
 end

Library/Homebrew/cmd/commands.rb

@@ -9,27 +9,30 @@ module Homebrew
 
   def commands
     if ARGV.include? "--quiet"
-      cmds = internal_commands + external_commands
+      cmds = internal_commands
+      cmds += external_commands
       cmds += internal_developer_commands
       cmds += HOMEBREW_INTERNAL_COMMAND_ALIASES.keys if ARGV.include? "--include-aliases"
       puts Formatter.columns(cmds.sort)
-    else
-      # Find commands in Homebrew/cmd
-      puts "Built-in commands"
-      puts Formatter.columns(internal_commands.sort)
-
-      # Find commands in Homebrew/dev-cmd
-      puts
-      puts "Built-in developer commands"
-      puts Formatter.columns(internal_developer_commands.sort)
-
-      # Find commands in the path
-      unless (exts = external_commands).empty?
-        puts
-        puts "External commands"
-        puts Formatter.columns(exts)
-      end
+      return
     end
+
+    # Find commands in Homebrew/cmd
+    puts "Built-in commands"
+    puts Formatter.columns(internal_commands.sort)
+
+    # Find commands in Homebrew/dev-cmd
+    puts
+    puts "Built-in developer commands"
+    puts Formatter.columns(internal_developer_commands.sort)
+
+    exts = external_commands
+    return if exts.empty?
+
+    # Find commands in the PATH
+    puts
+    puts "External commands"
+    puts Formatter.columns(exts)
   end
 
   def internal_commands
@@ -41,11 +44,13 @@ module Homebrew
   end
 
   def external_commands
-    paths.each_with_object([]) do |path, cmds|
+    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]
-        cmds << cmd unless cmd.include?(".")
+        next if cmd.include?(".")
+        cmds << cmd
       end
     end.sort
   end

Library/Homebrew/commands.rb

@@ -1,13 +1,10 @@
 module Commands
   def self.path(cmd)
-    if File.exist?(HOMEBREW_LIBRARY_PATH/"cmd/#{cmd}.sh")
-      HOMEBREW_LIBRARY_PATH/"cmd/#{cmd}.sh"
-    elsif File.exist?(HOMEBREW_LIBRARY_PATH/"dev-cmd/#{cmd}.sh")
-      HOMEBREW_LIBRARY_PATH/"dev-cmd/#{cmd}.sh"
-    elsif File.exist?(HOMEBREW_LIBRARY_PATH/"cmd/#{cmd}.rb")
-      HOMEBREW_LIBRARY_PATH/"cmd/#{cmd}.rb"
-    elsif File.exist?(HOMEBREW_LIBRARY_PATH/"dev-cmd/#{cmd}.rb")
-      HOMEBREW_LIBRARY_PATH/"dev-cmd/#{cmd}.rb"
-    end
+    [
+      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",
+    ].find(&:exist?)
   end
 end

Library/Homebrew/tap.rb

@@ -503,6 +503,11 @@ class Tap
     map(&:name).sort
   end
 
+  # an array of all tap cmd directory {Pathname}s
+  def self.cmd_directories
+    Pathname.glob TAP_DIRECTORY/"*/*/cmd"
+  end
+
   # @private
   def formula_file_to_name(file)
     "#{name}/#{file.basename(".rb")}"