ku1ik / rainbow

Ruby gem for colorizing printed text on ANSI terminals
MIT License
813 stars 68 forks source link

ArgumentError when using color("red") #82

Closed crypt1d closed 2 years ago

crypt1d commented 6 years ago

Specifying red color by name seems to fail, while it works for other colors. It also works if color name is specified as a symbol, eg :red

To reproduce

2.4.1 :001 > require 'rainbow'
 => true 
2.4.1 :002 > Rainbow("this is a test message").red
 => "\e[31mthis is a test message\e[0m" 
2.4.1 :003 > Rainbow("this is a test message").color(:red)
 => "\e[31mthis is a test message\e[0m" 
2.4.1 :004 > Rainbow("this is a test message").color("red")
ArgumentError: wrong number of arguments (given 1, expected 0)
    from /usr/local/rvm/gems/ruby-2.4.1/gems/rainbow-3.0.0/lib/rainbow/color.rb:42:in `to_i'
    from /usr/local/rvm/gems/ruby-2.4.1/gems/rainbow-3.0.0/lib/rainbow/color.rb:42:in `parse_hex_color'
    from /usr/local/rvm/gems/ruby-2.4.1/gems/rainbow-3.0.0/lib/rainbow/color.rb:34:in `build'
    from /usr/local/rvm/gems/ruby-2.4.1/gems/rainbow-3.0.0/lib/rainbow/presenter.rb:20:in `color'
    from (irb):4
    from /usr/local/rvm/rubies/ruby-2.4.1/bin/irb:11:in `<main>'
2.4.1 :005 > Rainbow("this is a test message").color("green")
 => "\e[38;5;46mthis is a test message\e[0m" 
2.4.1 :006 > 

Ruby: 2.4.1 rubygems: 2.6.14 rainbow gem: 3.0.0 OS: CentOS 7.4

olleolleolle commented 6 years ago

Looking at the code, I find it weird that the "green" worked, since only the Symbol code path looks for Named colors.

ku1ik commented 6 years ago

That's really weird indeed 🤔

chiting commented 6 years ago

Hi,

From what I've read in the code, "green" decomposed in RGB would be gr,ee,n when translated to hexadecimal would be 0,238,0 as to_i would return 0 for non valid numbers.

The fact that "green" actually produces a green (not as green as the actual green output which gives "\e[32mthis is a test message\e[0m") output is just pure luck 😄

As for "red", it errors because the B of RGB would be nil and nil.to_i doesn't accept any parameters.

Maybe Rainbow::Color.parse_hex_color should check its argument is an hexadecimal string with a length of 6 characters after removing the potential # prefix, if it makes sense?

ku1ik commented 6 years ago

@chiting wow, that's an awesome detective work 👍

Indeed it would make sense to validate the string, maybe with a regexp like /^#?[a-f0-9]{6}/i or similar, and raise an exception if it doesn't match.

passbe commented 5 years ago

Would it be possible to release a new gem version with this fix? Breaking a few projects.

chiting commented 5 years ago

I’m split between recommending bumping the patch version and the major version for this fix. Although this is a bug fix, it has a breaking change that might have gone un-noticed and not really impacting the code that uses that gem.

olleolleolle commented 2 years ago

https://rubygems.org/gems/rainbow/versions/3.1.0 has been released. @chiting thanks for making a PR!