Compiler: use Colorize.on_tty_only!

To support TTY detection

Fixed specs and wrapper script (especially wrapper script is buggy)
Added Colorize.wrap_enabled, enable, disable utility method

bin/crystal

@@ -115,7 +115,11 @@ __has_colors() {
     return 1
   fi
 }
-SUPPORTS_COLORS=$(__has_colors)
+if __has_colors && [ -t 1 ] && [ -t 2 ]; then
+  SUPPORTS_COLORS=true
+else
+  SUPPORTS_COLORS=false
+fi
 __error_msg() {
   if $SUPPORTS_COLORS; then
     # bold red coloring

spec/compiler/semantic/did_you_mean_spec.cr

@@ -237,8 +237,6 @@ describe "Semantic: did you mean" do
   end
 
   it "suggests a better alternative to logical operators (#2715)" do
-    message = "undefined method 'and'"
-    message = " (did you mean '&&'?)".colorize.yellow.bold.to_s
     assert_error %(
       def rand(x : Int32)
       end
@@ -252,7 +250,7 @@ describe "Semantic: did you mean" do
       if "a".bytes and 1
         1
       end
-      ), message
+      ), "did you mean '&&'"
   end
 
   it "says did you mean in instance var declaration" do

spec/compiler/semantic/generic_class_spec.cr

@@ -636,7 +636,7 @@ describe "Semantic: generic class" do
     begin
       semantic(nodes)
     rescue ex : TypeException
-      msg = ex.to_s.lines.map(&.strip)
+      msg = Colorize.disable { ex.to_s.lines.map(&.strip) }
       msg.count("- Foo(T).foo(x : Int32)").should eq(1)
     end
   end

src/colorize.cr

@@ -95,6 +95,27 @@ module Colorize
   def self.on_tty_only!
     self.enabled = STDOUT.tty? && STDERR.tty?
   end
+
+  # Set given value to `Colorize.enabled`, then execute a block and reset `Colorize.enabled`.
+  def self.wrap_enabled(enabled)
+    old_enabled = enabled?
+    begin
+      self.enabled = enabled
+      yield
+    ensure
+      self.enabled = old_enabled
+    end
+  end
+
+  # Alias for `Colorize.wrap_enabled(true) { ... }`.
+  def self.enable
+    self.wrap_enabled(true) { yield }
+  end
+
+  # Alias for `Colorize.wrap_enabled(false) { ... }`.
+  def self.disable
+    self.wrap_enabled(false) { yield }
+  end
 end
 
 def with_color

src/compiler/crystal/command.cr

@@ -52,11 +52,12 @@ class Crystal::Command
   private getter options
 
   def initialize(@options : Array(String))
-    @color = true
     @progress_tracker = ProgressTracker.new
   end
 
   def run
+    Colorize.on_tty_only!
+
     command = options.first?
 
     if command
@@ -114,7 +115,6 @@ class Crystal::Command
   rescue ex : Crystal::ToolException
     error ex.message
   rescue ex : Crystal::Exception
-    ex.color = @color
     if @config.try(&.output_format) == "json"
       puts ex.to_json
     else
@@ -350,9 +350,12 @@ class Crystal::Command
         end
       end
 
+      opts.on("--color", "Enable colored output") do
+        Colorize.enabled = true
+      end
+
       opts.on("--no-color", "Disable colored output") do
-        @color = false
-        compiler.color = false
+        Colorize.enabled = false
       end
 
       unless no_codegen
@@ -488,9 +491,11 @@ class Crystal::Command
       puts opts
       exit
     end
+    opts.on("--color", "Enable colored output") do
+      Colorize.enabled = true
+    end
     opts.on("--no-color", "Disable colored output") do
-      @color = false
-      compiler.color = false
+      Colorize.enabled = false
     end
     opts.invalid_option { }
   end
@@ -506,7 +511,17 @@ class Crystal::Command
 
   private def error(msg, exit_code = 1)
     # This is for the case where the main command is wrong
-    @color = false if ARGV.includes?("--no-color")
-    Crystal.error msg, @color, exit_code: exit_code
+    ARGV.each do |arg|
+      case arg
+      when "--"
+        break
+      when "--color"
+        Colorize.enabled = true
+      when "--no-color"
+        Colorize.enabled = false
+      end
+    end
+
+    Crystal.error msg, exit_code: exit_code
   end
 end

src/compiler/crystal/command/format.cr

@@ -34,8 +34,12 @@ class Crystal::Command
           exit
         end
 
+        opts.on("--color", "Enable colored output") do
+          Colorize.enabled = true
+        end
+
         opts.on("--no-color", "Disable colored output") do
-          @color = false
+          Colorize.enabled = false
         end
       end
 
@@ -86,8 +90,8 @@ class Crystal::Command
       print result
       STDOUT.flush
     rescue ex : InvalidByteSequenceError
-      print "Error: ".colorize.toggle(@color).red.bold
-      print "source is not a valid Crystal source file: ".colorize.toggle(@color).bold
+      print "Error: ".colorize.red.bold
+      print "source is not a valid Crystal source file: ".colorize.bold
       puts ex.message
       exit 1
     rescue ex : Crystal::SyntaxException
@@ -114,8 +118,8 @@ class Crystal::Command
 
       File.write(filename, result)
     rescue ex : InvalidByteSequenceError
-      print "Error: ".colorize.toggle(@color).red.bold
-      print "file '#{Crystal.relative_filename(filename)}' is not a valid Crystal source file: ".colorize.toggle(@color).bold
+      print "Error: ".colorize.bold
+      print "file '#{Crystal.relative_filename(filename)}' is not a valid Crystal source file: ".colorize.bold
       puts ex.message
       exit 1
     rescue ex : Crystal::SyntaxException
@@ -162,21 +166,21 @@ class Crystal::Command
         check_files << FormatResult.new(filename, FormatResult::Code::FORMAT)
       else
         File.write(filename, result)
-        STDOUT << "Format".colorize(:green).toggle(@color) << " " << filename << "\n"
+        STDOUT << "Format".colorize(:green) << " " << filename << "\n"
       end
     rescue ex : InvalidByteSequenceError
       if check_files
         check_files << FormatResult.new(filename, FormatResult::Code::INVALID_BYTE_SEQUENCE)
       else
-        print "Error: ".colorize.toggle(@color).red.bold
-        print "file '#{Crystal.relative_filename(filename)}' is not a valid Crystal source file: ".colorize.toggle(@color).bold
+        print "Error: ".colorize.red.bold
+        print "file '#{Crystal.relative_filename(filename)}' is not a valid Crystal source file: ".colorize.bold
         puts ex.message
       end
     rescue ex : Crystal::SyntaxException
       if check_files
         check_files << FormatResult.new(filename, FormatResult::Code::SYNTAX)
       else
-        STDOUT << "Syntax Error:".colorize(:yellow).toggle(@color) << " " << ex.message << " at " << filename << ":" << ex.line_number << ":" << ex.column_number << "\n"
+        STDOUT << "Syntax Error:".colorize(:yellow) << " " << ex.message << " at " << filename << ":" << ex.line_number << ":" << ex.column_number << "\n"
       end
     rescue ex
       if check_files
@@ -190,7 +194,7 @@ class Crystal::Command
   end
 
   private def couldnt_format(file)
-    STDERR << "Error:".colorize(:red).toggle(@color) << ", " <<
+    STDERR << "Error:".colorize(:red) << ", " <<
       "couldn't format " << file << ", please report a bug including the contents of it: https://github.com/crystal-lang/crystal/issues"
   end
 end

src/compiler/crystal/compiler.cr

@@ -57,9 +57,6 @@ module Crystal
     # Sets the mattr (features). Check LLVM docs to learn about this.
     property mattr : String?
 
-    # If `false`, color won't be used in output messages.
-    property? color = true
-
     # If `true`, skip cleanup process on semantic analysis.
     property? no_cleanup = false
 
@@ -175,7 +172,6 @@ module Crystal
       program.flags << "debug" unless debug.none?
       program.flags.concat @flags
       program.wants_doc = wants_doc?
-      program.color = color?
       program.stdout = stdout
       program.show_error_trace = show_error_trace?
       program.progress_tracker = @progress_tracker
@@ -207,8 +203,8 @@ module Crystal
       parser.wants_doc = wants_doc?
       parser.parse
     rescue ex : InvalidByteSequenceError
-      stdout.print colorize("Error: ").red.bold
-      stdout.print colorize("file '#{Crystal.relative_filename(source.filename)}' is not a valid Crystal source file: ").bold
+      stdout.print "Error: ".colorize.red.bold
+      stdout.print "file '#{Crystal.relative_filename(source.filename)}' is not a valid Crystal source file: ".colorize.bold
       stdout.puts ex.message
       exit 1
     end
@@ -410,7 +406,7 @@ module Crystal
         TargetMachine.create(triple, @mcpu || "", @mattr || "", @release)
       end
     rescue ex : ArgumentError
-      stdout.print colorize("Error: ").red.bold
+      stdout.print "Error: ".colorize.red.bold
       stdout.print "llc: "
       stdout.puts ex.message
       exit 1
@@ -466,11 +462,7 @@ module Crystal
     end
 
     private def error(msg, exit_code = 1)
-      Crystal.error msg, @color, exit_code, stderr: stderr
-    end
-
-    private def colorize(obj)
-      obj.colorize.toggle(@color)
+      Crystal.error msg, exit_code, stderr: stderr
     end
 
     # An LLVM::Module with information to compile it.

src/compiler/crystal/exception.cr

@@ -3,8 +3,6 @@ require "colorize"
 
 module Crystal
   abstract class Exception < ::Exception
-    property? color = false
-
     @filename : String | VirtualFile | Nil
 
     def to_s(io)
@@ -46,14 +44,6 @@ module Crystal
       Crystal.relative_filename(filename)
     end
 
-    def colorize(obj)
-      obj.colorize.toggle(@color)
-    end
-
-    def with_color
-      ::with_color.toggle(@color)
-    end
-
     def replace_leading_tabs_with_spaces(line)
       found_non_space = false
       line.gsub do |char|

src/compiler/crystal/program.cr

@@ -67,9 +67,6 @@ module Crystal
     # If `true`, doc comments are attached to types and methods.
     property? wants_doc = false
 
-    # If `true`, error messages can be colorized
-    property? color = true
-
     # All required files. The set stores absolute files. This way
     # files loaded by `require` nodes are only processed once.
     getter requires = Set(String).new
@@ -497,12 +494,6 @@ module Crystal
       "__temp_#{@temp_var_counter}"
     end
 
-    # Colorizes the given object, depending on whether this program
-    # is configured to use colors.
-    def colorize(obj)
-      obj.colorize.toggle(@color)
-    end
-
     private def abstract_value_type(type)
       type.abstract = true
       type.struct = true

src/compiler/crystal/semantic/call_error.cr

@@ -29,7 +29,7 @@ class Crystal::Path
 
     similar_name = type.lookup_similar_path(self)
     if similar_name
-      self.raise("undefined constant #{self} #{type.program.colorize("(did you mean '#{similar_name}')").yellow.bold}")
+      self.raise("undefined constant #{self} #{"(did you mean '#{similar_name}')".colorize.yellow.bold}")
     else
       self.raise("undefined constant #{self}")
     end
@@ -101,15 +101,15 @@ class Crystal::Call
         end
 
         if obj && obj.type != owner
-          msg << colorize(" (compile-time type is #{obj.type})").yellow.bold
+          msg << " (compile-time type is #{obj.type})".colorize.yellow.bold
         end
 
         if similar_name
           if similar_name == def_name
             # This check is for the case `a if a = 1`
-            msg << colorize(" (If you declared '#{def_name}' in a suffix if, declare it in a regular if for this to work. If the variable was declared in a macro it's not visible outside it)").yellow.bold
+            msg << " (If you declared '#{def_name}' in a suffix if, declare it in a regular if for this to work. If the variable was declared in a macro it's not visible outside it)".colorize.yellow.bold
           else
-            msg << colorize(" (did you mean '#{similar_name}'?)").yellow.bold
+            msg << " (did you mean '#{similar_name}'?)".colorize.yellow.bold
           end
         end
 
@@ -121,9 +121,9 @@ class Crystal::Call
           if deps && deps.size == 1 && deps.first.same?(program.nil_var)
             similar_name = scope.lookup_similar_instance_var_name(ivar.name)
             if similar_name
-              msg << colorize(" (#{ivar.name} was never assigned a value, did you mean #{similar_name}?)").yellow.bold
+              msg << " (#{ivar.name} was never assigned a value, did you mean #{similar_name}?)".colorize.yellow.bold
             else
-              msg << colorize(" (#{ivar.name} was never assigned a value)").yellow.bold
+              msg << " (#{ivar.name} was never assigned a value)".colorize.yellow.bold
             end
           end
         end
@@ -135,7 +135,7 @@ class Crystal::Call
 
     # If it's on an initialize method and there's a similar method name, it's probably a typo
     if (def_name == "initialize" || def_name == "new") && (similar_def = owner.instance_type.lookup_similar_def("initialize", self.args.size, block))
-      inner_msg = colorize("do you maybe have a typo in this '#{similar_def.name}' method?").yellow.bold.to_s
+      inner_msg = "do you maybe have a typo in this '#{similar_def.name}' method?".colorize.yellow.bold.to_s
       inner_exception = TypeException.for_node(similar_def, inner_msg)
     end
 
@@ -387,10 +387,10 @@ class Crystal::Call
       str << "\n - "
       append_def_full_name a_def.owner, a_def, arg_types, str
       if defs.size > 1 && a_def.same?(matched_def)
-        str << colorize(" (trying this one)").blue
+        str << " (trying this one)".colorize.blue
       end
       if a_def.args.any? { |arg| arg.default_value && arg.external_name == argument_name }
-        str << colorize(" (did you mean this one?)").yellow.bold
+        str << " (did you mean this one?)".colorize.yellow.bold
       end
     end
   end
@@ -550,7 +550,7 @@ class Crystal::Call
           str << named_arg.name
           str << "'"
           if similar_name
-            str << colorize(" (did you mean '#{similar_name}'?)").yellow.bold
+            str << " (did you mean '#{similar_name}'?)".colorize.yellow.bold
           end
 
           defs = owner.lookup_defs(a_def.name)
@@ -652,8 +652,4 @@ class Crystal::Call
       "#{owner}##{method_name}"
     end
   end
-
-  private def colorize(obj)
-    program.colorize(obj)
-  end
 end