Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--no-color flag doesn't suppress color output #4292

Closed
veelenga opened this issue Apr 14, 2017 · 7 comments
Closed

--no-color flag doesn't suppress color output #4292

veelenga opened this issue Apr 14, 2017 · 7 comments
Labels

Comments

@veelenga
Copy link
Contributor

I tried 3 cases:

  1. crystal eval:
$ crystal eval --no-color                                                                                                                                                             
require "colorize"
puts "hello".colorize(:red)
hello #=> colorized output
  1. crystal run:
$ cat test.cr                                                                                                                                                                          
require "colorize"
puts "hello".colorize(:red)

$ crystal test.cr --no-color                                                                                                                                                           
hello #=> colorized output
  1. crystal spec:
 $ crystal spec spec/std/bool_spec.cr --no-color                                                                                                                                        
....................  #=> colorized output

Finished in 830 microseconds
20 examples, 0 failures, 0 errors, 0 pending #=> colorized output

I expect the output colors to be suppressed in each of those cases.

Crystal 0.21.1+70 [7ca33b9] (2017-04-14) LLVM 4.0.0 on OS X 10.12

@bew
Copy link
Contributor

bew commented Apr 14, 2017

The --no-color flag disable formatting (color, bold,..) in the compiler, not in the executed program.
For example, a compilation error with --no-color won't be in bold.

@veelenga
Copy link
Contributor Author

veelenga commented Apr 14, 2017

@bew ok, that's not really intuitive at the first glance but makes sense:

$ crystal eval                                                                                                                                                                   
puts a
Error in line 1: undefined local variable or method 'a' (did you mean 'p'?) # => colorized

$ crystal eval --no-color                                                                                                                                                              
puts a
Error in line 1: undefined local variable or method 'a' (did you mean 'p'?) # => not colorized

What about spec output (case 3) ? #1587

@bew
Copy link
Contributor

bew commented Apr 14, 2017

About the spec output, the mentioned patch seems to be gone now..

Maybe we could add an ENV variable SPEC_NO_COLOR when --no-color is specified, so the compiled specs can disable colors at compile-time

@bew
Copy link
Contributor

bew commented Apr 14, 2017

something like this for example:

diff --git a/src/compiler/crystal/command/spec.cr b/src/compiler/crystal/command/spec.cr
index 49f1803df..b87849d4c 100644
--- a/src/compiler/crystal/command/spec.cr
+++ b/src/compiler/crystal/command/spec.cr
@@ -60,6 +60,10 @@ class Crystal::Command
       end
     end
 
+    if @color
+      compiler.flags << "SPEC_NO_COLOR"
+    end
+
     source_filename = File.expand_path("spec")
 
     source = target_filenames.map { |filename| %(require "./#{filename}") }.join("\n")
diff --git a/src/spec.cr b/src/spec.cr
index cbc605a6c..d08539686 100644
--- a/src/spec.cr
+++ b/src/spec.cr
@@ -113,6 +113,10 @@ if ENV["SPEC_VERBOSE"]? == "1"
   Spec.override_default_formatter(Spec::VerboseFormatter.new)
 end
 
+{% if flag?("SPEC_NO_COLOR") %}
+  Spec.use_colors = false
+{% end %}
+
 Signal::INT.trap { Spec.abort! }
 
 Spec.run

@ysbaddaden
Copy link
Contributor

Couldn't the crystal spec command accept --no-color, or pass it along?

@luislavena
Copy link
Contributor

You can pass --no-color to the compiled spec and it will partially disable coloring.

The option is not accepted or passed by to the child program by crystal spec (or displayed at all):

$ cat 48.cr 
require "spec"

$ crystal spec 48.cr -- --help
crystal spec runner
    -e , --example STRING            run examples whose full nested names include STRING
    -l , --line LINE                 run examples whose line matches LINE
    -p, --profile                    Print the 10 slowest specs
    --fail-fast                      abort the run on first failure
    --location file:line             run example at line 'line' in file 'file', multiple allowed
    --junit_output OUTPUT_DIR        generate JUnit XML output
    --help                           show this help
    -v, --verbose                    verbose output
    --no-color                       Disable colored output

$ cat 50.cr 
require "spec"

describe "Something" do
  it "fails" do
    true.should be_false
  end
end

$ crystal spec 50.cr -- --no-color
Using compiled compiler at .build/crystal
F

Failures:

  1) Something fails
     Failure/Error: true.should be_false

       Expected: false
            got: true

     # 50.cr:5

Finished in 96 microseconds
1 examples, 1 failures, 0 errors, 0 pending

Failed examples:

crystal spec 50.cr:4 # Something fails

And even so contains colored elements:

screenshot from 2017-04-14 12-25-14

Versus full colored one:

screenshot from 2017-04-14 12-26-17

asterite pushed a commit that referenced this issue Apr 18, 2017
Spec runner have the option to disable colored output (ANSI codes) when
`--no-color` option is used, example:

    $ crystal spec -- --no-color

However, certain elements in the output didn't fully support that option,
resulting in a mix of non-color and color.

This change unifies that setting and ensures that `--no-color` usage
is fully honored by the output.

Ref #4292
@veelenga
Copy link
Contributor Author

Thx @luislavena

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants