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

Some airbrussh output is "invisible" in certain terminal color schemes #84

Closed
mattbrictson opened this issue Jul 5, 2016 · 10 comments
Closed

Comments

@mattbrictson
Copy link
Owner

mattbrictson commented Jul 5, 2016

We've received multiple reports (from @Startouf and @dan-klasson in capistrano/capistrano#1669) that important error messages are not being rendered by Airbrussh.

Specifically, the color of the message is indistinguishable from the background color of the terminal, making them "invisible".

Suggestions of how to reproduce this (what OS and terminal is affected?) and how to fix it would be greatly appreciated.

234c8794-1612-11e6-9f86-00bf3e6686e3

@huerlisi
Copy link
Contributor

Same issue here. I'm using the 'Solarized' Color Scheme of KDE's Konsole in version 15.08.0 as provided by Ubuntu 15.10.

@dan-klasson
Copy link

I'm using 'Solarized' Color Scheme on Ubuntu Gnome 16.04

On Tue, Jul 26, 2016 at 6:47 PM, Simon Hürlimann [email protected]
wrote:

Same issue here. I'm using the 'Solarized' Color Scheme of KDE's Konsole
in version 15.08.0 as provided by Ubuntu 15.10.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#84 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABQQFrvQPD1z757wWi44zN4eMe0JciJ2ks5qZfO3gaJpZM4JFYuH
.

@mattbrictson
Copy link
Owner Author

Thanks for the additional data points.

Can both of you run this simple script in irb and post a screenshot of your output?

gem "airbrussh", "1.0.2"
require "airbrussh/colors"
Airbrussh::Colors::ANSI_CODES.keys.each do |color|
  puts Airbrussh::Colors.public_send(color, color)
end

Here's what I see:

screen shot 2016-07-26 at 9 47 23 am

@huerlisi
Copy link
Contributor

Only solarized has the issue. Even solarized light uses a proper gray:
solarized

solarized-light

So it's probably an issue with the solarized theme.

@mattbrictson
Copy link
Owner Author

Great, looks like we've got this figured out.

I know that solarized is popular, and I don't want Airbrussh to obscure important messages when people are using it with solarized. I think that warrants changing Airbrussh's default colors (even if solarized is ultimately at fault).

Do you agree?

Right now Airbrussh uses the gray to display info log messages (e.g. "linked file does not exist", upload progress, etc.), and also the time elapsed for each command. If we don't use gray for those because of the solarized issue, what color should we use instead?

@huerlisi
Copy link
Contributor

Cool!

I'd go with the default color, like the irb output.

@mattbrictson
Copy link
Owner Author

Any chance you'd be willing to do a PR? It should just be a matter of removing the use of gray from command_formatter.rb and console_formatter.rb and then updating the expected output in the tests.

@huerlisi
Copy link
Contributor

Yepp, can give it a try:-)

huerlisi added a commit to huerlisi/airbrussh that referenced this issue Jul 26, 2016
When using the `gray` color on some implementation of the `solarized`
theme the text is not visible. The `solarized` theme is popular and
bugs have been reported about missed error messages, so this patch
switches these messages to the default color.

Fixes mattbrictson#84
@huerlisi
Copy link
Contributor

PR created, please review:-)

@huerlisi
Copy link
Contributor

@mattbrictson upstream issue filed in KDE: https://bugs.kde.org/show_bug.cgi?id=366143

@dan-klasson maybe you want to also file an issue in Gnome and link this and the KDE bug.

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

No branches or pull requests

3 participants