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

Disable color when output to file using `--out` / allow `--color` to be a per-formatter option like `--out` #1288

Open
TylerRick opened this issue Apr 6, 2018 · 5 comments

Comments

@TylerRick
Copy link

commented Apr 6, 2018

Summary

It seems like the default behavior should be for formatters that are output to the screen to have color enabled by default, and formatters that are directed to files to have no color by default, so that you can open them in an editor and not see gobbledeygook.

The default should be overridable, however, in case you really did want to view the saved output on a terminal later (perhaps with less -R).

Current Behavior

lib/cucumber/formatter/ansicolor.rb only disables color if you redirect stdout, as in cucumber > file:

Cucumber::Term::ANSIColor.coloring = false if !STDOUT.tty?

However, it doesn't provide any way to disable color for when directing a specific formatter to a
file with:

  --format #{format} --out #{file}

Expected Behavior

1. Default behavior

Formatters that are output to the screen (@io.tty?) should have color enabled by default

Formatters that are directed to files (@io.is_a?(File)) to have no color by default.

The help says it is based on the output destination but that is not true when using --out.

    -c, --[no-]color                 Whether or not to use ANSI color in the output. Cucumber decides
                                     based on your platform and the output destination if not specified.

2. Ability to enable/disable color on a per-formatter basis (like --out)

It would be nice if you could disable color on a per-formatter basis, like how you can direct to a
different output on a per-formatter basis.

    -o, --out [FILE|DIR]             Write output to a file/directory instead of STDOUT. This option
                                     applies to the previously specified --format,

Maybe something like this:

  --format format1 --no-color --out file --format format2 --color

but since --color/--no-color is a global state, the last flag changes it for all formatters.

So the best you can do with --no-color is turn off color for all formatters, including the one
being output to a tty STDOUT.

Possible Solution

One solution/workaround would be to strip out color in format_string:

module Cucumber
  module Formatter
    module Console
      def format_string(o, status)
        fmt = format_for(status)
        o.to_s.split("\n").map do |line|
          if @io.is_a?(File)
            line.gsub(Cucumber::Term::ANSIColor::COLORED_REGEXP, '')
          else
            if Proc === fmt
              fmt.call(line)
            else
              fmt % line
            end
          end
        end.join("\n")
      end
    end
  end
end

That seems to work for me, striping out all color except the summary at the bottom. Probably not the best solution, though.

Probably should move Cucumber::Term::ANSIColor.coloring? so that it's an instance method instead of a class method...

Steps to Reproduce (for bugs)

  1. cucumber --format pretty --out pretty.txt --format progress features/
  2. Observe that stdout has color (as desired).
  3. Observe that pretty.txt contains unwanted color codes.

Your Environment

  • cucumber-3.1.0
  • cucumber-rails-1.5.0
@danascheider

This comment has been minimized.

Copy link
Contributor

commented May 26, 2018

I like this idea. WDYT @mattwynne?

@mattwynne

This comment has been minimized.

Copy link
Member

commented May 31, 2018

Hello @danascheider long time no see!

Yes, this seems like a good idea to me.

@danascheider

This comment has been minimized.

Copy link
Contributor

commented May 31, 2018

I know! Life's been crazy. I might pick this up when I'm done with the other one I'm working on but don't want to leave a bunch of half-finished PR's lying around!

@stale

This comment has been minimized.

Copy link

commented Jul 30, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the Stale label Jul 30, 2018
@xtrasimplicity xtrasimplicity added Slow Burner and removed Stale labels Jul 30, 2018
@danascheider

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2018

Hey, just noticed I said I'd pick this up when things calmed down. I'm taking a look at it now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.