[Webkit-unassigned] [Bug 30907] Web Inspector: Rewrite CSSSourceSyntaxHighlighter so it shares more code

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30907


Timothy Hatcher <timothy at hatcher.name> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #42290|review?                     |review-
               Flag|                            |




--- Comment #3 from Timothy Hatcher <timothy at hatcher.name>  2009-11-01 19:27:31 PDT ---
(From update of attachment 42290)
> +    const urlPattern = /^url\(\s*(?:(?:"(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*"|'(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*')|(?:[!#$%&*-~]|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*)\s*\)/i;
> +    const stringPattern = /^(?:"(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*"|'(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*')/i;
> +    const identPattern = /^-?(?:[_a-zA-Z]|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))(?:[_a-zA-Z0-9-]|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*/i;

The only thing I think needs fixed about this patch is the regexs. They are
super complex.

There are a number of places than can be simplified. Like: [ \n\r\t\f] should
just be \s.

There is also a mistake with \\n being used when it should be \n.

Also [\200-\377] is odd. I know it is fro mthe spec, but the spec also says:

> The "\377" represents the highest character number that current versions of
> Flex can deal with (decimal 255). It should be read as "\4177777" (decimal
> 1114111), which is the highest possible code point in Unicode/ISO-10646.

We can support full unicode.

The multiple character rnages ORed together should just be combined inot one
character range. Like the follow example.

> [_a-zA-Z0-9-]|[\200-\377]

So you would have:

[-_a-zA-Z0-9\200-\4177777]

I wonder if we should just use \w and call it a day?


This \ should be \\.

\[0-9a-fA-F]{1,6}

It is really hard to review this. These are just the things that stood out to
me. I recomend we just write simple regexs and not try to generate them from
the spec's FLEX tokenizer.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.