From 8327c6bd636ab209773f375c9f3a88ddf68f7bb4 Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 25 Oct 2021 21:17:36 -0500 Subject: [PATCH] Use default value obj for terminal check In #91 the behavior of dead_end is now optimized to output values based on if the io object is a TTY (interactive terminal) or not. This behavior is awesome. The way we were checking default values though introduces a subtle bug, in Ruby `nil` and `false` are usually assumed to have the same/similar behavior (being falsey). This updates the logic to use a `Object.new` value to indicate a default value rather than rely on specific contents. It will reduce the chance the programmer accidentally passes in a value that triggers the default behavior. I also REALLY wanted to find a way to assert the inverse of the test added in "passing --terminal will force color codes" but it turns out it's really hard (if not impossible). I asked on twitter as well https://twitter.com/schneems/status/1452814816456945672. Instead I added some unit tests to at least assert that calling DisplayInvalidBlocks with default args defaulted to `io.isatty`. I also got rid of the describe block for now as there's only one thing in there. It makes diffs a bit more muddled if the contents of the tests get moved around. (Personal preference) --- exe/dead_end | 2 +- lib/dead_end/display_invalid_blocks.rb | 5 +-- lib/dead_end/internals.rb | 8 +++-- spec/integration/exe_cli_spec.rb | 18 +++++----- spec/unit/display_invalid_blocks_spec.rb | 44 ++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 15 deletions(-) diff --git a/exe/dead_end b/exe/dead_end index 2ea5070..e83ac53 100755 --- a/exe/dead_end +++ b/exe/dead_end @@ -70,7 +70,7 @@ warn "Record dir: #{options[:record_dir]}" if options[:record_dir] display = DeadEnd.call( source: file.read, filename: file.expand_path, - terminal: options[:terminal], + terminal: options.fetch(:terminal, DeadEnd::DEFAULT_VALUE), record_dir: options[:record_dir] ) diff --git a/lib/dead_end/display_invalid_blocks.rb b/lib/dead_end/display_invalid_blocks.rb index f659473..c0b0959 100644 --- a/lib/dead_end/display_invalid_blocks.rb +++ b/lib/dead_end/display_invalid_blocks.rb @@ -9,8 +9,9 @@ module DeadEnd class DisplayInvalidBlocks attr_reader :filename - def initialize(code_lines:, blocks:, io: $stderr, filename: nil, terminal: nil, invalid_obj: WhoDisSyntaxError::Null.new) - @terminal = terminal.nil? ? io.isatty : terminal + def initialize(code_lines:, blocks:, io: $stderr, filename: nil, terminal: DEFAULT_VALUE, invalid_obj: WhoDisSyntaxError::Null.new) + @terminal = terminal == DEFAULT_VALUE ? io.isatty : terminal + @filename = filename @io = io diff --git a/lib/dead_end/internals.rb b/lib/dead_end/internals.rb index 26008f9..9dc4a76 100644 --- a/lib/dead_end/internals.rb +++ b/lib/dead_end/internals.rb @@ -12,6 +12,10 @@ require "timeout" module DeadEnd + # Used to indicate a default value that cannot + # be confused with another input + DEFAULT_VALUE = Object.new.freeze + class Error < StandardError; end SEARCH_SOURCE_ON_ERROR_DEFAULT = true TIMEOUT_DEFAULT = ENV.fetch("DEAD_END_TIMEOUT", 1).to_i @@ -27,14 +31,14 @@ def self.handle_error(e, search_source_on_error: SEARCH_SOURCE_ON_ERROR_DEFAULT) if search_source_on_error call( source: Pathname(filename).read, - filename: filename, + filename: filename ) end raise e end - def self.call(source:, filename:, terminal: nil, record_dir: nil, timeout: TIMEOUT_DEFAULT, io: $stderr) + def self.call(source:, filename:, terminal: DEFAULT_VALUE, record_dir: nil, timeout: TIMEOUT_DEFAULT, io: $stderr) search = nil Timeout.timeout(timeout) do record_dir ||= ENV["DEBUG"] ? "tmp" : nil diff --git a/spec/integration/exe_cli_spec.rb b/spec/integration/exe_cli_spec.rb index b3b606b..7e26d8f 100644 --- a/spec/integration/exe_cli_spec.rb +++ b/spec/integration/exe_cli_spec.rb @@ -50,17 +50,15 @@ def exe(cmd) end end - describe "terminal coloring" do - # When ruby sub shells it is not a interactive shell and dead_end will - # default to no coloring. - - it "passing --terminal will force color codes" do - ruby_file = fixtures_dir.join("this_project_extra_def.rb.txt") - out = exe("#{ruby_file} --terminal") + # When ruby sub shells it is not a interactive shell and dead_end will + # default to no coloring. Colors/bold can be forced with `--terminal` + # flag + it "passing --terminal will force color codes" do + ruby_file = fixtures_dir.join("this_project_extra_def.rb.txt") + out = exe("#{ruby_file} --terminal") - expect(out.strip).to include("Missing `end` detected") - expect(out.strip).to include("\e[0m❯ 36 \e[1;3m def filename") - end + expect(out.strip).to include("Missing `end` detected") + expect(out.strip).to include("\e[0m❯ 36 \e[1;3m def filename") end it "records search" do diff --git a/spec/unit/display_invalid_blocks_spec.rb b/spec/unit/display_invalid_blocks_spec.rb index 64e9674..96da06b 100644 --- a/spec/unit/display_invalid_blocks_spec.rb +++ b/spec/unit/display_invalid_blocks_spec.rb @@ -27,6 +27,50 @@ def hai expect(io.string).to include("Syntax OK") end + it "selectively prints to terminal if input is a tty by default" do + source = <<~EOM + class OH + def hello + def hai + end + end + EOM + + code_lines = CleanDocument.new(source: source).call.lines + + io = StringIO.new + def io.isatty + true + end + + block = CodeBlock.new(lines: code_lines[1]) + display = DisplayInvalidBlocks.new( + io: io, + blocks: block, + code_lines: code_lines + ) + display.call + expect(io.string).to include([ + "❯ 2 ", + DisplayCodeWithLineNumbers::TERMINAL_HIGHLIGHT, + " def hello" + ].join) + + io = StringIO.new + def io.isatty + false + end + + block = CodeBlock.new(lines: code_lines[1]) + display = DisplayInvalidBlocks.new( + io: io, + blocks: block, + code_lines: code_lines + ) + display.call + expect(io.string).to include("❯ 2 def hello") + end + it "outputs to io when using `call`" do source = <<~EOM class OH