ruby-changes:55927
From: Takashi <ko1@a...>
Date: Fri, 31 May 2019 06:58:20 +0900 (JST)
Subject: [ruby-changes:55927] Takashi Kokubun: cb40a21da0 (trunk): Warn compile_error only when input is finished
https://git.ruby-lang.org/ruby.git/commit/?id=cb40a21da0 From cb40a21da0687b5dd3cd251c9e66bb0edf67f2b9 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun <takashikkbn@g...> Date: Fri, 31 May 2019 06:03:18 +0900 Subject: Warn compile_error only when input is finished Let's say we are in progress to write `"foo"`: ``` irb> "fo ``` at this moment, nothing is wrong. It would be just a normal way to write `"foo"`. Prior to this commit, the `fo` part was warned because of 5b64d7ac6e7cbf759b859428f125539e58bac0bd. But I think warning such a normal input is not valuable for users. However, we'd like to warn `:@1` or `@@1` which is also a syntax error. Then this commit switches the syntax highlight based on whether the input text is finished or not. When it's not finished yet, it does not warn compile_error. diff --git a/lib/irb/color.rb b/lib/irb/color.rb index 41b559b..4c83a9b 100644 --- a/lib/irb/color.rb +++ b/lib/irb/color.rb @@ -98,14 +98,17 @@ module IRB # :nodoc: https://github.com/ruby/ruby/blob/trunk/lib/irb/color.rb#L98 "#{seq}#{text}#{clear}" end - def colorize_code(code) + # If `complete` is false (code is incomplete), this does not warm compile_error. + # This option is needed to avoid warning a user when the compile_error is happening + # because the input is not wrong but just incomplete. + def colorize_code(code, complete: true) return code unless colorable? symbol_state = SymbolState.new colored = +'' length = 0 - scan(code) do |token, str, expr| + scan(code, detect_compile_error: complete) do |token, str, expr| in_symbol = symbol_state.scan_token(token) str.each_line do |line| line = Reline::Unicode.escape_for_print(line) @@ -127,23 +130,29 @@ module IRB # :nodoc: https://github.com/ruby/ruby/blob/trunk/lib/irb/color.rb#L130 private - def scan(code) - pos = [1, 0] - - Ripper::Lexer.new(code).scan.each do |elem| - str = elem.tok - next if ([elem.pos[0], elem.pos[1] + str.bytesize] <=> pos) <= 0 - - str.each_line do |line| - if line.end_with?("\n") - pos[0] += 1 - pos[1] = 0 - else - pos[1] += line.bytesize + def scan(code, detect_compile_error:) + if detect_compile_error + pos = [1, 0] + + Ripper::Lexer.new(code).scan.each do |elem| + str = elem.tok + next if ([elem.pos[0], elem.pos[1] + str.bytesize] <=> pos) <= 0 + + str.each_line do |line| + if line.end_with?("\n") + pos[0] += 1 + pos[1] = 0 + else + pos[1] += line.bytesize + end end - end - yield(elem.event, str, elem.state) + yield(elem.event, str, elem.state) + end + else + ParseErrorLexer.new(code).parse.sort_by(&:pos).each do |elem| + yield(elem.event, elem.tok, elem.state) + end end end @@ -162,6 +171,15 @@ module IRB # :nodoc: https://github.com/ruby/ruby/blob/trunk/lib/irb/color.rb#L171 end end + class ParseErrorLexer < Ripper::Lexer + if method_defined?(:token) + def on_parse_error(mesg) + @buf.push Elem.new([lineno(), column()], __callee__, token(), state()) + end + end + end + private_constant :ParseErrorLexer + # A class to manage a state to know whether the current token is for Symbol or not. class SymbolState def initialize diff --git a/lib/irb/input-method.rb b/lib/irb/input-method.rb index 9d3580a..aa62a62 100644 --- a/lib/irb/input-method.rb +++ b/lib/irb/input-method.rb @@ -224,9 +224,9 @@ module IRB https://github.com/ruby/ruby/blob/trunk/lib/irb/input-method.rb#L224 Reline.completion_proc = IRB::InputCompletor::CompletionProc Reline.output_modifier_proc = if IRB.conf[:USE_COLORIZE] - proc do |output| + proc do |output, complete:| next unless IRB::Color.colorable? - IRB::Color.colorize_code(output) + IRB::Color.colorize_code(output, complete: complete) end else proc do |output| diff --git a/lib/reline/line_editor.rb b/lib/reline/line_editor.rb index a8a3f67..acdc9a2 100644 --- a/lib/reline/line_editor.rb +++ b/lib/reline/line_editor.rb @@ -436,6 +436,8 @@ class Reline::LineEditor https://github.com/ruby/ruby/blob/trunk/lib/reline/line_editor.rb#L436 line = modify_lines(whole_lines)[@line_index] if @is_multiline if finished? + # Always rerender on finish because output_modifier_proc may return a different output. + render_partial(prompt, prompt_width, line) scroll_down(1) Reline::IOGate.move_cursor_column(0) Reline::IOGate.erase_after_cursor @@ -498,7 +500,7 @@ class Reline::LineEditor https://github.com/ruby/ruby/blob/trunk/lib/reline/line_editor.rb#L500 private def modify_lines(before) return before if before.nil? || before.empty? - if after = @output_modifier_proc&.call("#{before.join("\n")}\n") + if after = @output_modifier_proc&.call("#{before.join("\n")}\n", complete: finished?) after.lines(chomp: true) else before diff --git a/test/irb/test_color.rb b/test/irb/test_color.rb index 778874a..ebae790 100644 --- a/test/irb/test_color.rb +++ b/test/irb/test_color.rb @@ -23,6 +23,7 @@ module TestIRB https://github.com/ruby/ruby/blob/trunk/test/irb/test_color.rb#L23 skip "this Ripper version is not supported" end + # Common behaviors. Warn parser error, but do not warn compile error. { "1" => "#{BLUE}#{BOLD}1#{CLEAR}", "2.3" => "#{MAGENTA}#{BOLD}2.3#{CLEAR}", @@ -40,7 +41,6 @@ module TestIRB https://github.com/ruby/ruby/blob/trunk/test/irb/test_color.rb#L41 "'a\nb'" => "#{RED}'#{CLEAR}#{RED}a#{CLEAR}\n#{RED}b#{CLEAR}#{RED}'#{CLEAR}", "4.5.6" => "#{MAGENTA}#{BOLD}4.5#{CLEAR}#{RED}#{REVERSE}.6#{CLEAR}", "[1]]]" => "[1]]]", - "\e[0m\n" => "#{RED}#{REVERSE}^[#{CLEAR}[#{BLUE}#{BOLD}0#{CLEAR}m\n", "%w[a b]" => "#{RED}%w[#{CLEAR}#{RED}a#{CLEAR} #{RED}b#{CLEAR}#{RED}]#{CLEAR}", "%i[c d]" => "#{RED}%i[#{CLEAR}#{RED}c#{CLEAR} #{RED}d#{CLEAR}#{RED}]#{CLEAR}", "{'a': 1}" => "{#{RED}'#{CLEAR}#{RED}a#{CLEAR}#{RED}':#{CLEAR} #{BLUE}#{BOLD}1#{CLEAR}}", @@ -66,10 +66,38 @@ module TestIRB https://github.com/ruby/ruby/blob/trunk/test/irb/test_color.rb#L66 "\t" => "\t", # not ^I "foo(*%W(bar))" => "foo(*#{RED}%W(#{CLEAR}#{RED}bar#{CLEAR}#{RED})#{CLEAR})", "$stdout" => "#{GREEN}#{BOLD}$stdout#{CLEAR}", + }.each do |code, result| + actual = with_term { IRB::Color.colorize_code(code, complete: true) } + assert_equal(result, actual, "Case: colorize_code(#{code.dump}, complete: true)\nResult: #{humanized_literal(actual)}") + + actual = with_term { IRB::Color.colorize_code(code, complete: false) } + assert_equal(result, actual, "Case: colorize_code(#{code.dump}, complete: false)\nResult: #{humanized_literal(actual)}") + end + end + + def test_colorize_code_complete_true + # `complete: true` behaviors. Warn compile_error. + { + "\e[0m\n" => "#{RED}#{REVERSE}^[#{CLEAR}[#{BLUE}#{BOLD}0#{CLEAR}m\n", "'foo' + 'bar" => "#{RED}'#{CLEAR}#{RED}foo#{CLEAR}#{RED}'#{CLEAR} + #{RED}'#{CLEAR}#{RED}#{REVERSE}bar#{CLEAR}", + ":@1" => "#{YELLOW}:#{CLEAR}#{RED}#{REVERSE}@1#{CLEAR}", + "@@1" => "#{RED}#{REVERSE}@@1#{CLEAR}", + }.each do |code, result| + actual = with_term { IRB::Color.colorize_code(code, complete: true) } + assert_equal(result, actual, "Case: colorize_code(#{code.dump}, complete: true)\nResult: #{humanized_literal(actual)}") + end + end + + def test_colorize_code_complete_false + # `complete: false` behaviors. Do not warn compile_error. + { + "\e[0m\n" => "#{CLEAR}\n", + "'foo' + 'bar" => "#{RED}'#{CLEAR}#{RED}foo#{CLEAR}#{RED}'#{CLEAR} + #{RED}'#{CLEAR}#{RED}bar#{CLEAR}", + ":@1" => "#{YELLOW}:#{CLEAR}#{YELLOW}@1#{CLEAR}", + "@@1" => "@@1", }.each do |code, result| - actual = with_term { IRB::Color.colorize_code(code) } - assert_equal(result, actual, "Case: colorize_code(#{code.dump})\nResult: #{humanized_literal(actual)}") + actual = with_term { IRB::Color.colorize_code(code, complete: false) } + assert_equal(result, actual, "Case: colorize_code(#{code.dump}, complete: false)\nResult: #{humanized_literal(actual)}") end end -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/