[webkit-reviews] review granted: [Bug 34402] Implement numeric CSS3 list-style-types : [Attachment 48291] Patch with test case

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 34402: Implement numeric CSS3 list-style-types
https://bugs.webkit.org/show_bug.cgi?id=34402

Attachment 48291: Patch with test case
https://bugs.webkit.org/attachment.cgi?id=48291&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +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 };

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.

> +    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.

> +	   numberShadow = -number;

I'm glad this works for MIN_INT, but I'm not sure how it works.

r=me