[Webkit-unassigned] [Bug 106553] [V8] Add IDL Enum support WIP

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Nils Barth <nbarth at google.com>  2013-01-11 03:16:12 PST ---
(In reply to comment #1)
> (From update of attachment 182112 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182112&action=review
> 
> Please add a ChangeLog. You can generate a ChangeLog by ./Tools/Scripts/prepare-ChangeLog.

Ok, will do.

> > Source/WebCore/bindings/scripts/CodeGenerator.pm:56
> > +my %enumTypeHash = ();  # populated from IDL
> 
> WebKit is not likely to add such a non-helpful comment. Ditto for other parts. Please leave helpful comments only.

Got it -- the reason for this comment is that all the other hashes (for various types) are static lists like:
my %stringTypeHash = ("DOMString" => 1, "AtomicString" => 1);
...and only this one is (initially) empty, b/c we don't know what the Enum types are until we've parsed the IDL.

It's clearer in context (bunch of hashes, one blank one which looks weird, with this comment explaining why it's empty), and I was trying to keep it terse; perhaps different wording or a bit more detail would help?

> This patch also contains a lot of debug comments. I'd be happy if you could remove them for review.

Of course -- the debug code is there while I'm working on it (and for own reference), but I'll remove it before submitting for review.

> > Source/WebCore/bindings/scripts/IDLParser.pm:1
> > -# 
> > +#
> 
> Please do not edit unrelated code. Ditto for other style changes in this patch. Conventionally you are allowed to make style changes when you make some meaningful changes around it.

Ok, I was wondering about that; obviously actual patch shouldn't have unrelated style changes (I'll remove these before submitting actual patch), but I was wondering whether there was a place for style fixes.

(These blank-looking changes are removing trailing whitespace, though I see that's clear from the colorized diff.)

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