lib/prawn/text.rb - consider deleting '#' characters

[ghost]

Hi,

Not sure if sandal considers this good or not - if not it can just be closed. Thanks.

Currently you can do this code:

text 'Ruby in Red', :color => 'FF0000'
text 'Ruby in Red', :color => '#FF0000'

The first variant works fine, and displays the text in red.

The second line displays the code in green (???)

Screenshot:

My proposals:
(a) Leading '#' characters will be ignored (this behaviour should be documented). The effect would be that the second line would be treated like the first line (unless perhaps there was a reason for this which I overlooked, but to me right now this seems more like a tiny bug)
(b) The green colour here, I am not sure if it makes sense. I assume that currently the '#' marks this as invalid? In that event, perhaps it should default to black? (e. g. black text on white background, by default)

[practicingruby]

Well, we certainly should do something about this, because that's obviously a bad behavior, and an easy mistake to make.

Unsure whether we should support # or not though. It is a little confusing that Prawn isn't like HTML here, but I don't know how I feel about supporting two different formats, even if they're nearly identical
to one another.

I'll give this some thought, thanks for reporting the issue!

[packetmonkey]

I would think simplest to understand solution would be to expect 6 hexadecimal characters, without the leading hash as the only option. If we support the leading # that could let the developer think other things more html-like are available, like the CSS shortcut of using 3 characters if they repeat (like using 'ABC' instead of 'AABBCC').

It's also really easy to test for and we can raise an error if we get passed an invalid format to help guide the developer.

[practicingruby]

@packetmonkey: Probably the right thing to do. Since the current behavior is undefined anyway, can you go ahead and write a patch that works as you suggest?

[packetmonkey]

Sure I'll get a PR for this pulled together.

[practicingruby]

@packetmonkey: Do you still want to work on this? Since it's a small patch, I could probably write it myself if you don't plan to do it in the next few days, so that we can include it in the 2.0.0 release.

[packetmonkey]

This totally dropped off my radar, I'll try to knock it out this weekend but I won't feel hurt if you beat me to it.

[practicingruby]

I'll drop a note on this ticket if I start working on this, otherwise go ahead and give it a shot.

[packetmonkey]

Going to assign the issue to myself so it shows up on my GH dashboard.

[practicingruby]

I realized that this isn't a release blocker because we'd just be turning an undefined behavior into an exception. So rather than rushing to build a fix before the 2.0.0 release (which may happen today or tomorrow), just fix this when you get around to it @packetmonkey.

[ghost]

Gregory, it is time to say it - I <3 your work on prawn. :) (There was an older "port", ruby-freepdf (fpdf) loosely based on the php-pdf, but it never was high quality code, very difficult to navigate and modify; prawn is much much better from a technical point of view)

[practicingruby]

Thanks @shevegen. Prawn is very much the work of dozens of contributors. Most of the good ideas aren't mine. :-P

[petergoldstein]

@pointlessone This issue should be closed, as a behavioral decision was made and merged as discussed in #869