Fixes to hover preview color RegExp #3540

redmunds · 2013-04-22T23:09:02Z

The RegExp is difficult to diff, so here are my changes. I made 3 changes:

For [hover preview] No color preview for rgba/hsla color format with 1.0 alpha. #3454, add "1.0" list of possible to alpha values for rgba()/hsla().
For [hover preview] No color preview for hsl/hsla format with hue values greater than 360. #3455 The hsl()/hsla() functions now accept integer up to 999 (degrees) for the first parameter. According to the spec (http://www.w3.org/TR/css3-color/#hsl-color), "As an angle, it implicitly wraps around...". It would be easy to add more digits, if desired.
I noticed that zero or one space chars was accepted for optional whitespace (" ?"), so I changed it to zero or more whitespace chars ("\s*") to be more forgiving (and accurate).

TomMalbran · 2013-04-23T02:22:19Z

src/extensions/default/HoverPreview/main.js

        // Check for color
-        var colorRegEx = /#[a-f0-9]{6}\b|#[a-f0-9]{3}\b|rgb\( ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?\)|rgb\( ?\b([0-9]{1,2}%|100%) ?, ?\b([0-9]{1,2}%|100%) ?, ?\b([0-9]{1,2}%|100%) ?\)|rgba\( ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?(1|0|0?\.[0-9]{1,3}) ?\)|rgba\( ?\b([0-9]{1,2}%|100%) ?, ?\b([0-9]{1,2}%|100%) ?, ?\b([0-9]{1,2}%|100%) ?, ?(1|0|0?\.[0-9]{1,3}) ?\)|hsl\( ?\b([0-9]{1,2}|[12][0-9]{2}|3[0-5][0-9]|360)\b ?, ?\b([0-9]{1,2}|100)\b% ?, ?\b([0-9]{1,2}|100)\b% ?\)|hsla\( ?\b([0-9]{1,2}|[12][0-9]{2}|3[0-5][0-9]|360)\b ?, ?\b([0-9]{1,2}|100)\b% ?, ?\b([0-9]{1,2}|100)\b% ?, ?(1|0|0?\.[0-9]{1,3}) ?\)|\baliceblue\b|\bantiquewhite\b|\baqua\b|\baquamarine\b|\bazure\b|\bbeige\b|\bbisque\b|\bblack\b|\bblanchedalmond\b|\bblue\b|\bblueviolet\b|\bbrown\b|\bburlywood\b|\bcadetblue\b|\bchartreuse\b|\bchocolate\b|\bcoral\b|\bcornflowerblue\b|\bcornsilk\b|\bcrimson\b|\bcyan\b|\bdarkblue\b|\bdarkcyan\b|\bdarkgoldenrod\b|\bdarkgray\b|\bdarkgreen\b|\bdarkgrey\b|\bdarkkhaki\b|\bdarkmagenta\b|\bdarkolivegreen\b|\bdarkorange\b|\bdarkorchid\b|\bdarkred\b|\bdarksalmon\b|\bdarkseagreen\b|\bdarkslateblue\b|\bdarkslategray\b|\bdarkslategrey\b|\bdarkturquoise\b|\bdarkviolet\b|\bdeeppink\b|\bdeepskyblue\b|\bdimgray\b|\bdimgrey\b|\bdodgerblue\b|\bfirebrick\b|\bfloralwhite\b|\bforestgreen\b|\bfuchsia\b|\bgainsboro\b|\bghostwhite\b|\bgold\b|\bgoldenrod\b|\bgray\b|\bgreen\b|\bgreenyellow\b|\bgrey\b|\bhoneydew\b|\bhotpink\b|\bindianred\b|\bindigo\b|\bivory\b|\bkhaki\b|\blavender\b|\blavenderblush\b|\blawngreen\b|\blemonchiffon\b|\blightblue\b|\blightcoral\b|\blightcyan\b|\blightgoldenrodyellow\b|\blightgray\b|\blightgreen\b|\blightgrey\b|\blightpink\b|\blightsalmon\b|\blightseagreen\b|\blightskyblue\b|\blightslategray\b|\blightslategrey\b|\blightsteelblue\b|\blightyellow\b|\blime\b|\blimegreen\b|\blinen\b|\bmagenta\b|\bmaroon\b|\bmediumaquamarine\b|\bmediumblue\b|\bmediumorchid\b|\bmediumpurple\b|\bmediumseagreen\b|\bmediumslateblue\b|\bmediumspringgreen\b|\bmediumturquoise\b|\bmediumvioletred\b|\bmidnightblue\b|\bmintcream\b|\bmistyrose\b|\bmoccasin\b|\bnavajowhite\b|\bnavy\b|\boldlace\b|\bolive\b|\bolivedrab\b|\borange\b|\borangered\b|\borchid\b|\bpalegoldenrod\b|\bpalegreen\b|\bpaleturquoise\b|\bpalevioletred\b|\bpapayawhip\b|\bpeachpuff\b|\bperu\b|\bpink\b|\bplum\b|\bpowderblue\b|\bpurple\b|\bred\b|\brosybrown\b|\broyalblue\b|\bsaddlebrown\b|\bsalmon\b|\bsandybrown\b|\bseagreen\b|\bseashell\b|\bsienna\b|\bsilver\b|\bskyblue\b|\bslateblue\b|\bslategray\b|\bslategrey\b|\bsnow\b|\bspringgreen\b|\bsteelblue\b|\btan\b|\bteal\b|\bthistle\b|\btomato\b|\bturquoise\b|\bviolet\b|\bwheat\b|\bwhite\b|\bwhitesmoke\b|\byellow\b|\byellowgreen\b/gi,
+        var colorRegEx = /#[a-f0-9]{6}\b|#[a-f0-9]{3}\b|rgb\(\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*\)|rgb\(\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*\)|rgba\(\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*(1|1\.0|0|0?\.[0-9]{1,3})\s*\)|rgba\(\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*,\s*(1|1\.0|0|0?\.[0-9]{1,3})\s*\)|hsl\(\s*\b([0-9]{1,3})\b\s*,\s*\b([0-9]{1,2}|100)\b%\s*,\s*\b([0-9]{1,2}|100)\b%\s*\)|hsla\(\s*\b([0-9]{1,3})\b\s*,\s*\b([0-9]{1,2}|100)\b%\s*,\s*\b([0-9]{1,2}|100)\b%\s*,\s*(1|1\.0|0|0?\.[0-9]{1,3})\s*\)|\baliceblue\b|\bantiquewhite\b|\baqua\b|\baquamarine\b|\bazure\b|\bbeige\b|\bbisque\b|\bblack\b|\bblanchedalmond\b|\bblue\b|\bblueviolet\b|\bbrown\b|\bburlywood\b|\bcadetblue\b|\bchartreuse\b|\bchocolate\b|\bcoral\b|\bcornflowerblue\b|\bcornsilk\b|\bcrimson\b|\bcyan\b|\bdarkblue\b|\bdarkcyan\b|\bdarkgoldenrod\b|\bdarkgray\b|\bdarkgreen\b|\bdarkgrey\b|\bdarkkhaki\b|\bdarkmagenta\b|\bdarkolivegreen\b|\bdarkorange\b|\bdarkorchid\b|\bdarkred\b|\bdarksalmon\b|\bdarkseagreen\b|\bdarkslateblue\b|\bdarkslategray\b|\bdarkslategrey\b|\bdarkturquoise\b|\bdarkviolet\b|\bdeeppink\b|\bdeepskyblue\b|\bdimgray\b|\bdimgrey\b|\bdodgerblue\b|\bfirebrick\b|\bfloralwhite\b|\bforestgreen\b|\bfuchsia\b|\bgainsboro\b|\bghostwhite\b|\bgold\b|\bgoldenrod\b|\bgray\b|\bgreen\b|\bgreenyellow\b|\bgrey\b|\bhoneydew\b|\bhotpink\b|\bindianred\b|\bindigo\b|\bivory\b|\bkhaki\b|\blavender\b|\blavenderblush\b|\blawngreen\b|\blemonchiffon\b|\blightblue\b|\blightcoral\b|\blightcyan\b|\blightgoldenrodyellow\b|\blightgray\b|\blightgreen\b|\blightgrey\b|\blightpink\b|\blightsalmon\b|\blightseagreen\b|\blightskyblue\b|\blightslategray\b|\blightslategrey\b|\blightsteelblue\b|\blightyellow\b|\blime\b|\blimegreen\b|\blinen\b|\bmagenta\b|\bmaroon\b|\bmediumaquamarine\b|\bmediumblue\b|\bmediumorchid\b|\bmediumpurple\b|\bmediumseagreen\b|\bmediumslateblue\b|\bmediumspringgreen\b|\bmediumturquoise\b|\bmediumvioletred\b|\bmidnightblue\b|\bmintcream\b|\bmistyrose\b|\bmoccasin\b|\bnavajowhite\b|\bnavy\b|\boldlace\b|\bolive\b|\bolivedrab\b|\borange\b|\borangered\b|\borchid\b|\bpalegoldenrod\b|\bpalegreen\b|\bpaleturquoise\b|\bpalevioletred\b|\bpapayawhip\b|\bpeachpuff\b|\bperu\b|\bpink\b|\bplum\b|\bpowderblue\b|\bpurple\b|\bred\b|\brosybrown\b|\broyalblue\b|\bsaddlebrown\b|\bsalmon\b|\bsandybrown\b|\bseagreen\b|\bseashell\b|\bsienna\b|\bsilver\b|\bskyblue\b|\bslateblue\b|\bslategray\b|\bslategrey\b|\bsnow\b|\bspringgreen\b|\bsteelblue\b|\btan\b|\bteal\b|\bthistle\b|\btomato\b|\bturquoise\b|\bviolet\b|\bwheat\b|\bwhite\b|\bwhitesmoke\b|\byellow\b|\byellowgreen\b/gi,


To make this long regular expressions easy to read, it would be possible to split them at the "|" separators when appropriate, crating an array of string, and then creating the regular expression by joining them.

var colorRegPts = [ "#[a-f0-9]{6}\b|#[a-f0-9]{3}\b", "rgb\(\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*\)", "rgb\(\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*\)", "rgba\(\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*(1|1\.0|0|0?\.[0-9]{1,3})\s*\)", "rgba\(\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*,\s*\b([0-9]{1,2}%|100%)\s*,\s*(1|1\.0|0|0?\.[0-9]{1,3})\s*\)", "hsl\(\s*\b([0-9]{1,3})\b\s*,\s*\b([0-9]{1,2}|100)\b%\s*,\s*\b([0-9]{1,2}|100)\b%\s*\)", "hsla\(\s*\b([0-9]{1,3})\b\s*,\s*\b([0-9]{1,2}|100)\b%\s*,\s*\b([0-9]{1,2}|100)\b%\s*,\s*(1|1\.0|0|0?\.[0-9]{1,3})\s*\)", "\baliceblue\b|\bantiquewhite\b|\baqua\b|\baquamarine\b|\bazure\b|\bbeige\b|\bbisque\b|\bblack\b|\bblanchedalmond\b", "\bblue\b|\bblueviolet\b|\bbrown\b|\bburlywood\b|\bcadetblue\b|\bchartreuse\b|\bchocolate\b|\bcoral\b|\bcornflowerblue\b", "\bcornsilk\b|\bcrimson\b|\bcyan\b|\bdarkblue\b|\bdarkcyan\b|\bdarkgoldenrod\b|\bdarkgray\b|\bdarkgreen\b|\bdarkgrey\b", "\bdarkkhaki\b|\bdarkmagenta\b|\bdarkolivegreen\b|\bdarkorange\b|\bdarkorchid\b|\bdarkred\b|\bdarksalmon\b|\bdarkseagreen\b", "\bdarkslateblue\b|\bdarkslategray\b|\bdarkslategrey\b|\bdarkturquoise\b|\bdarkviolet\b|\bdeeppink\b|\bdeepskyblue\b|\bdimgray\b", "\bdimgrey\b|\bdodgerblue\b|\bfirebrick\b|\bfloralwhite\b|\bforestgreen\b|\bfuchsia\b|\bgainsboro\b|\bghostwhite\b|\bgold\b", "\bgoldenrod\b|\bgray\b|\bgreen\b|\bgreenyellow\b|\bgrey\b|\bhoneydew\b|\bhotpink\b|\bindianred\b|\bindigo\b|\bivory\b|\bkhaki\b", "\blavender\b|\blavenderblush\b|\blawngreen\b|\blemonchiffon\b|\blightblue\b|\blightcoral\b|\blightcyan\b|\blightgoldenrodyellow\b", "\blightgray\b|\blightgreen\b|\blightgrey\b|\blightpink\b|\blightsalmon\b|\blightseagreen\b|\blightskyblue\b|\blightslategray\b", "\blightslategrey\b|\blightsteelblue\b|\blightyellow\b|\blime\b|\blimegreen\b|\blinen\b|\bmagenta\b|\bmaroon\b", "\bmediumaquamarine\b|\bmediumblue\b|\bmediumorchid\b|\bmediumpurple\b|\bmediumseagreen\b|\bmediumslateblue\b", "\bmediumspringgreen\b|\bmediumturquoise\b|\bmediumvioletred\b|\bmidnightblue\b|\bmintcream\b|\bmistyrose\b|\bmoccasin\b", "\bnavajowhite\b|\bnavy\b|\boldlace\b|\bolive\b|\bolivedrab\b|\borange\b|\borangered\b|\borchid\b|\bpalegoldenrod\b|\bpalegreen\b", "\bpaleturquoise\b|\bpalevioletred\b|\bpapayawhip\b|\bpeachpuff\b|\bperu\b|\bpink\b|\bplum\b|\bpowderblue\b|\bpurple\b|\bred\b", "\brosybrown\b|\broyalblue\b|\bsaddlebrown\b|\bsalmon\b|\bsandybrown\b|\bseagreen\b|\bseashell\b|\bsienna\b|\bsilver\b", "\bskyblue\b|\bslateblue\b|\bslategray\b|\bslategrey\b|\bsnow\b|\bspringgreen\b|\bsteelblue\b|\btan\b|\bteal\b|\bthistle\b", "\btomato\b|\bturquoise\b|\bviolet\b|\bwheat\b|\bwhite\b|\bwhitesmoke\b|\byellow\b|\byellowgreen\b" ], colorRegEx = new RegExp(colorRegPts.join("|"), "gi");

It could also be a big separated string.

You'd have to double escape everything then, though, which makes is harder to read (e.g. rgb\\(\\s*\\b(...). This sort of inability to modularize cleanly is one of my pet peeves about regexps :-(

But if the "|"s are at the top level -- which I believe they are in this case -- then you can have an array of actual RegExp objects, and iterate over the array calling exec() on each one in turn. I'm guessing that wouldn't be appreciably less efficient than glomming them all together with "|"...

I haven't thought about the double escapes, and it does makes it harder to read. But I like the array of RegExp idea. In this case I cut it at the first level "|", since with that cut it makes it easier to read. If it works, the same idea could be applied to the Inline Color Editor.

We can avoid double escaping by using StringUtils.regexEscape(). It would be nice to split this regex into multiple lines.

This regex should be moved into a core file so it can be shared with the InlineColorEditor. All of the bugs fixed here apply to the Inline Color Editor as well.

I'm confused about how StringUtils.regexEscape() could help -- doesn't it do the opposite of what we need here? Taking a regular string and making it safe to embed in a regexp without special meaning, vs. taking a regexp and making it safe to store in a plain string without special meaning. (For example \b in a string literal turns into ASCII "BEL" or some such, and regexEscape() doesn't do anything to transform that back into the chars \ and b that we'd want to pass the RegExp constructor).

But I still think splitting the top-level |s into an array of separate regexp literals, then looping over them to find a match, should work ok.

Ah, right, StringUtils.regexEscape() won't work here...

There are a couple minor downsides to splitting into multiple regexp literals:

This makes it harder to share the regex with Inline Color Editor.

The code is already looping over a line to detect which color the cursor is over. Now it will need to additionally loop over each regexp.

This is basically trading off the complexity of maintaining an unwieldy regexp with the simplicity of using a single regexp. Since we only need to get the regexp right once, but may end up using it in multiple places, I have a slight preference for the simpler use (and huge regexp), but I don't feel too strongly about it.

gruehle · 2013-04-23T19:47:18Z

Initial review complete. It would be nice to split the regex into multiple lines and move into a core file so it can be shared with InlineColorEditor.

redmunds · 2013-04-23T22:44:25Z

@gruehle The color regex used by the InlineColorEditor does not have teh named colors. Is it OK if it does, or will that cause problems?

RaymondLim · 2013-04-23T23:07:37Z

I do have color regex in my branch rlim/improved-color-picker which construct a named color array from tinycolor library.

    InlineColorEditor.colorNameArray = $.map(tinycolor.names, function (value, key) {
        return "\\b" + key + "\\b";
    });

gruehle · 2013-04-23T23:07:54Z

The

gruehle · 2013-04-23T23:08:17Z

Whoops, accidentally closed. Sorry about that!

gruehle · 2013-04-24T00:13:25Z

@redmunds - it looks like some changes to StringUtils got added with your last commit. These are causing the Travis failure.

TomMalbran · 2013-04-24T00:18:54Z

The color regular expression was moved to the StringUtils as an array, but without the double escapes, which caused the JSHint error.

redmunds · 2013-04-24T02:13:47Z

I tried to break the color regex into an array of regexes, but I couldn't quite get it working, so I punted for now.

I implemented a small performance improvement: don't look for a color if we already have a gradient.

I fixed a couple more problems with color regex:

make functions start on word boundary
remove unnecessary word boundary check from numbers

Copied changes to InlineColorEditor color regex (everything except named colors).

Ready for final review.

gruehle · 2013-04-24T02:39:36Z

Thanks for trying the regexp split. Too bad it didn't work out.

Everything else looks good.

Fixes to hover preview color RegExp

fixes to color RegExp

b042a5a

TomMalbran reviewed Apr 23, 2013
View reviewed changes

ghost assigned gruehle Apr 23, 2013

gruehle closed this Apr 23, 2013

gruehle reopened this Apr 23, 2013

merge with master

38bb465

peterflynn mentioned this pull request Apr 24, 2013

Hide popover when doc is edited or changes #3538

Merged

redmunds added 3 commits April 23, 2013 18:55

backout changes to StringUtils

9327c8c

don't look for color if already found a gradient

82ccb41

more color regex fixes

2788980

gruehle added a commit that referenced this pull request Apr 24, 2013

Merge pull request #3540 from adobe/randy/issue-3454

e16779b

Fixes to hover preview color RegExp

gruehle merged commit e16779b into master Apr 24, 2013

gruehle deleted the randy/issue-3454 branch April 24, 2013 02:39

core-ai-bot mentioned this pull request Aug 29, 2021

[CLOSED] Fixes to hover preview color RegExp brackets-archive/bracketsIssues#3325

Open

aliceUnhinged613 mentioned this pull request Nov 24, 2023

[Snyk] Security upgrade less from 2.7.2 to 3.12.0 aliceUnhinged613/brackets#51

Open

qmutz mentioned this pull request Nov 29, 2023

[Snyk] Security upgrade less from 2.7.2 to 3.12.0 inteI-cloud/brackets#8

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes to hover preview color RegExp #3540

Fixes to hover preview color RegExp #3540

redmunds commented Apr 22, 2013

TomMalbran Apr 23, 2013

peterflynn Apr 23, 2013

peterflynn Apr 23, 2013

TomMalbran Apr 23, 2013

gruehle Apr 23, 2013

peterflynn Apr 23, 2013

gruehle Apr 23, 2013

gruehle commented Apr 23, 2013

redmunds commented Apr 23, 2013

RaymondLim commented Apr 23, 2013

gruehle commented Apr 23, 2013

gruehle commented Apr 23, 2013

gruehle commented Apr 24, 2013

TomMalbran commented Apr 24, 2013

redmunds commented Apr 24, 2013

gruehle commented Apr 24, 2013

Fixes to hover preview color RegExp #3540

Fixes to hover preview color RegExp #3540

Conversation

redmunds commented Apr 22, 2013

TomMalbran Apr 23, 2013

Choose a reason for hiding this comment

peterflynn Apr 23, 2013

Choose a reason for hiding this comment

peterflynn Apr 23, 2013

Choose a reason for hiding this comment

TomMalbran Apr 23, 2013

Choose a reason for hiding this comment

gruehle Apr 23, 2013

Choose a reason for hiding this comment

peterflynn Apr 23, 2013

Choose a reason for hiding this comment

gruehle Apr 23, 2013

Choose a reason for hiding this comment

gruehle commented Apr 23, 2013

redmunds commented Apr 23, 2013

RaymondLim commented Apr 23, 2013

gruehle commented Apr 23, 2013

gruehle commented Apr 23, 2013

gruehle commented Apr 24, 2013

TomMalbran commented Apr 24, 2013

redmunds commented Apr 24, 2013

gruehle commented Apr 24, 2013