[Webkit-unassigned] [Bug 27425] adding auto-generator support for GDOMHTMLElementWrapperFactory[.cpp/.h]

[bugzilla-daemon at webkit.org]

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


Mark Rowe (bdash) <mrowe at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34262|review+, commit-queue-      |review-
               Flag|                            |




--- Comment #38 from Mark Rowe (bdash) <mrowe at apple.com>  2009-08-12 13:53:25 PDT ---
(From update of attachment 34262)
> +#die "You must specify a --bindingType <type> e.g. Gdom or JS" unless (length($binding));
> +

We don't land commented-out code.

> +# FIXME: make_names is being utilised for six different purposes,
> +# with generation of HTMLNames.cpp/h being done at the same time
> +# as the JS bindings wrapper factories.

It's not clear how this comment relates to the code around it.

> +# FIXME: this is here because the gobject bindings were created in
> +# november 2008, when #ifdef DATAGRID did not exist.  to avoid
> +# significant extra work in getting the gobject bindings landed,
> +# and to also ensure that webkit-gtk will still compile even when
> +# DATAGRID is enabled, this workaround skips the #include
> +# of headers which the CodeGeneratorGObject.pm does not yet create.

Rather than adding a hack like this to the script we should just fix the
related issues.

> @@ -313,6 +342,8 @@
>  
>          $uniqueTags{$interfaceName} = '1';
>  
> +        next if (temporaryWorkaroundSkipTag($tagName));
> +

There are redundant braces here.

>          my $conditional = $tags{$tagName}{"conditional"};
>          if ($conditional) {
>              my $conditionalString = "ENABLE(" . join(") && ENABLE(", split(/&/, $conditional)) . ")";
> @@ -346,6 +377,8 @@
>  
>      for my $tagName (sort keys %tagConstructorMap) {
>  
> +        next if (temporaryWorkaroundSkipTag($tagName));
> +

And here.  And in other similar places.

> +sub printWrapperFunctionGdomInterface
> +{
> +    my $F = shift;
> +    my $JSInterfaceName = shift;

This seems like a poor choice of variable name.

> +            } 
> +            if ($binding eq 'Gdom') {
> +                printWrapperFunctionGdomInterface($F, $JSInterfaceName);
> +            }

As does this.

> +            if ($binding eq 'Gdom') {
> +                printWrapperFunctionGdomInterface($F, $JSInterfaceName);
> +            }
>          }


In general I don't think that a bunch of "if ($binding eq 'Foo')" is a good way
to structure this code.  Given the number of issues I've mentioned so far I'm
marking this as r-.

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