[Webkit-unassigned] [Bug 70107] Custom written CSS lexer

[bugzilla-daemon at webkit.org]

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


Oliver Hunt <oliver at apple.com> changed:

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




--- Comment #43 from Oliver Hunt <oliver at apple.com>  2012-01-06 13:36:21 PST ---
(From update of attachment 121239)
View in context: https://bugs.webkit.org/attachment.cgi?id=121239&action=review

Okay, i'm doing a preliminary r- due to the large number of unnecessary formatting changes sorry.  A patch of this scale and complexity shouldn't have unnecessary no-op changes in it :-/

I'll continue with actual content review, but this should let you start working on the formatting stuff without waiting for me to finish this review

> Source/WebCore/css/CSSParser.cpp:263
> -    int length = string.length() + strlen(prefix) + strlen(suffix) + 2;
> +    int length = string.length() + strlen(prefix) + strlen(suffix) + 1;

Why have we dropped to only a single extra null terminator?  I recall (though haven't got that far in the patch) that we rely on two nulls at the end of the string

> Source/WebCore/css/CSSParser.cpp:925
> -        if (id == CSSValueStatic ||
> -             id == CSSValueRelative ||
> -             id == CSSValueAbsolute ||
> -             id == CSSValueFixed)
> +        if (id == CSSValueStatic
> +            || id == CSSValueRelative
> +            || id == CSSValueAbsolute
> +            || id == CSSValueFixed)

This change seems unnecessary formatting which i'd rather avoid in a patch of this size and complexity

> Source/WebCore/css/CSSParser.cpp:939
> -        if (id == CSSValueAuto ||
> -             id == CSSValueAlways ||
> -             id == CSSValueAvoid ||
> -             id == CSSValueLeft ||
> -             id == CSSValueRight)
> +        if (id == CSSValueAuto
> +            || id == CSSValueAlways
> +            || id == CSSValueAvoid
> +            || id == CSSValueLeft
> +            || id == CSSValueRight)

ditto

> Source/WebCore/css/CSSParser.cpp:952
> -        if (id == CSSValueShow ||
> -             id == CSSValueHide)
> +        if (id == CSSValueShow
> +            || id == CSSValueHide)

ditto

> Source/WebCore/css/CSSParser.cpp:965
> -        if (id == CSSValueNormal ||
> -            id == CSSValuePre ||
> -            id == CSSValuePreWrap ||
> -            id == CSSValuePreLine ||
> -            id == CSSValueNowrap)
> +        if (id == CSSValueNormal
> +            || id == CSSValuePre
> +            || id == CSSValuePreWrap
> +            || id == CSSValuePreLine
> +            || id == CSSValueNowrap)

ditto

>> Source/WebCore/css/CSSParser.cpp:1424
>> -     case CSSPropertyCounterReset:        // [ <identifier> <integer>? ]+ | none | inherit
>> +    case CSSPropertyCounterReset:        // [ <identifier> <integer>? ]+ | none | inherit
> 
> One space before end of line comments  [whitespace/comments] [5]

ditto

> Source/WebCore/css/CSSParser.cpp:2025
> -             validPrimitive = true;
> +            validPrimitive = true;

ditto

> Source/WebCore/css/CSSParser.cpp:2894
> -       return cssValuePool()->createIdentifierValue(id);
> +        return cssValuePool()->createIdentifierValue(id);

ditto

> Source/WebCore/css/CSSParser.cpp:5186
> -                              // it is a color, but a color isn't allowed at this point.
> +                          // it is a color, but a color isn't allowed at this point.

ditto

> Source/WebCore/css/CSSParser.cpp:5474
> -     return id == CSSValueStretch || id == CSSValueRepeat || id == CSSValueSpace || id == CSSValueRound;
> +    return id == CSSValueStretch || id == CSSValueRepeat || id == CSSValueSpace || id == CSSValueRound;

ditto

> Source/WebCore/css/CSSParser.cpp:5559
> -             m_left = m_cssValuePool->createValue(m_right->getDoubleValue(), (CSSPrimitiveValue::UnitTypes)m_right->primitiveType());
> +            m_left = m_cssValuePool->createValue(m_right->getDoubleValue(), (CSSPrimitiveValue::UnitTypes)m_right->primitiveType());

ditto

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