Rewrite colors into shortest possible name

[nthykier]

trafficstars

Improve the code for rewriting colors into recognising that some colors are shorter by name. This commit enables scour to rewrite rgb(255, 0, 0) into red which is slightly shorter than #f00 (ditto for tan and pink).

When the color name ties in length with the hexcode, then scour will leave it as-is if the input file used a variant of same length (e.g. blue, cyan and aqua will be left as-is). But if scour is rewriting the color code, it will prefer the hex code variant.

Signed-off-by: Niels Thykier [email protected]

[Ede123]

Would this still create a group and propagate the fill to the parent for the following?

<rect fill="#f00" x="10" y="10" width="10" height="10"/>
<rect fill="red" x="20" y="20" width="10" height="10"/>

[nthykier]

Would this still create a group and propagate the fill to the parent for the following?

[...]

Neither the old nor the new code gets this consistently right because that is governed by another part of the code. For reference, the new code gets your particular example correct. However, I would expect both the old and the new code to fail that test with e.g. cyan vs aqua vs #0ff.

For that to (always) work, then we would also have to change:

            newColorValue = convertColor(oldColorValue)
            oldBytes = len(oldColorValue)
            newBytes = len(newColorValue)
            if oldBytes > newBytes:
                element.setAttribute(attr, newColorValue)
                numBytes += (oldBytes - len(element.getAttribute(attr)))

Notably, the if oldBytes > newBytes: which should be rewritten as if oldBytes >= newBytes and oldColorValue != newColorValue:

[Ede123]

Yep, you're right, as long as we convert consistently (even if it's not shorter but the same length) we should be fine optimization-wise. We should probably add a few testcases for these border cases as well...

Remaining questions I'm mulling about:

• The related preference is named --disable-simplify-colors. You already adjusted the description, but I figure the name is not really suitable anymore. Maybe we should consider renaming it (keeping the old name as fall-back as we did for other preferences), e.g. --disable-shorten-colors?
• I have to admit seeing RGB colors and color names mixed in a document makes my skin crawl. Is this just me? Objectively speaking going for the shortest name seems reasonable, so I probably should bite the bullet here, but I wanted to put it out there for consideration at least. 😉

[nthykier]

Ok, I have:

• Added --disable-shorten-colors as an alias of --disable-simplify-colors
• Made scour always normalize to the same variant of a color (except when we are not shorting colors).
• Fixed a bug in handling of upper case hex code writing two scour runs to be normalized to the shortest possible code.

[nthykier]

As for mixing color codes with color names - scour is here to write short svg files, not beautiful ones. :)

Also, if your beef with this is about consistency, then we are trading one consistency (form) for another (shortest length). :smile:

[eweitz]

As someone who independently developed a rough version of this, I'd find this functionality useful.

Could a Scour maintainer chime in to note what could move this PR forward?