[Webkit-unassigned] [Bug 27431] splitting CodeGeneratorGObject.pm into library files

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Luke Kenneth Casson Leighton <lkcl at lkcl.net>  2009-08-07 13:11:55 PDT ---
(In reply to comment #4)
> (From update of attachment 34289 [details])
> This looks like a big hack.

 yep!  you got that right :)

  it's pieced together from _both_ the CodeGeneratorJS.pm _and_ the
CodeGeneratorObjC.pm, and is the only major piece of perl programming i've ever
forced myself to work on.  i'm deeply proud of both how much of a dog's dinner
it is and that it actually works at all. *beam* :)


>  If we're going to do this, lets do it right.  No
> commented out code.  No long lists of exclusions.  Nicely named methods which
> actually follow WebKit style guidlines.  Small files that reviewers have a
> chance of understanding.  Detailed ChangeLogs.  Use of sets/hashes instead of
> long lists of copy/paste ifs.  

 .... not being funny or anything, but... seriously, take a look at the code
from which this was cut/paste copied (verbatim in some cases).  i'm just
looking through it, now, but some of the stuff that _was_ there has gone.

* UsesManualToGDOMImplementation came from the august 2008 version of
CodeGeneratorObjC.pm.  it's gone, now.

* likewise isGDOMSubClass - that was cut/paste verbatim conceptually from the
other CodeGenerators - but they're gone, now, too.

* see addIncludesForType in CodeGeneratorObjC.pm - that concept is still there,
you can see it shining through in addIncHdr and addPropIncHdr.

* look at the code-density of lines 1082 to 1206 in CodeGeneratorJS.pm!  

* 1325 to 1334 in CodeGeneratorJS.pm i'm particularly fond of :)  makes me feel
like i'm 5 years old again, just slide down the brackets, wheeeee :)

* GetNativeType is the _new_ version of what i originally cut/paste from
CodeGeneratorJS.pm, to create GetGType() and GetPropGType().



so, what you're seeing now is what the code _used_ to look like back in august
2008, when this work was in active development, when an intelligent
non-perl-programmer gets their hands on it and just goes, "err, where's
something i can blatantly copy that will get me from a to b?  oh look -
_there_!"

so - _fortunately_, there is yet more examples for me to blatantly
cut-and-paste (GetNativeType)








> As is, either you're just looking for a rubber-stamp.  Or you don't realize how
> much patches like this are just a waste of time.

 not quiiite true - and not quite fair, either.  do take a look at the older
versions of CodeGeneratorJS.pm and CodeGeneratorObjC.pm:


http://svn.webkit.org/repository/webkit/trunk/WebCore/bindings/scripts/CodeGeneratorJS.pm?r=26579

http://svn.webkit.org/repository/webkit/trunk/WebCore/bindings/scripts/CodeGeneratorObjC.pm?r=26579

take a look at TypeCanFailConversion, GetNativeType,
UsesManualToJSImplementation and you'll soon see that i'm really _not_ taking
the piss, i was just working from what was there at the time.


> Glad to have you contributing. 

 *beam* :)

> But if you're going to contribute to a product
> as mature as WebKit, you need to expect to take a bit longer on your patches,
> and write mature, well structured patches instead of hacks.

 eric - you're going to be amused re-reading this after looking at the older
versions i was working from, at the time, i guarantee it.


> Contributing to a large project like WebKit is a lot of work. :)

 mmmm :)

 ok.

 rrrright.

 i'll start with some splits into sensible / reasonable sizes, and make some
more cut/paste copies, from what's already there.

 but - PLEASE do not expect me to make "functionality" changes / logic changes.
 i've validated this code, hair-raisingly nasty as it is, by using
pyjamas-desktop's 20 examples / samples, so i know that it works.

 i reaaallly don't want to be making additions / changes whilst also at the
same going through a massive restructuring.

 if it wasn't for the massive amount of in-built logic and decisions, yes i'd
do a from-scratch rewrite.  but, like the linux kernel floppy code, there's
simply too much that's been taken into account to go throwing it away and
starting again.

 i'm happy to make it at least _look_ nicer, though :)

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