[Webkit-unassigned] [Bug 34402] Implement numeric CSS3 list-style-types

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Daniel Bates <dbates at webkit.org>  2010-02-07 11:55:47 PST ---
(In reply to comment #8)
> (From update of attachment 48291 [details])
> > +static inline String toAlphabeticOrNumeric(int number, const UChar* sequence, int sequenceSize, bool isAlphabeticSequence)
> 
> Elsewhere in WebKit code we've found that enums are easier to read than bools
> for arguments like this where the value passed is a constant.
> 
>     enum SequenceType { NumericSequence, AlphabeticSequence };

Added enum and modified code accordingly.

> 
> You can still name the argument isAlphabeticSequence and use boolean-style
> logic inside the function.
> 
> > +    ASSERT(sizeof(number) <= 4);
> 
> This could instead be a compile-time assert.

Removed assert.

> 
> > +    const int lettersSize = 32 + 1; // big enough to hold the ASCII representation of the largest negative 32-bit int in binary plus a minus sign.
> 
> Since this is the only place we are depending on the size of the number
> argument, then I think you we just use sizeof in this expression instead of
> using an assertion.
> 
>     const int lettersSize = sizeof(number) * 8 + 1; // Binary is the worst
> case; requires one character per bit plus a minus sign.
> 

Changed code to reflect this.

> > +        numberShadow = -number;
> 
> I'm glad this works for MIN_INT, but I'm not sure how it works.

By the definition of two's complement and unsigned int.

> r=me

Thank you for reviewing this patch.

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