From 14a51e3b2eb2e117e25cb40d49cdb1d4003d66b6 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Fri, 11 Oct 2024 00:52:54 +0200 Subject: [PATCH 1/3] Handle very long lines --- lib/error_highlight/formatter.rb | 43 ++++++++++++++++++++++++++++++++ test/test_error_highlight.rb | 26 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/lib/error_highlight/formatter.rb b/lib/error_highlight/formatter.rb index 20ca78d..b4fce93 100644 --- a/lib/error_highlight/formatter.rb +++ b/lib/error_highlight/formatter.rb @@ -3,6 +3,8 @@ class DefaultFormatter def self.message_for(spot) # currently only a one-line code snippet is supported if spot[:first_lineno] == spot[:last_lineno] + spot = truncate(spot) + indent = spot[:snippet][0...spot[:first_column]].gsub(/[^\t]/, " ") marker = indent + "^" * (spot[:last_column] - spot[:first_column]) @@ -11,6 +13,47 @@ def self.message_for(spot) "" end end + + def self.viewport_size + Ractor.current[:__error_highlight_viewport_size__] || terminal_columns + end + + def self.viewport_size=(viewport_size) + Ractor.current[:__error_highlight_viewport_size__] = viewport_size + end + + private + + def self.truncate(spot) + ellipsis = '...' + snippet = spot[:snippet] + diff = snippet.size - (viewport_size - ellipsis.size) + + # snippet fits in the terminal + return spot if diff.negative? + + if spot[:first_column] < diff + snippet = snippet[0...snippet.size - diff] + { + **spot, + snippet: snippet + ellipsis + "\n", + last_column: [spot[:last_column], snippet.size].min + } + else + { + **spot, + snippet: ellipsis + snippet[diff..-1], + first_column: spot[:first_column] - (diff - ellipsis.size), + last_column: spot[:last_column] - (diff - ellipsis.size) + } + end + end + + def self.terminal_columns + # lazy load io/console in case viewport_size is set + require "io/console" + IO.console.winsize[1] + end end def self.formatter diff --git a/test/test_error_highlight.rb b/test/test_error_highlight.rb index f0da5b5..fd5e32a 100644 --- a/test/test_error_highlight.rb +++ b/test/test_error_highlight.rb @@ -12,6 +12,8 @@ def self.message_for(corrections) end def setup + ErrorHighlight::DefaultFormatter.viewport_size = 80 + if defined?(DidYouMean) @did_you_mean_old_formatter = DidYouMean.formatter DidYouMean.formatter = DummyFormatter @@ -1285,6 +1287,30 @@ def test_no_final_newline end end + def test_errors_on_small_viewports_when_error_lives_at_the_end + assert_error_message(NoMethodError, <<~END) do +undefined method 'gsuub' for an instance of String + +...ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".gsuub(//, "") + ^^^^^^ + END + + "fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".gsuub(//, "") + end + end + + def test_errors_on_small_viewports_when_error_lives_at_the_beginning + assert_error_message(NoMethodError, <<~END) do +undefined method 'gsuub' for an instance of Integer + + 1.gsuub(//, "fooooooooooooooooooooooooooooooooooooooooooooooooooooooooo... + ^^^^^^ + END + + 1.gsuub(//, "fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo") + end + end + def test_simulate_funcallv_from_embedded_ruby assert_error_message(NoMethodError, <<~END) do undefined method `foo' for #{ NIL_RECV_MESSAGE } From 10c69c4b9e3483c2b28981d1d324f8f889154ffc Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Sun, 13 Oct 2024 20:40:45 +0200 Subject: [PATCH 2/3] Handle very long lines with errors in the middle of the line --- lib/error_highlight/formatter.rb | 63 ++++++++++++++------------------ test/test_error_highlight.rb | 61 +++++++++++++++++++++++++------ 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/lib/error_highlight/formatter.rb b/lib/error_highlight/formatter.rb index b4fce93..4d5c82f 100644 --- a/lib/error_highlight/formatter.rb +++ b/lib/error_highlight/formatter.rb @@ -2,55 +2,46 @@ module ErrorHighlight class DefaultFormatter def self.message_for(spot) # currently only a one-line code snippet is supported - if spot[:first_lineno] == spot[:last_lineno] - spot = truncate(spot) + return "" unless spot[:first_lineno] == spot[:last_lineno] - indent = spot[:snippet][0...spot[:first_column]].gsub(/[^\t]/, " ") - marker = indent + "^" * (spot[:last_column] - spot[:first_column]) + snippet = spot[:snippet] + first_column = spot[:first_column] + last_column = spot[:last_column] - "\n\n#{ spot[:snippet] }#{ marker }" - else - "" + # truncate snippet to fit in the viewport + if snippet.size > viewport_size + visible_start = [first_column - viewport_size / 2, 0].max + visible_end = visible_start + viewport_size + + # avoid centering the snippet when the error is at the end of the line + visible_start = snippet.size - viewport_size if visible_end > snippet.size + + prefix = visible_start.positive? ? "..." : "" + suffix = visible_end < snippet.size ? "..." : "" + + snippet = prefix + snippet[(visible_start + prefix.size)...(visible_end - suffix.size)] + suffix + snippet << "\n" unless snippet.end_with?("\n") + + first_column = first_column - visible_start + last_column = [last_column - visible_start, snippet.size - 1].min end + + indent = snippet[0...first_column].gsub(/[^\t]/, " ") + marker = indent + "^" * (last_column - first_column) + + "\n\n#{ snippet }#{ marker }" end def self.viewport_size - Ractor.current[:__error_highlight_viewport_size__] || terminal_columns + Ractor.current[:__error_highlight_viewport_size__] ||= terminal_columns end def self.viewport_size=(viewport_size) Ractor.current[:__error_highlight_viewport_size__] = viewport_size end - private - - def self.truncate(spot) - ellipsis = '...' - snippet = spot[:snippet] - diff = snippet.size - (viewport_size - ellipsis.size) - - # snippet fits in the terminal - return spot if diff.negative? - - if spot[:first_column] < diff - snippet = snippet[0...snippet.size - diff] - { - **spot, - snippet: snippet + ellipsis + "\n", - last_column: [spot[:last_column], snippet.size].min - } - else - { - **spot, - snippet: ellipsis + snippet[diff..-1], - first_column: spot[:first_column] - (diff - ellipsis.size), - last_column: spot[:last_column] - (diff - ellipsis.size) - } - end - end - def self.terminal_columns - # lazy load io/console in case viewport_size is set + # lazy load io/console, so it's not loaded when viewport_size is set require "io/console" IO.console.winsize[1] end diff --git a/test/test_error_highlight.rb b/test/test_error_highlight.rb index fd5e32a..d00c049 100644 --- a/test/test_error_highlight.rb +++ b/test/test_error_highlight.rb @@ -5,6 +5,8 @@ require "tempfile" class ErrorHighlightTest < Test::Unit::TestCase + ErrorHighlight::DefaultFormatter.viewport_size = 80 + class DummyFormatter def self.message_for(corrections) "" @@ -12,8 +14,6 @@ def self.message_for(corrections) end def setup - ErrorHighlight::DefaultFormatter.viewport_size = 80 - if defined?(DidYouMean) @did_you_mean_old_formatter = DidYouMean.formatter DidYouMean.formatter = DummyFormatter @@ -1287,27 +1287,64 @@ def test_no_final_newline end end - def test_errors_on_small_viewports_when_error_lives_at_the_end + def test_errors_on_small_viewports_at_the_end + assert_error_message(NoMethodError, <<~END) do +undefined method `time' for #{ ONE_RECV_MESSAGE } + +...0000000000000000000000000000000000000000000000000000000000000000 + 1.time {} + ^^^^^ + END + + 100000000000000000000000000000000000000000000000000000000000000000000000000000 + 1.time {} + end + end + + def test_errors_on_small_viewports_at_the_beginning + assert_error_message(NoMethodError, <<~END) do +undefined method `time' for #{ ONE_RECV_MESSAGE } + + 1.time { 10000000000000000000000000000000000000000000000000000000000000... + ^^^^^ + END + + 1.time { 100000000000000000000000000000000000000000000000000000000000000000000000000000 } + + end + end + + def test_errors_on_small_viewports_at_the_middle + assert_error_message(NoMethodError, <<~END) do +undefined method `time' for #{ ONE_RECV_MESSAGE } + +...000000000000000000000000000000000 + 1.time { 10000000000000000000000000000... + ^^^^^ + END + + 100000000000000000000000000000000000000 + 1.time { 100000000000000000000000000000000000000 } + end + end + + def test_errors_on_small_viewports_when_larger_than_viewport assert_error_message(NoMethodError, <<~END) do -undefined method 'gsuub' for an instance of String +undefined method `timessssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss!' for #{ ONE_RECV_MESSAGE } -...ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".gsuub(//, "") - ^^^^^^ + 1.timesssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss... + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ END - "fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo".gsuub(//, "") + 1.timessssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss! end end - def test_errors_on_small_viewports_when_error_lives_at_the_beginning + def test_errors_on_small_viewports_when_exact_size_of_viewport assert_error_message(NoMethodError, <<~END) do -undefined method 'gsuub' for an instance of Integer +undefined method `timessssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss!' for #{ ONE_RECV_MESSAGE } - 1.gsuub(//, "fooooooooooooooooooooooooooooooooooooooooooooooooooooooooo... - ^^^^^^ + 1.timessssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss!... + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ END - 1.gsuub(//, "fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo") + 1.timessssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss! * 1000 end end From ce99773d0a3a7709bea8d2ba9dc4f451074b5eae Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Fri, 18 Oct 2024 01:12:21 +0200 Subject: [PATCH 3/3] Adjust truncation, add opt-out mechanism, rename methods, and prepare error highlighting to render on extremely small screens --- lib/error_highlight/formatter.rb | 47 ++++++++++++------ test/test_error_highlight.rb | 83 +++++++++++++++++++++++++++++--- 2 files changed, 108 insertions(+), 22 deletions(-) diff --git a/lib/error_highlight/formatter.rb b/lib/error_highlight/formatter.rb index 4d5c82f..0656a88 100644 --- a/lib/error_highlight/formatter.rb +++ b/lib/error_highlight/formatter.rb @@ -1,5 +1,7 @@ module ErrorHighlight class DefaultFormatter + MIN_SNIPPET_WIDTH = 20 + def self.message_for(spot) # currently only a one-line code snippet is supported return "" unless spot[:first_lineno] == spot[:last_lineno] @@ -7,22 +9,24 @@ def self.message_for(spot) snippet = spot[:snippet] first_column = spot[:first_column] last_column = spot[:last_column] + ellipsis = "..." # truncate snippet to fit in the viewport - if snippet.size > viewport_size - visible_start = [first_column - viewport_size / 2, 0].max - visible_end = visible_start + viewport_size + if snippet_max_width && snippet.size > snippet_max_width + available_width = snippet_max_width - ellipsis.size + center = first_column - snippet_max_width / 2 - # avoid centering the snippet when the error is at the end of the line - visible_start = snippet.size - viewport_size if visible_end > snippet.size + visible_start = last_column < available_width ? 0 : [center, 0].max + visible_end = visible_start + snippet_max_width + visible_start = snippet.size - snippet_max_width if visible_end > snippet.size - prefix = visible_start.positive? ? "..." : "" - suffix = visible_end < snippet.size ? "..." : "" + prefix = visible_start.positive? ? ellipsis : "" + suffix = visible_end < snippet.size ? ellipsis : "" snippet = prefix + snippet[(visible_start + prefix.size)...(visible_end - suffix.size)] + suffix snippet << "\n" unless snippet.end_with?("\n") - first_column = first_column - visible_start + first_column -= visible_start last_column = [last_column - visible_start, snippet.size - 1].min end @@ -32,18 +36,31 @@ def self.message_for(spot) "\n\n#{ snippet }#{ marker }" end - def self.viewport_size - Ractor.current[:__error_highlight_viewport_size__] ||= terminal_columns + def self.snippet_max_width + return if Ractor.current[:__error_highlight_max_snippet_width__] == :disabled + + Ractor.current[:__error_highlight_max_snippet_width__] ||= terminal_width end - def self.viewport_size=(viewport_size) - Ractor.current[:__error_highlight_viewport_size__] = viewport_size + def self.snippet_max_width=(width) + return Ractor.current[:__error_highlight_max_snippet_width__] = :disabled if width.nil? + + width = width.to_i + + if width < MIN_SNIPPET_WIDTH + warn "'snippet_max_width' adjusted to minimum value of #{MIN_SNIPPET_WIDTH}." + width = MIN_SNIPPET_WIDTH + end + + Ractor.current[:__error_highlight_max_snippet_width__] = width end - def self.terminal_columns - # lazy load io/console, so it's not loaded when viewport_size is set + def self.terminal_width + # lazy load io/console, so it's not loaded when snippet_max_width is set require "io/console" - IO.console.winsize[1] + STDERR.winsize[1] if STDERR.tty? + rescue LoadError, NoMethodError, SystemCallError + # do not truncate when window size is not available end end diff --git a/test/test_error_highlight.rb b/test/test_error_highlight.rb index d00c049..be36fca 100644 --- a/test/test_error_highlight.rb +++ b/test/test_error_highlight.rb @@ -5,7 +5,7 @@ require "tempfile" class ErrorHighlightTest < Test::Unit::TestCase - ErrorHighlight::DefaultFormatter.viewport_size = 80 + ErrorHighlight::DefaultFormatter.snippet_max_width = 80 class DummyFormatter def self.message_for(corrections) @@ -1287,7 +1287,7 @@ def test_no_final_newline end end - def test_errors_on_small_viewports_at_the_end + def test_errors_on_small_terminal_window_at_the_end assert_error_message(NoMethodError, <<~END) do undefined method `time' for #{ ONE_RECV_MESSAGE } @@ -1299,7 +1299,7 @@ def test_errors_on_small_viewports_at_the_end end end - def test_errors_on_small_viewports_at_the_beginning + def test_errors_on_small_terminal_window_at_the_beginning assert_error_message(NoMethodError, <<~END) do undefined method `time' for #{ ONE_RECV_MESSAGE } @@ -1312,7 +1312,19 @@ def test_errors_on_small_viewports_at_the_beginning end end - def test_errors_on_small_viewports_at_the_middle + def test_errors_on_small_terminal_window_at_the_middle_near_beginning + assert_error_message(NoMethodError, <<~END) do +undefined method `time' for #{ ONE_RECV_MESSAGE } + + 100000000000000000000000000000000000000 + 1.time { 1000000000000000000000... + ^^^^^ + END + + 100000000000000000000000000000000000000 + 1.time { 100000000000000000000000000000000000000 } + end + end + + def test_errors_on_small_terminal_window_at_the_middle assert_error_message(NoMethodError, <<~END) do undefined method `time' for #{ ONE_RECV_MESSAGE } @@ -1320,11 +1332,68 @@ def test_errors_on_small_viewports_at_the_middle ^^^^^ END - 100000000000000000000000000000000000000 + 1.time { 100000000000000000000000000000000000000 } + 10000000000000000000000000000000000000000000000000000000000000000000000 + 1.time { 1000000000000000000000000000000 } + end + end + + def test_errors_on_extremely_small_terminal_window + custom_max_width = 30 + original_max_width = ErrorHighlight::DefaultFormatter.snippet_max_width + + ErrorHighlight::DefaultFormatter.snippet_max_width = custom_max_width + + assert_error_message(NoMethodError, <<~END) do +undefined method `time' for #{ ONE_RECV_MESSAGE } + +...00000000 + 1.time { 1000... + ^^^^^ + END + + 100000000000000 + 1.time { 100000000000000 } + end + ensure + ErrorHighlight::DefaultFormatter.snippet_max_width = original_max_width + end + + def test_errors_on_terminal_window_smaller_than_min_width + custom_max_width = 5 + original_max_width = ErrorHighlight::DefaultFormatter.snippet_max_width + + ErrorHighlight::DefaultFormatter.snippet_max_width = custom_max_width + + assert_error_message(NoMethodError, <<~END) do +undefined method `time' for #{ ONE_RECV_MESSAGE } + +...000 + 1.time {... + ^^^^^ + END + + 100000000000000 + 1.time { 100000000000000 } + end + ensure + ErrorHighlight::DefaultFormatter.snippet_max_width = original_max_width + end + + def test_errors_on_terminal_window_when_truncation_is_disabled + custom_max_width = nil + original_max_width = ErrorHighlight::DefaultFormatter.snippet_max_width + + ErrorHighlight::DefaultFormatter.snippet_max_width = custom_max_width + + assert_error_message(NoMethodError, <<~END) do +undefined method `time' for #{ ONE_RECV_MESSAGE } + + 10000000000000000000000000000000000000000000000000000000000000000000000 + 1.time { 1000000000000000000000000000000 } + ^^^^^ + END + + 10000000000000000000000000000000000000000000000000000000000000000000000 + 1.time { 1000000000000000000000000000000 } end + ensure + ErrorHighlight::DefaultFormatter.snippet_max_width = original_max_width end - def test_errors_on_small_viewports_when_larger_than_viewport + def test_errors_on_small_terminal_window_when_larger_than_viewport assert_error_message(NoMethodError, <<~END) do undefined method `timessssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss!' for #{ ONE_RECV_MESSAGE } @@ -1336,7 +1405,7 @@ def test_errors_on_small_viewports_when_larger_than_viewport end end - def test_errors_on_small_viewports_when_exact_size_of_viewport + def test_errors_on_small_terminal_window_when_exact_size_of_viewport assert_error_message(NoMethodError, <<~END) do undefined method `timessssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss!' for #{ ONE_RECV_MESSAGE }