Bug 19200 - *.in files should embed more information
Summary: *.in files should embed more information
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-22 14:27 PDT by Julien Chaffraix
Modified: 2008-08-12 17:22 PDT (History)
0 users

See Also:


Attachments
Proof of concept (2.38 KB, text/plain)
2008-05-25 11:54 PDT, Julien Chaffraix
no flags Details
First patch (using XML::Tiny) (47.49 KB, patch)
2008-05-27 19:34 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Second part - add XML attributes' parsing and start moving some hacks (11.16 KB, patch)
2008-06-07 02:15 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Third part - add a parameters hash and move some command line options to the *.in files (22.22 KB, patch)
2008-06-07 06:18 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Fourth part: move more parameters (9.85 KB, patch)
2008-06-09 07:47 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Final patch - Move more parameters in the XML files - Clean up the build systems (21.17 KB, patch)
2008-06-10 17:24 PDT, Julien Chaffraix
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 2008-05-22 14:27:42 PDT
Now that bug9191 is resolved, it would be nice to have some parameters/data in make_names.pl moved to the *.in files.
As pointed in the bug, XML seems like a good choice.
Comment 1 Julien Chaffraix 2008-05-25 11:54:13 PDT
Created attachment 21337 [details]
Proof of concept

Attached is a sample of what I intend to do.

make_names.pl will discriminate the files according to the first element ("tags" or "attrs").
It will use the attribute/value pair from that first element to build a parameter map that will be used for code generation.
The different tags/attrs ("a", ...) follow and contain the different properties that were once in make_names.pl.

In the end, make_names.pl will only accept a limited number of parameters that cannot be included in the XML files.
Internally, we will move from arrays for attrs/tags to hashes of hashes so that each tags/attrs holds the full list of properties (this is a bit of an overkill but it will greatly simply extensibility).
Comment 2 Julien Chaffraix 2008-05-27 05:22:57 PDT
We will also need to introduce a dependency on some Perl module to parse XML using SAX.

We have several choices:
- XML::Parser (or XML::Parser::Expat), which requires the expat library
- XML::libXML which will introduce a library dependency on the Qt port.
- ...

I am no Perl expert but those 2 seem the best available solutions. I have no preference as they works the same so the code will be roughly the same in each case.
Comment 3 Julien Chaffraix 2008-05-27 19:32:15 PDT
> We have several choices:
> - XML::Parser (or XML::Parser::Expat), which requires the expat library
> - XML::libXML which will introduce a library dependency on the Qt port.
> - ...

As discussed on IRC, those module includes new dependencies (on the library or the Perl module that needs to be compiled) so are not good candidates for our needs.

Furthermore, Sam pointed that other formats exist such as json, yaml, plist...

I had a look at the different formats and IMHO XML has the best zero-dependency Perl module (XML::Tiny).
Comment 4 Julien Chaffraix 2008-05-27 19:34:27 PDT
Created attachment 21380 [details]
First patch (using XML::Tiny)
Comment 5 Eric Seidel (no email) 2008-06-06 11:10:43 PDT
Comment on attachment 21380 [details]
First patch (using XML::Tiny)

So this kinda looks like a work-in-progress to me.  I'm OK with landing this as-is, but I'd like to know more about the long-term plan for the syntax of this file.  It's kinda strange to have the xml tag name be the element name.  I guess I would have expected something more like:
<tag>a</tag>
instead of
<a/>
although, I must admit,the later is much more concise.

Can you give more of a grand vision for what these files will contain when you're done hacking this all?
Comment 6 Eric Seidel (no email) 2008-06-06 14:10:31 PDT
Comment on attachment 21380 [details]
First patch (using XML::Tiny)

Ok, Julien convinced me this was an incremental first step.  I think this is fine as-is.  I'd like to see the rest of the "Grand vision" impelmented sooner rather than later though, since this syntax as-is is more combersome than the old files (for no gain).
Comment 7 Julien Chaffraix 2008-06-06 19:13:59 PDT
Comment on attachment 21380 [details]
First patch (using XML::Tiny)

Committed in r34410.
Comment 8 Julien Chaffraix 2008-06-07 02:15:13 PDT
Created attachment 21556 [details]
Second part - add XML attributes' parsing and start moving some hacks
Comment 9 Eric Seidel (no email) 2008-06-07 02:19:16 PDT
Comment on attachment 21556 [details]
Second part - add XML attributes' parsing and start moving some hacks

looks good.
Comment 10 Julien Chaffraix 2008-06-07 02:59:34 PDT
Comment on attachment 21556 [details]
Second part - add XML attributes' parsing and start moving some hacks

Committed in r34415.
Comment 11 Julien Chaffraix 2008-06-07 06:18:06 PDT
Created attachment 21559 [details]
Third part - add a parameters hash and move some command line options to the *.in files
Comment 12 Darin Adler 2008-06-08 13:58:49 PDT
Comment on attachment 21559 [details]
Third part - add a parameters hash and move some command line options to the *.in files

r=me
Comment 13 Julien Chaffraix 2008-06-09 04:03:03 PDT
Comment on attachment 21559 [details]
Third part - add a parameters hash and move some command line options to the *.in files

landed in r34467.
Comment 14 Julien Chaffraix 2008-06-09 07:47:56 PDT
Created attachment 21593 [details]
Fourth part: move more parameters
Comment 15 Eric Seidel (no email) 2008-06-10 10:22:38 PDT
Comment on attachment 21593 [details]
Fourth part: move more parameters

attrs shoudl have a null namespace by default.  SVG Attrs do not have a namespaceURI.

I don't think generateFactory and generateWrapperFactory shoudl be in these text files.  Those should be controlled by the scripts.  (i.e. you can generate one of htem if you want using the scripts).

The part of this which seems good in the files is the guardFactoryWith logic.

We can talk about it more over IRC if you like.
Comment 16 Eric Seidel (no email) 2008-06-10 10:44:19 PDT
Comment on attachment 21593 [details]
Fourth part: move more parameters

Sigh.  I've decided this is not really worth the trouble to make this more complicated.
Comment 17 Julien Chaffraix 2008-06-10 13:51:30 PDT
Comment on attachment 21593 [details]
Fourth part: move more parameters

landed in r34484.
Comment 18 Julien Chaffraix 2008-06-10 17:24:05 PDT
Created attachment 21612 [details]
Final patch - Move more parameters in the XML files - Clean up the build systems
Comment 19 Darin Adler 2008-06-11 11:58:38 PDT
Comment on attachment 21612 [details]
Final patch - Move more parameters in the XML files - Clean up the build systems

Why the mix of '1' in some places and "1" in others?

Seems fine.

r=me
Comment 20 Julien Chaffraix 2008-06-11 17:46:29 PDT
(In reply to comment #19)
> (From update of attachment 21612 [details] [edit])
> Why the mix of '1' in some places and "1" in others?

I did not pay enough attention to the mix (the parser will not make a difference between the 2). I will correct the problem before landing.

I will also hold the patch a bit as there is an ongoing discussion on webkit-dev about the new format which could impact it.
Comment 21 Julien Chaffraix 2008-08-12 17:22:28 PDT
Closing the bug as the patch is outdated, it does not go in the direction that was discussed on the mailing list and I have something coming to solve it in a better way.