Merge pull request #1208 from reitermarkus/puts-columns

Refactor `puts_columns` to allow `$stderr.puts_columns`.

Library/Homebrew/cask/lib/hbc/cli/install.rb

@@ -42,13 +42,14 @@ module Hbc
 
       def self.warn_unavailable_with_suggestion(cask_token, e)
         exact_match, partial_matches = Search.search(cask_token)
-        errmsg = e.message
+        error_message = e.message
         if exact_match
-          errmsg.concat(". Did you mean:\n#{exact_match}")
+          error_message.concat(". Did you mean:\n#{exact_match}")
         elsif !partial_matches.empty?
-          errmsg.concat(". Did you mean one of:\n#{puts_columns(partial_matches.take(20))}\n")
+          error_message.concat(". Did you mean one of:\n")
+                       .concat(Formatter.columns(partial_matches.take(20)))
         end
-        onoe errmsg
+        onoe error_message
       end
 
       def self.help

Library/Homebrew/cask/lib/hbc/cli/list.rb

@@ -68,7 +68,7 @@ module Hbc
         elsif @options[:versions]
           puts installed_casks.map(&method(:format_versioned))
         else
-          puts_columns installed_casks.map(&:to_s)
+          puts Formatter.columns(installed_casks.map(&:to_s))
         end
 
         installed_casks.empty? ? nil : true

Library/Homebrew/cask/lib/hbc/cli/search.rb

@@ -47,7 +47,7 @@ module Hbc
           else
             ohai "Partial matches"
           end
-          puts_columns partial_matches
+          puts Formatter.columns(partial_matches)
         end
       end
 

Library/Homebrew/cask/spec/formatter_spec.rb

@@ -0,0 +1,56 @@
+require "spec_helper"
+require "utils/formatter"
+require "utils/tty"
+
+describe Formatter do
+  describe "::columns" do
+    let(:input) {
+      [
+        'aa',
+        'bbb',
+        'ccc',
+        'dd'
+      ]
+    }
+    subject { described_class.columns(input) }
+
+    it "doesn't output columns if $stdout is not a TTY." do
+      allow_any_instance_of(IO).to receive(:tty?).and_return(false)
+      allow(Tty).to receive(:width).and_return(10)
+
+      expect(subject).to eq(
+        "aa\n" \
+        "bbb\n" \
+        "ccc\n" \
+        "dd\n"
+      )
+    end
+
+    describe "$stdout is a TTY" do
+      it "outputs columns" do
+        allow_any_instance_of(IO).to receive(:tty?).and_return(true)
+        allow(Tty).to receive(:width).and_return(10)
+
+        expect(subject).to eq(
+          "aa    ccc\n" \
+          "bbb   dd\n"
+        )
+      end
+
+      it "outputs only one line if everything fits" do
+        allow_any_instance_of(IO).to receive(:tty?).and_return(true)
+        allow(Tty).to receive(:width).and_return(20)
+
+        expect(subject).to eq(
+          "aa   bbb  ccc  dd\n"
+        )
+      end
+    end
+
+    describe "with empty input" do
+      let(:input) { [] }
+
+      it { is_expected.to eq("\n") }
+    end
+  end
+end

Library/Homebrew/cmd/commands.rb

@@ -12,22 +12,22 @@ module Homebrew
       cmds = internal_commands + external_commands
       cmds += internal_developer_commands
       cmds += HOMEBREW_INTERNAL_COMMAND_ALIASES.keys if ARGV.include? "--include-aliases"
-      puts_columns cmds.sort
+      puts Formatter.columns(cmds.sort)
     else
       # Find commands in Homebrew/cmd
       puts "Built-in commands"
-      puts_columns internal_commands
+      puts Formatter.columns(internal_commands)
 
       # Find commands in Homebrew/dev-cmd
       puts
       puts "Built-in developer commands"
-      puts_columns internal_developer_commands
+      puts Formatter.columns(internal_developer_commands)
 
       # Find commands in the path
       unless (exts = external_commands).empty?
         puts
         puts "External commands"
-        puts_columns exts
+        puts Formatter.columns(exts)
       end
     end
   end

Library/Homebrew/cmd/install.rb

@@ -179,11 +179,11 @@ module Homebrew
           ofail "No similarly named formulae found."
         when 1
           puts "This similarly named formula was found:"
-          puts_columns(formulae_search_results)
+          puts formulae_search_results
           puts "To install it, run:\n  brew install #{formulae_search_results.first}"
         else
           puts "These similarly named formulae were found:"
-          puts_columns(formulae_search_results)
+          puts Formatter.columns(formulae_search_results)
           puts "To install one of them, run (for example):\n  brew install #{formulae_search_results.first}"
         end
 
@@ -194,11 +194,11 @@ module Homebrew
           ofail "No formulae found in taps."
         when 1
           puts "This formula was found in a tap:"
-          puts_columns(taps_search_results)
+          puts taps_search_results
           puts "To install it, run:\n  brew install #{taps_search_results.first}"
         else
           puts "These formulae were found in taps:"
-          puts_columns(taps_search_results)
+          puts Formatter.columns(taps_search_results)
           puts "To install one of them, run (for example):\n  brew install #{taps_search_results.first}"
         end
       end

Library/Homebrew/cmd/list.rb

@@ -48,7 +48,8 @@ module Homebrew
             a <=> b
           end
         end
-        puts_columns full_names
+        return if full_names.empty?
+        puts Formatter.columns(full_names)
       else
         ENV["CLICOLOR"] = nil
         exec "ls", *ARGV.options_only << HOMEBREW_CELLAR

Library/Homebrew/cmd/search.rb

@@ -27,7 +27,7 @@ module Homebrew
 
   def search
     if ARGV.empty?
-      puts_columns Formula.full_names
+      puts Formatter.columns(Formula.full_names)
     elsif ARGV.include? "--macports"
       exec_browser "https://www.macports.org/ports.php?by=name&substr=#{ARGV.next}"
     elsif ARGV.include? "--fink"
@@ -54,14 +54,15 @@ module Homebrew
         result = search_tap(user, repo, name)
       end
 
-      puts_columns Array(result)
+      results = Array(result)
+      puts Formatter.columns(results) unless results.empty?
     else
       query = ARGV.first
       regex = query_regexp(query)
       local_results = search_formulae(regex)
-      puts_columns(local_results)
+      puts Formatter.columns(local_results) unless local_results.empty?
       tap_results = search_taps(regex)
-      puts_columns(tap_results)
+      puts Formatter.columns(tap_results) unless tap_results.empty?
 
       if $stdout.tty?
         count = local_results.length + tap_results.length

Library/Homebrew/cmd/update-report.rb

@@ -553,7 +553,7 @@ class ReporterHub
     return if formulae.empty?
     # Dump formula list.
     ohai title
-    puts_columns(formulae)
+    puts Formatter.columns(formulae)
   end
 
   def installed?(formula)

Library/Homebrew/cmd/uses.rb

@@ -93,6 +93,7 @@ module Homebrew
       end
     end
 
-    puts_columns uses.map(&:full_name)
+    return if uses.empty?
+    puts Formatter.columns(uses.map(&:full_name))
   end
 end

Library/Homebrew/compat/utils.rb

@@ -16,3 +16,8 @@ module Tty
     reset.bold
   end
 end
+
+def puts_columns(items)
+  odeprecated "puts_columns", "puts Formatter.columns"
+  puts Formatter.columns(items)
+end

Library/Homebrew/test/test_utils.rb

@@ -110,8 +110,12 @@ class UtilTests < Homebrew::TestCase
   end
 
   def test_put_columns_empty
-    # Issue #217 put columns with no results fails.
-    assert_silent { puts_columns [] }
+    out, err = capture_io do
+      puts Formatter.columns([])
+    end
+
+    assert_equal out, "\n"
+    assert_equal err, ""
   end
 
   def test_which

Library/Homebrew/utils.rb

@@ -1,16 +1,16 @@
 require "pathname"
 require "emoji"
 require "exceptions"
-require "utils/formatter"
-require "utils/hash"
-require "utils/json"
-require "utils/inreplace"
-require "utils/popen"
-require "utils/fork"
-require "utils/git"
 require "utils/analytics"
-require "utils/github"
 require "utils/curl"
+require "utils/fork"
+require "utils/formatter"
+require "utils/git"
+require "utils/github"
+require "utils/hash"
+require "utils/inreplace"
+require "utils/json"
+require "utils/popen"
 require "utils/tty"
 
 def ohai(title, *sput)
@@ -286,43 +286,6 @@ def quiet_system(cmd, *args)
   end
 end
 
-def puts_columns(items)
-  return if items.empty?
-
-  unless $stdout.tty?
-    puts items
-    return
-  end
-
-  # TTY case: If possible, output using multiple columns.
-  console_width = Tty.width
-  console_width = 80 if console_width <= 0
-  plain_item_lengths = items.map { |s| Tty.strip_ansi(s).length }
-  max_len = plain_item_lengths.max
-  col_gap = 2 # number of spaces between columns
-  gap_str = " " * col_gap
-  cols = (console_width + col_gap) / (max_len + col_gap)
-  cols = 1 if cols < 1
-  rows = (items.size + cols - 1) / cols
-  cols = (items.size + rows - 1) / rows # avoid empty trailing columns
-
-  if cols >= 2
-    col_width = (console_width + col_gap) / cols - col_gap
-    items = items.each_with_index.map do |item, index|
-      item + "".ljust(col_width - plain_item_lengths[index])
-    end
-  end
-
-  if cols == 1
-    puts items
-  else
-    rows.times do |row_index|
-      item_indices_for_row = row_index.step(items.size - 1, rows).to_a
-      puts items.values_at(*item_indices_for_row).join(gap_str)
-    end
-  end
-end
-
 def which(cmd, path = ENV["PATH"])
   path.split(File::PATH_SEPARATOR).each do |p|
     begin

Library/Homebrew/utils/formatter.rb

@@ -49,4 +49,45 @@ module Formatter
     end
   end
   private_class_method :prefix
+
+  def columns(*objects, gap_size: 2)
+    objects = objects.flatten.map(&:to_s)
+
+    fallback = proc do
+      return objects.join("\n").concat("\n")
+    end
+
+    fallback.call if objects.empty?
+    fallback.call if respond_to?(:tty?) ? !tty? : !$stdout.tty?
+
+    console_width = Tty.width
+    object_lengths = objects.map { |obj| Tty.strip_ansi(obj).length }
+    cols = (console_width + gap_size) / (object_lengths.max + gap_size)
+
+    fallback.call if cols < 2
+
+    rows = (objects.count + cols - 1) / cols
+    cols = (objects.count + rows - 1) / rows # avoid empty trailing columns
+
+    col_width = (console_width + gap_size) / cols - gap_size
+
+    gap_string = "".rjust(gap_size)
+
+    output = ""
+
+    rows.times do |row_index|
+      item_indices_for_row = row_index.step(objects.size - 1, rows).to_a
+
+      first_n = item_indices_for_row[0...-1].map { |index|
+        objects[index] + "".rjust(col_width - object_lengths[index])
+      }
+
+      # don't add trailing whitespace to last column
+      last = objects.values_at(item_indices_for_row.last)
+
+      output.concat((first_n + last).join(gap_string)).concat("\n")
+    end
+
+    output
+  end
 end