ruby-changes:72345
From: Stan <ko1@a...>
Date: Tue, 28 Jun 2022 22:30:54 +0900 (JST)
Subject: [ruby-changes:72345] 44c1316293 (master): [ruby/irb] Centralize coloring control (https://github.com/ruby/irb/pull/374)
https://git.ruby-lang.org/ruby.git/commit/?id=44c1316293 From 44c1316293f80abaa0e76b3818322544b9372a97 Mon Sep 17 00:00:00 2001 From: Stan Lo <stan001212@g...> Date: Tue, 28 Jun 2022 14:30:36 +0100 Subject: [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 --- lib/irb.rb | 6 +++--- lib/irb/cmd/ls.rb | 7 +++---- lib/irb/cmd/nop.rb | 3 +-- lib/irb/cmd/show_source.rb | 4 ++-- lib/irb/color.rb | 2 +- lib/irb/context.rb | 5 ----- lib/irb/inspector.rb | 12 ++---------- lib/irb/workspace.rb | 19 ++++++------------- test/irb/test_context.rb | 46 ++++++++++++++++++++++++++++------------------ 9 files changed, 46 insertions(+), 58 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 https://github.com/ruby/ruby/blob/trunk/lib/irb.rb#L828 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 https://github.com/ruby/ruby/blob/trunk/lib/irb.rb#L840 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/cmd/ls.rb#L10 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/cmd/ls.rb#L45 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/cmd/ls.rb#L56 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/cmd/nop.rb#L29 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/cmd/show_source.rb#L30 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/cmd/show_source.rb#L78 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: https://github.com/ruby/ruby/blob/trunk/lib/irb/color.rb#L77 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/context.rb#L53 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/context.rb#L185 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/context.rb#L329 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: https://github.com/ruby/ruby/blob/trunk/lib/irb/inspector.rb#L108 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 https://github.com/ruby/ruby/blob/trunk/lib/irb/workspace.rb#L158 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 diff --git a/test/irb/test_context.rb b/test/irb/test_context.rb index 20f80e4af0..64296d7352 100644 --- a/test/irb/test_context.rb +++ b/test/irb/test_context.rb @@ -1 (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/