Allow 6-digit color spec w/o # #1690
Conversation
src/Parser.js
Outdated
| // If not predefined, allow a 6-digit color specification without | ||
| // a leading #, as in the xcolor package's HTML color model. | ||
| const colorWordRegEx = RegExp("^(silver|maroon|purple|yellow|orange" + | ||
| "|bisque|indigo|orchid|salmon|sienna|tomato|violet)$",'i'); |
There was a problem hiding this comment.
Since we're passing through strings whose length aren't 6 maybe we check that color matches a hex 6-tuple that way we're not prefix things like cherry. What's LaTeX's behaviour if it gets a named color that isn't defined? Also, you said this PR was minimal so may we can deal with those situations in subsequent PR.
There was a problem hiding this comment.
Every supported 6-letter color names seem to be out of hexadecimal digits range ([0-9a-f]). I think it'd be better to test for [0-9a-f]{6}. Also, why not support 3-digit color codes?
There was a problem hiding this comment.
why not support 3-digit color codes?
- 3-digit codes preceded by
#are already supported by KaTeX. No change there. - 3-digit codes not preceded by
#are not supported by thexcolorpackage. Since I'm trying to nudge this a little closer toxcolor, I thought it best to fall in line with this behavior.
There was a problem hiding this comment.
What's LaTeX's behaviour if it gets a named color that isn't defined?
There is an error message such as: !Package xcolor Error: Undefined color 'cherry'. quicklatex.com will still render a black expression if the Report LaTeX errors box is unchecked. So the error behavior is somewhat similar to KaTeX. That is, KaTeX also will render a black expression.
maybe we check that color matches a hex 6-tuple
I'll see if that can be done fairly easily.
Codecov Report
@@ Coverage Diff @@
## master #1690 +/- ##
==========================================
+ Coverage 93.84% 93.85% +<.01%
==========================================
Files 78 78
Lines 4568 4571 +3
Branches 801 802 +1
==========================================
+ Hits 4287 4290 +3
Misses 248 248
Partials 33 33
Continue to review full report at Codecov.
|
|
The latest commit revises the RegEx pattern as recommended by @ylemkimon. (Thanks!) This simplifies the code but it does not raise a parse error on a mis-typed predefined color name. I'd like to raise an error on a mis-typed predefined color name, but the only way I can think to accomplish that is via a RegEx pattern that enumerates all 150 of the HTML predefined colors. Is that something I should do? |
src/Parser.js
Outdated
| return newArgument({ | ||
| type: "color-token", | ||
| mode: this.mode, | ||
| color: match[0], | ||
| color: color, |
I think it is impossible without access to DOM APIs and there is no need to check the color name is valid. |
|
Just wanted to come in here and say thanks for this! |
This PR takes a minimalist approach to addressing issue #1657. This does not completely change the color specification syntax to align with the
colorpackage or thexcolorpackage. It does not accommodate RGB notation or a cmyk color model. It does, however, allow a 6-digit HTML color specification to pass without a leading#symbol, as inxcolor's HTML color model.