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 terminal ANSI escape sequences when the output is a file #498

Closed
yujinakayama opened this issue Sep 19, 2013 · 14 comments
Closed

Disable terminal ANSI escape sequences when the output is a file #498

yujinakayama opened this issue Sep 19, 2013 · 14 comments
Labels

Comments

@yujinakayama
Copy link
Collaborator

The continuation of #497.

@fuadsaud
Copy link

I'm not very familiar with rubocop's source code, but I checked it out and it seemed like a good solution to decorate the output used in the base formatter and then check whether it is a tty in order to add ansi escape sequences.

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 22, 2013

@fuadsaud Feel free to send a PR implementing your proposed solution :-) This won't affect paging programs understanding ANSI colors like less, right?

@fuadsaud
Copy link

Yes, a simple check for IO#tty? will return false for pipes and such. I'm not sure how to handle this either.

I'm not sure when I'll have time, but I plan to look into it. If anyone has a better idea, please go on.

@ghost ghost assigned jonas054 Dec 1, 2013
@jonas054
Copy link
Collaborator

jonas054 commented Dec 1, 2013

I think the problem should be solved in the Rainbow gem. I have opened an issue over there.

@jonas054
Copy link
Collaborator

Issue 16 in sickill/rainbow has been closed and version 1.99.0 of that gem has the fix. I have tested manually that color codes are not included when stderr is redirected to a file. A bundle update in the rubocop directory is needed.

@ku1ik
Copy link

ku1ik commented Dec 26, 2013

Copy pasting what I just wrote in #497 :

I have just released a new rainbow 1.99 which has this fixed (checks if any of stdout and stderr is not a tty). rainbow will drop monkey patching of String class in 2.0 and this 1.99 release is a preparation for that. You can now upgrade to 1.99 and convert all "foo".color(:red) to Rainbow("foo").color(:red). When 2.0 is out you'll be prepared.

@yujinakayama
Copy link
Collaborator Author

I don't think this issue is solved.

The rainbow's update solves the @fuadsaud's log that is caused by shell redirection. However, rainbow cannot detect whether a formatter's output is a file (with -o/--out option) or not, and as I wrote it cannot handle multiple formatters, though these are not rainbow's fault.

@yujinakayama yujinakayama reopened this Dec 26, 2013
@ku1ik
Copy link

ku1ik commented Dec 26, 2013

@yujinakayama rubocop can disable rainbow (via Rainbow.enabled = false) when a formatter's output is a file. I understand the case here is that you would want to both print to terminal with colors enabled and at the same time to a file with colors disabled, right?

A solution for that would be something like this:

FileRainbow = Rainbow.get_instance
FileRainbow.enabled = <...your custom condition...>

For example:

# this adds color if the terminal is a TTY
Rainbow("foo").color(:red) # => "\e[31mfoo\e[0m"

# this never adds a color
FileRainbow.enabled = false
FileRainbow("foo").color(:red) # => "foo"

Would that solve this rubocop problem?

@ku1ik
Copy link

ku1ik commented Dec 26, 2013

Basically, each formatter could acquire its own instance of Rainbow and rubocop could disable coloring on the Rainbow instances that are used by formatters that output to files.

@yujinakayama
Copy link
Collaborator Author

@sickill

I understand the case here is that you would want to both print to terminal with colors enabled and at the same time to a file with colors disabled, right?

Yes.

The proposed solution is great. If rainbow provides the feature, we can solve the problem without migrating from rainbow. Although I think Rainbow.new would be more idiomatic than Rainbow.get_instance.

@ku1ik
Copy link

ku1ik commented Dec 27, 2013

👍 on Rainbow.new, I agree that it's better.

I should find some time for this in the coming days, I'll let you know when it gets implemented.

@yujinakayama
Copy link
Collaborator Author

@sickill Thanks!

@ku1ik
Copy link

ku1ik commented Dec 28, 2013

@yujinakayama it's there. I have just released 1.99.1 with a support for multiple rainbow wrapper instances. Check the readme section "Advanced usage".

@yujinakayama
Copy link
Collaborator Author

@sickill Thanks! I'll try later.

yujinakayama added a commit that referenced this issue Dec 30, 2013
kpj referenced this issue in brucemiller/LaTeXML Aug 11, 2015
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