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 · 7 comments
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team ✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality

Comments

@TylerRick
Copy link

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 danascheider added ⚡ enhancement Request for new functionality good first issue Good for newcomers labels May 26, 2018
@danascheider
Copy link
Contributor

I like this idea. WDYT @mattwynne?

@mattwynne
Copy link
Member

Hello @danascheider long time no see!

Yes, this seems like a good idea to me.

@danascheider
Copy link
Contributor

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
Copy link

stale bot 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 Will soon be closed by stalebot unless there is activity label Jul 30, 2018
@xtrasimplicity xtrasimplicity added 🧷 pinned Tells Stalebot not to close this issue and removed ⌛ stale Will soon be closed by stalebot unless there is activity labels Jul 30, 2018
@danascheider
Copy link
Contributor

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

@aslakhellesoy aslakhellesoy removed Hacktoberfest 🧷 pinned Tells Stalebot not to close this issue labels Feb 1, 2021
@aslakhellesoy aslakhellesoy added this to Next in Cucumber Open Feb 2, 2021
@aslakhellesoy aslakhellesoy added the ✅ accepted The core team has agreed that it is a good idea to fix this label Feb 2, 2021
@aurelien-reeves aurelien-reeves removed this from Next in Cucumber Open Feb 16, 2021
@aurelien-reeves aurelien-reeves added the 🙏 help wanted Help wanted - not prioritized by core team label Feb 17, 2021
@CrossCascade013

This comment was marked as off-topic.

@luke-hill

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team ✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality
Projects
None yet
Development

No branches or pull requests

9 participants