[webkit-reviews] review granted: [Bug 65046] Add parsing support for extended attributes on IDL constants : [Attachment 101766] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 65046: Add parsing support for extended attributes on IDL constants
https://bugs.webkit.org/show_bug.cgi?id=65046

Attachment 101766: Patch
https://bugs.webkit.org/attachment.cgi?id=101766&action=review

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=101766&action=review


Looks good. r+, but please consider my suggestions.

> Source/WebCore/bindings/scripts/IDLParser.pm:332
> +		   my $constExtendedAttributes = (defined($1) ? $1 : " ");
chop($constExtendedAttributes);

I'm not a fan of having two bits of logic on a single line like this.  I'd
rather see the 'chop' on its own line.

> Source/WebCore/bindings/scripts/IDLStructure.pm:97
> +our $constantSelector = 'const\s*(' . $extendedAttributeSyntax . ' )?' .
$supportedTypes . '\s*(' . $idlType . '*)\s*=\s*(' . $constValue . ')';

Does adding this new elements to the IDL structure require lots of the existing
IDL to be changed?  I think the answer is no (based on the EWS), but I just
wanted to double-check.