[Webkit-unassigned] [Bug 27147] JS Syntax Highlight is incorrect for keyword followed by string or regexp

[bugzilla-daemon at webkit.org]

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


Timothy Hatcher <timothy at hatcher.name> changed:

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




--- Comment #15 from Timothy Hatcher <timothy at hatcher.name>  2009-10-26 10:28:57 PDT ---
(From update of attachment 41842)

> +        DivAllowed: 2,

I assume this means division allowed. Spell it out in the constant.


> +    this.state = this.LexState.Initial;

This be this.lexState to match the constant names. Or the constant name should
just be this.State.


> +    }
> +    
> +    
> +    function singleQuoteStringStartAction(token)

Extra blank line here.


> +        var keywords = ["null", "true", "false", "break", "case", "catch", "const", "default", "finally", "for", "instanceof", "new", "var", "continue", "function", "return", "void", "delete", "if", "this", "do", "while", "else", "in", "switch", "throw", "try", "typeof", "with", "debugger", "class", "enum", "export", "extends", "import", "super", "get", "set"];

Use "const keywords" instead of "var keywords".


> +        if (keywords.indexOf(token) < 0) {

=== -1 is better here, since it is speced to return only -1. Or flip it to be
!== -1 if you want.


> +            var i;
> +            for (i = 0; i < tokensPerChunk; i++) {

Put the var i in the for loop:

for (var i = 0; …


> +                var messageBubble = line.lastChild;
> +                if (messageBubble && messageBubble.nodeType === Node.ELEMENT_NODE && messageBubble.hasStyleClass("webkit-html-message-bubble"))
> +                    line.removeChild(messageBubble);
> +                else
> +                    messageBubble = null;

I think it would be better to start messageBubble out as null and assign it
only after confirming the type and class on the lastChild.


> +                while (line.firstChild)
> +                    line.removeChild(line.firstChild);

We have a helper for this. Calling line.removeChildren() should do.


> +                this.lineFragment =null;

Missing a space after the "=".


> +    appendNonToken: function () {

Brace on the next line.


> +    syntaxHighlightNode: function(node) {

Ditto.


> +        while (1) {

Use "true".


> +        result.push(dumpSyntaxHighlight("return'foo';"));
> +        result.push(dumpSyntaxHighlight("/\\\//"));
> +        result.push(dumpSyntaxHighlight("//ig';"));
> +        result.push(dumpSyntaxHighlight("1 / 2 + /a/.test('a');"));
> +        result.push(dumpSyntaxHighlight("\"\\\"/\".length / 2"));
> +        result.push(dumpSyntaxHighlight("var foo = 1/****//2"));
> +        result.push(dumpSyntaxHighlight("/*comment*//.*/.test('a')"));

You should write some more tests that are multi-line and deal with things like
message bubbles and nodes directly.

Can be another patch.

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