Merge remote-tracking branch 'origin/fix_inspect_output'

Conflicts:
	lib/pry/pager.rb

lib/pry.rb

@@ -20,41 +20,10 @@ class Pry
 
   # The default print
   DEFAULT_PRINT = proc do |output, value|
-    output_with_default_format(output, value, :hashrocket => true)
+    Pry::Pager.with_pager(output) do |pager|
+      pager.print "=> "
+      Pry::ColorPrinter.pp(value, pager, Pry::Terminal.width! - 1)
     end
-
-  def self.output_with_default_format(output, value, options = {})
-    stringified = begin
-                    value.pretty_inspect
-                  rescue RescuableException
-                    nil
-                  end
-
-    unless String === stringified
-      # Read the class name off of the singleton class to provide a default
-      # inspect.
-      eig = class << value; self; end
-      klass = Pry::Method.safe_send(eig, :ancestors).first
-      id = value.__id__.to_s(16) rescue 0
-      stringified = "#<#{klass}:0x#{id}>"
-    end
-
-    nonce = SecureRandom.hex(4)
-
-    stringified.gsub!(/#</, "%<#{nonce}")
-    # Don't recolorize output with color (for cucumber, looksee, etc.) [Issue #751]
-    colorized = if stringified =~ /\e\[/
-                  stringified
-                else
-                  Helpers::BaseHelpers.colorize_code(stringified)
-                end
-
-    # avoid colour-leak from CodeRay and any of the users' previous output
-    colorized = colorized.sub(/(\n*)\z/, "\e[0m\\1") if Pry.color
-
-    result = colorized.gsub(/%<(.*?)#{nonce}/, '#<\1')
-    result = "=> #{result}" if options[:hashrocket]
-    Helpers::BaseHelpers.stagger_output(result, output)
   end
 
   # may be convenient when working with enormous objects and
@@ -225,6 +194,7 @@ require 'pry/core_extensions'
 require 'pry/pry_class'
 require 'pry/pry_instance'
 require 'pry/cli'
+require 'pry/color_printer'
 require 'pry/pager'
 require 'pry/terminal'
 require 'pry/editor'

lib/pry/color_printer.rb

@@ -0,0 +1,54 @@
+# PP subclass for streaming inspect output in color.
+class Pry
+  class ColorPrinter < ::PP
+    OBJ_COLOR = begin
+      code = CodeRay::Encoders::Terminal::TOKEN_COLORS[:keyword]
+      if code.start_with? "\e"
+        code
+      else
+        "\e[0m\e[0;#{code}m"
+      end
+    end
+
+    def self.pp(obj, out = $>, width = 79)
+      q = ColorPrinter.new(out, width)
+      q.guard_inspect_key { q.pp obj }
+      q.flush
+      out << "\n"
+    end
+
+    def text(str, width = str.length)
+      super *if !Pry.color
+        [str, width]
+      # Don't recolorize output with color [Issue #751]
+      elsif str.include?("\e[")
+        ["#{str}\e[0m", width]
+      elsif str.start_with?('#<') || str == '=' || str == '>'
+        [highlight_object_literal(str), width]
+      else
+        [CodeRay.scan(str, :ruby).term, width]
+      end
+    end
+
+    def pp(obj)
+      super
+    rescue => e
+      raise if e.is_a? Pry::Pager::StopPaging
+
+      # Read the class name off of the singleton class to provide a default
+      # inspect.
+      eig    = class << obj; self; end
+      klass  = Pry::Method.safe_send(eig, :ancestors).first
+      obj_id = obj.__id__.to_s(16) rescue 0
+      str    = "#<#{klass}:0x#{obj_id}>"
+
+      text(Pry.color ? highlight_object_literal(str) : str)
+    end
+
+    private
+
+    def highlight_object_literal(object_literal)
+      "#{OBJ_COLOR}#{object_literal}\e[0m"
+    end
+  end
+end

lib/pry/commands/ls.rb

@@ -327,9 +327,7 @@ class Pry
     end
 
     def format_value(value)
-      accumulator = StringIO.new
-      Pry.output_with_default_format(accumulator, value, :hashrocket => false)
-      accumulator.string
+      Pry::ColorPrinter.pp(value, "")
     end
 
     # Add a new section to the output. Outputs nothing if the section would be empty.

lib/pry/helpers/base_helpers.rb

@@ -98,9 +98,8 @@ class Pry
         mri? && RUBY_VERSION =~ /1.9/
       end
 
-
-      # Try to use `less` for paging, if it fails then use
-      # simple_pager. Also do not page if Pry.pager is falsey
+      # Send the given text through the best available pager (if Pry.pager is
+      # enabled). Infers where to send the output if used as a mixin.
       def stagger_output(text, out = nil)
         out ||= case
                 when respond_to?(:output)
@@ -114,14 +113,7 @@ class Pry
                   $stdout
                 end
 
-        if text.lines.count < Pry::Pager.page_size || !Pry.pager
-          out.puts text
-        else
-          Pry::Pager.page(text)
-        end
-      rescue Errno::ENOENT
-        Pry::Pager.page(text, :simple)
-      rescue Errno::EPIPE
+        Pry::Pager.page(text, out)
       end
 
       # @param [String] arg_string The object path expressed as a string.

lib/pry/pager.rb

@@ -1,64 +1,117 @@
 require 'pry/terminal'
-class Pry::Pager
-  # @param [String] text
-  #   A piece of text to run through a pager.
-  # @param [Symbol?] pager
-  #   `:simple` -- Use the pure ruby pager.
-  #   `:system` -- Use the system pager (less) or the environment variable
-  #                $PAGER if set.
-  #   `nil`     -- Infer what pager to use from the environment.  What this
-  #                really means is that JRuby and systems that do not have
-  #                access to 'less' will run through the pure ruby pager.
-  def self.page(text, pager = nil)
-    case pager
-    when nil
-      no_pager = !SystemPager.available?
-      if no_pager || Pry::Helpers::BaseHelpers.jruby?
-        SimplePager.new(text).page
+
+# A pager is an `IO`-like object that accepts text and either prints it
+# immediately, prints it one page at a time, or streams it to an external
+# program to print one page at a time.
+module Pry::Pager
+  class StopPaging < StandardError
+  end
+
+  # Send the given text through the best available pager (if `Pry.pager` is
+  # enabled).
+  # @param [String] text A piece of text to run through a pager.
+  # @param [IO] output (`$stdout`) An object to send output to.
+  def self.page(text, output = $stdout)
+    with_pager(output) do |pager|
+      pager << text
+    end
+  end
+
+  # Yields a pager object (`NullPager`, `SimplePager`, or `SystemPager`).  All
+  # pagers accept output with `#puts`, `#print`, `#write`, and `#<<`.
+  # @param [IO] output (`$stdout`) An object to send output to.
+  def self.with_pager(output = $stdout)
+    pager = best_available(output)
+    yield pager
+  rescue StopPaging
+  ensure
+    pager.close if pager
+  end
+
+  # Return an instance of the "best" available pager class -- `SystemPager` if
+  # possible, `SimplePager` if `SystemPager` isn't available, and `NullPager`
+  # if the user has disabled paging. All pagers accept output with `#puts`,
+  # `#print`, `#write`, and `#<<`. You must call `#close` when you're done
+  # writing output to a pager, and you must rescue `Pry::Pager::StopPaging`.
+  # These requirements can be avoided by using `.with_pager` instead.
+  # @param [#<<] output ($stdout) An object to send output to.
+  def self.best_available(output)
+    if !Pry.pager
+      NullPager.new(output)
+    elsif !SystemPager.available? || Pry::Helpers::BaseHelpers.jruby?
+      SimplePager.new(output)
     else
-        SystemPager.new(text).page
-      end
-    when :simple
-      SimplePager.new(text).page
-    when :system
-      SystemPager.new(text).page
-    else
-      raise "'#{pager}' is not a recognized pager."
+      SystemPager.new(output)
     end
   end
 
-  def self.page_size
-    @page_size ||= Pry::Terminal.height!
+  # `NullPager` is a "pager" that actually just prints all output as it comes
+  # in. Used when `Pry.pager` is false.
+  class NullPager
+    def initialize(out)
+      @out = out
     end
 
-  def initialize(text)
-    @text = text
+    def puts(str)
+      print "#{str.chomp}\n"
     end
 
-  class SimplePager < Pry::Pager
-    def page
-      # The pager size minus the number of lines used by the simple pager info bar.
-      page_size = Pry::Pager.page_size - 3
-      text_array = @text.lines.to_a
+    def print(str)
+      write str
+    end
+    alias << print
 
-      text_array.each_slice(page_size) do |chunk|
-        puts chunk.join
-        break if chunk.size < page_size
-        if text_array.size > page_size
-          puts "\n<page break> --- Press enter to continue ( q<enter> to break ) --- <page break>"
-          break if Readline.readline.chomp == "q"
+    def write(str)
+      @out.write str
+    end
+
+    def close
+    end
+
+    private
+
+    def height
+      @height ||= Pry::Terminal.height!
+    end
+
+    def width
+      @width ||= Pry::Terminal.width!
+    end
+  end
+
+  # `SimplePager` is a straightforward pure-Ruby pager. We use it on JRuby and
+  # when we can't find a usable external pager.
+  class SimplePager < NullPager
+    def initialize(*)
+      super
+      @tracker = PageTracker.new(height - 3, width)
+    end
+
+    def write(str)
+      str.lines.each do |line|
+        @out.print line
+        @tracker.record line
+
+        if @tracker.page?
+          @out.puts "\n<page break> --- Press enter to continue " \
+                    "( q<enter> to break ) --- <page break>"
+          raise StopPaging if Readline.readline.chomp == "q"
+          @tracker.reset
         end
       end
     end
   end
 
-  class SystemPager < Pry::Pager
+  # `SystemPager` buffers output until we're pretty sure it's at least a page
+  # long, then invokes an external pager and starts streaming output to it. If
+  # `#close` is called before then, it just prints out the buffered content.
+  class SystemPager < NullPager
     def self.default_pager
       pager = ENV["PAGER"] || ""
 
       # Default to less, and make sure less is being passed the correct options
-      if pager.strip.empty? or pager =~ /^less\s*/
-        pager = "less -R -S -F -X"
+      if pager.strip.empty? or pager =~ /^less\b/
+        pager = "less -R -F -X"
       end
 
       pager
@@ -79,13 +132,84 @@ class Pry::Pager
 
     def initialize(*)
       super
-      @pager = SystemPager.default_pager
+      @tracker = PageTracker.new(height, width)
+      @buffer  = ""
     end
 
-    def page
-      IO.popen(@pager, 'w') do |io|
-        io.write @text
+    def write(str)
+      if invoked_pager?
+        pager.write str
+      else
+        @tracker.record str
+        @buffer << str
+
+        if @tracker.page?
+          pager.write @buffer
+        end
+      end
+    rescue Errno::EPIPE
+      raise StopPaging
+    end
+
+    def close
+      if invoked_pager?
+        pager.close
+      else
+        @out.puts @buffer
+      end
+    end
+
+    private
+
+    def invoked_pager?
+      @pager
+    end
+
+    def pager
+      @pager ||= IO.popen(self.class.default_pager, 'w')
+    end
+  end
+
+  # `PageTracker` tracks output to determine whether it's likely to take up a
+  # whole page. This doesn't need to be super precise, but we can use it for
+  # `SimplePager` and to avoid invoking the system pager unnecessarily.
+  #
+  # One simplifying assumption is that we don't need `#page?` to return `true`
+  # on the basis of an incomplete line. Long lines should be counted as
+  # multiple lines, but we don't have to transition from `false` to `true`
+  # until we see a newline.
+  class PageTracker
+    def initialize(rows, cols)
+      @rows, @cols = rows, cols
+      reset
+    end
+
+    def record(str)
+      str.lines.each do |line|
+        if line.end_with? "\n"
+          @row += ((@col + line_length(line) - 1) / @cols) + 1
+          @col  = 0
+        else
+          @col += line_length(line)
         end
       end
     end
+
+    def page?
+      @row >= @rows
+    end
+
+    def reset
+      @row = 0
+      @col = 0
+    end
+
+    private
+
+    # Approximation of the printable length of a given line, without the
+    # newline and without ANSI color codes.
+    def line_length(line)
+      line.chomp.gsub(/\e\[[\d;]*m/, '').length
+    end
+  end
 end

spec/pager_spec.rb

@@ -0,0 +1,67 @@
+require "helper"
+
+describe "Pry::Pager" do
+  describe "PageTracker" do
+    before do
+      @pt = Pry::Pager::PageTracker.new(10, 10)
+    end
+
+    def record_short_line
+      @pt.record "012345678\n"
+    end
+
+    def record_long_line
+      @pt.record "0123456789012\n"
+    end
+
+    def record_multiline
+      @pt.record "0123456789012\n01\n"
+    end
+
+    def record_string_without_newline
+      @pt.record "0123456789"
+    end
+
+    def record_string_with_color_codes
+      @pt.record(CodeRay.scan("0123456789", :ruby).term + "\n")
+    end
+
+    it "records short lines that don't add up to a page" do
+      9.times { record_short_line }
+      @pt.page?.should.be.false
+    end
+
+    it "records short lines that do add up to a page" do
+      10.times { record_short_line }
+      @pt.page?.should.be.true
+    end
+
+    it "treats a long line as taking up more than one row" do
+      4.times { record_long_line }
+      @pt.page?.should.be.false
+      record_long_line
+      @pt.page?.should.be.true
+    end
+
+    it "records a string with an embedded newline" do
+      3.times { record_multiline }
+      @pt.page?.should.be.false
+      record_short_line
+      @pt.page?.should.be.true
+    end
+
+    it "doesn't count a line until it ends" do
+      12.times { record_string_without_newline }
+      @pt.page?.should.be.false
+      record_short_line
+      @pt.page?.should.be.true
+    end
+
+    it "doesn't count ansi color codes towards length" do
+      9.times { record_string_with_color_codes }
+      @pt.page?.should.be.false
+      record_string_with_color_codes
+      @pt.page?.should.be.true
+    end
+  end
+end

spec/pry_output_spec.rb

@@ -60,8 +60,9 @@ describe Pry do
 
     it "should colorize strings as though they were ruby" do
       accumulator = StringIO.new
+      colorized   = CodeRay.scan("[1]", :ruby).term
       Pry.config.print.call(accumulator, [1])
-      accumulator.string.should == "=> [\e[1;34m1\e[0m]\e[0m\n"
+      accumulator.string.should == "=> #{colorized}\n"
     end
 
     it "should not colorize strings that already include color" do