[ruby/irb] Centralize coloring control (https://github.com/ruby/irb/pull/374)

* Use colorable: argument as the only coloring control

* Centalize color controling logic at Color.colorable?

There are 2 requirements for coloring output:

1. It's supported on the platform
2. The user wants it: `IRB.conf[:USE_COLORIZE] == true`

Right now we check 1 and 2 separately whenever we colorize things.
But it's error-prone because while 1 is the default of `colorable`
parameter, 2 always need to manually checked. When 2 is overlooked, it
causes issues like https://github.com/ruby/irb/pull/362

And there's 0 case where we may want to colorize even when the user
disables it. So I think we should merge 2 into `Color.colorable?` so it
can be automatically picked up.

* Add tests for all inspect modes

* Simplify inspectors' coloring logic

* Replace use_colorize? with Color.colorable?

* Remove Context#use_colorize cause it's redundant

https://github.com/ruby/irb/commit/1c53023ac4

8 files changed, 18 insertions, 40 deletions

diff --git a/lib/irb.rb b/lib/irb.rb
index 9d067b7a4d..8e40ffb4d5 100644
--- a/lib/irb.rb
+++ b/lib/irb.rb
@@ -828,11 +828,11 @@ module IRB
           if diff_size.positive? and output_width > winwidth
             lines, _ = Reline::Unicode.split_by_width(first_line, winwidth - diff_size - 3)
             str = "%s..." % lines.first
-            str += "\e[0m" if @context.use_colorize
+            str += "\e[0m" if Color.colorable?
             multiline_p = false
           else
             str = str.gsub(/(\A.*?\n).*/m, "\\1...")
-            str += "\e[0m" if @context.use_colorize
+            str += "\e[0m" if Color.colorable?
           end
         else
           output_width = Reline::Unicode.calculate_width(@context.return_format % str, true)
@@ -840,7 +840,7 @@ module IRB
           if diff_size.positive? and output_width > winwidth
             lines, _ = Reline::Unicode.split_by_width(str, winwidth - diff_size - 3)
             str = "%s..." % lines.first
-            str += "\e[0m" if @context.use_colorize
+            str += "\e[0m" if Color.colorable?
           end
         end
       end
diff --git a/lib/irb/cmd/ls.rb b/lib/irb/cmd/ls.rb
index 6a30a28ea0..f4a7348bd1 100644
--- a/lib/irb/cmd/ls.rb
+++ b/lib/irb/cmd/ls.rb
@@ -10,7 +10,7 @@ module IRB
   module ExtendCommand
     class Ls < Nop
       def execute(*arg, grep: nil)
-        o = Output.new(grep: grep, colorable: colorable)
+        o = Output.new(grep: grep)
 
         obj    = arg.empty? ? irb_context.workspace.main : arg.first
         locals = arg.empty? ? irb_context.workspace.binding.local_variables : []
@@ -45,8 +45,7 @@ module IRB
       class Output
         MARGIN = "  "
 
-        def initialize(grep: nil, colorable: true)
-          @colorable = colorable
+        def initialize(grep: nil)
           @grep = grep
           @line_width = screen_width - MARGIN.length # right padding
         end
@@ -57,7 +56,7 @@ module IRB
           return if strs.empty?
 
           # Attempt a single line
-          print "#{Color.colorize(name, [:BOLD, :BLUE], colorable: @colorable)}: "
+          print "#{Color.colorize(name, [:BOLD, :BLUE])}: "
           if fits_on_line?(strs, cols: strs.size, offset: "#{name}: ".length)
             puts strs.join(MARGIN)
             return
diff --git a/lib/irb/cmd/nop.rb b/lib/irb/cmd/nop.rb
index 17ff2f9b7e..881a736722 100644
--- a/lib/irb/cmd/nop.rb
+++ b/lib/irb/cmd/nop.rb
@@ -29,10 +29,9 @@ module IRB
 
       def initialize(conf)
         @irb_context = conf
-        @colorable = Color.colorable? && conf.use_colorize
       end
 
-      attr_reader :irb_context, :colorable
+      attr_reader :irb_context
 
       def irb
         @irb_context.irb
diff --git a/lib/irb/cmd/show_source.rb b/lib/irb/cmd/show_source.rb
index 7e790c3c93..f8a17822df 100644
--- a/lib/irb/cmd/show_source.rb
+++ b/lib/irb/cmd/show_source.rb
@@ -30,7 +30,7 @@ module IRB
         puts
         puts "#{bold("From")}: #{source.file}:#{source.first_line}"
         puts
-        code = IRB::Color.colorize_code(File.read(source.file), colorable: colorable)
+        code = IRB::Color.colorize_code(File.read(source.file))
         puts code.lines[(source.first_line - 1)...source.last_line].join
         puts
       end
@@ -78,7 +78,7 @@ module IRB
       end
 
       def bold(str)
-        Color.colorize(str, [:BOLD], colorable: colorable)
+        Color.colorize(str, [:BOLD])
       end
 
       Source = Struct.new(
diff --git a/lib/irb/color.rb b/lib/irb/color.rb
index f9204f4c98..c4513f5b0b 100644
--- a/lib/irb/color.rb
+++ b/lib/irb/color.rb
@@ -77,7 +77,7 @@ module IRB # :nodoc:
 
     class << self
       def colorable?
-        $stdout.tty? && (/mswin|mingw/ =~ RUBY_PLATFORM || (ENV.key?('TERM') && ENV['TERM'] != 'dumb'))
+        $stdout.tty? && (/mswin|mingw/ =~ RUBY_PLATFORM || (ENV.key?('TERM') && ENV['TERM'] != 'dumb')) && IRB.conf.fetch(:USE_COLORIZE, true)
       end
 
       def inspect_colorable?(obj, seen: {}.compare_by_identity)
diff --git a/lib/irb/context.rb b/lib/irb/context.rb
index 0a46c1b1d4..e6c993d423 100644
--- a/lib/irb/context.rb
+++ b/lib/irb/context.rb
@@ -53,7 +53,6 @@ module IRB
       else
         @use_multiline = nil
       end
-      @use_colorize = IRB.conf[:USE_COLORIZE]
       @use_autocomplete = IRB.conf[:USE_AUTOCOMPLETE]
       @verbose = IRB.conf[:VERBOSE]
       @io = nil
@@ -186,8 +185,6 @@ module IRB
     attr_reader :use_singleline
     # Whether colorization is enabled or not.
     #
-    # A copy of the default <code>IRB.conf[:USE_COLORIZE]</code>
-    attr_reader :use_colorize
     # A copy of the default <code>IRB.conf[:USE_AUTOCOMPLETE]</code>
     attr_reader :use_autocomplete
     # A copy of the default <code>IRB.conf[:INSPECT_MODE]</code>
@@ -332,8 +329,6 @@ module IRB
     alias use_readline use_singleline
     # backward compatibility
     alias use_readline? use_singleline
-    # Alias for #use_colorize
-    alias use_colorize? use_colorize
     # Alias for #use_autocomplete
     alias use_autocomplete? use_autocomplete
     # Alias for #rc
diff --git a/lib/irb/inspector.rb b/lib/irb/inspector.rb
index 8c37c0f174..d8c0ba90cf 100644
--- a/lib/irb/inspector.rb
+++ b/lib/irb/inspector.rb
@@ -108,18 +108,10 @@ module IRB # :nodoc:
 
   Inspector.def_inspector([false, :to_s, :raw]){|v| v.to_s}
   Inspector.def_inspector([:p, :inspect]){|v|
-    result = v.inspect
-    if IRB.conf[:MAIN_CONTEXT]&.use_colorize? && Color.inspect_colorable?(v)
-      result = Color.colorize_code(result)
-    end
-    result
+    Color.colorize_code(v.inspect, colorable: Color.colorable? && Color.inspect_colorable?(v))
   }
   Inspector.def_inspector([true, :pp, :pretty_inspect], proc{require_relative "color_printer"}){|v|
-    if IRB.conf[:MAIN_CONTEXT]&.use_colorize?
-      IRB::ColorPrinter.pp(v, '').chomp
-    else
-      v.pretty_inspect.chomp
-    end
+    IRB::ColorPrinter.pp(v, '').chomp
   }
   Inspector.def_inspector([:yaml, :YAML], proc{require "yaml"}){|v|
     begin
diff --git a/lib/irb/workspace.rb b/lib/irb/workspace.rb
index 2c4c40f348..e5ef52528a 100644
--- a/lib/irb/workspace.rb
+++ b/lib/irb/workspace.rb
@@ -158,27 +158,20 @@ EOF
         end
       end
 
-      # NOT using #use_colorize? of IRB.conf[:MAIN_CONTEXT] because this method may be called before IRB::Irb#run
-      use_colorize = IRB.conf.fetch(:USE_COLORIZE, true)
-      if use_colorize
-        lines = Color.colorize_code(code).lines
-      else
-        lines = code.lines
-      end
+      lines = Color.colorize_code(code).lines
       pos -= 1
 
       start_pos = [pos - 5, 0].max
       end_pos   = [pos + 5, lines.size - 1].min
 
-      if use_colorize
-        fmt = " %2s #{Color.colorize("%#{end_pos.to_s.length}d", [:BLUE, :BOLD])}: %s"
-      else
-        fmt = " %2s %#{end_pos.to_s.length}d: %s"
-      end
+      line_number_fmt = Color.colorize("%#{end_pos.to_s.length}d", [:BLUE, :BOLD])
+      fmt = " %2s #{line_number_fmt}: %s"
+
       body = (start_pos..end_pos).map do |current_pos|
         sprintf(fmt, pos == current_pos ? '=>' : '', current_pos + 1, lines[current_pos])
       end.join("")
-      "\nFrom: #{file} @ line #{pos + 1} :\n\n#{body}#{Color.clear if use_colorize}\n"
+
+      "\nFrom: #{file} @ line #{pos + 1} :\n\n#{body}#{Color.clear}\n"
     end
 
     def IRB.delete_caller