linkapps: modernize

Simplify code by using `Pathname` methods as much as possible. Also
avoid calling external commands for basic functionality like symlink
creation, refactor code that can be shared with `brew unlinkapps`, and
print a summary line at the end (if symlinks were created).

1 files changed, 24 insertions, 8 deletions

diff --git a/Library/Homebrew/cmd/linkapps.rb b/Library/Homebrew/cmd/linkapps.rb
index dba9beaa5..d97564932 100644
--- a/Library/Homebrew/cmd/linkapps.rb
+++ b/Library/Homebrew/cmd/linkapps.rb
@@ -1,12 +1,11 @@
-# Links any Applications (.app) found in installed prefixes to /Applications
 require "keg"
 require "formula"
 
 module Homebrew
   def linkapps
-    target_dir = ARGV.include?("--local") ? File.expand_path("~/Applications") : "/Applications"
+    target_dir = linkapps_target(:local => ARGV.include?("--local"))
 
-    unless File.exist? target_dir
+    unless target_dir.directory?
       opoo "#{target_dir} does not exist, stopping."
       puts "Run `mkdir #{target_dir}` first."
       exit 1
@@ -16,24 +15,41 @@ module Homebrew
       kegs = Formula.racks.map do |rack|
         keg = rack.subdirs.map { |d| Keg.new(d) }
         next if keg.empty?
-        keg.detect(&:linked?) || keg.max { |a, b| a.version <=> b.version }
+        keg.detect(&:linked?) || keg.max_by(&:version)
       end
     else
       kegs = ARGV.kegs
     end
 
+    link_count = 0
     kegs.each do |keg|
       keg.apps.each do |app|
-        puts "Linking #{app} to #{target_dir}."
-        target = "#{target_dir}/#{app.basename}"
+        puts "Linking: #{app}"
+        target_app = target_dir/app.basename
 
-        if File.exist?(target) && !File.symlink?(target)
-          onoe "#{target} already exists, skipping."
+        if target_app.exist? && !target_app.symlink?
+          onoe "#{target_app} already exists, skipping."
           next
         end
 
+        # We prefer system `ln` over `FileUtils.ln_sf` because the latter seems
+        # to have weird failure conditions (that were observed in the past).
         system "ln", "-sf", app, target_dir
+        link_count += 1
       end
     end
+
+    if link_count.zero?
+      puts "No apps linked to #{target_dir}" if ARGV.verbose?
+    else
+      puts "Linked #{link_count} app#{plural(link_count)} to #{target_dir}"
+    end
+  end
+
+  private
+
+  def linkapps_target(opts = {})
+    local = opts.fetch(:local, false)
+    Pathname.new(local ? "~/Applications" : "/Applications").expand_path
   end
 end