Created attachment 46732 [details]
In recent years we have added a couple of new attributes and tags and with HTML5
being there we will add some more tags as well.
The following patch is fixing it and as a follow up patch we should rename
the script to update-iexploder or update-iexploder-properties or some
Comment on attachment 46732 [details]
> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> +2010-01-15 Holger Hans Peter Freyther <email@example.com>
> + Reviewed by NOBODY (OOPS!).
> + [iexploder] Automatically update htmltags.in and htmlattrs.in too
> + Need a short description and bug URL (OOPS!)
Need the bug URL here instead of the placeholder text.
> diff --git a/WebKitTools/Scripts/update-iexploder-cssproperties b/WebKitTools/Scripts/update-iexploder-cssproperties
> @@ -35,38 +37,54 @@ use strict;
> use FindBin;
> use lib $FindBin::Bin;
> use webkitdirs;
> +use VCSUtils;
Alphabetically, VCSUtils should go before webkitdirs.
> +sub generateAttributesFromFile($);
> +sub readiExploderFile($);
> +sub writeiExploderFile($@);
> +sub update($$);
> +sub updateCSSProperties();
> +sub updateHTMLAttributes();
> +sub updateHTMLTags();
Alphabetically, writeiExploderFile() should go last in the list.
> +sub generateAttributesFromFile($)
"Attributes" sounds too specific. How about "Entities"?
Also, we're returning a list of entities, so I think this should be called generateEntityListFromFile().
> + my ($filename) = @_;
> + my $revision = svnRevisionForDirectory(dirname($filename));
> + my $path = File::Spec->abs2rel($filename, sourceDir());
> my $result = "# From WebKit svn r" . $revision . " (" . $path . ")\n";
> my @properties = ();
This should change to @entities to match the subroutine name.
> push(@properties, $l);
> +sub update($$)
> + my ($iexploder, $webcore) = @_;
I think these should be called $iexploderPath and $webcorePath.
> + $iexploder = File::Spec->catfile(sourceDir, split("/", $iexploder));
> + $webcore = File::Spec->catfile(sourceDir, split("/", $webcore));
Please add the parenthesis back to "sourceDir" here.
> +sub updateCSSProperties()
> + update("WebKitTools/iExploder/htdocs/cssproperties.in", "WebCore/css/CSSPropertyNames.in");
> +sub updateHTMLAttributes()
> + update("WebKitTools/iExploder/htdocs/htmlattrs.in", "WebCore/html/HTMLAttributeNames.in");
> +sub updateHTMLTags()
> + update("WebKitTools/iExploder/htdocs/htmltags.in", "WebCore/html/HTMLTagNames.in");
I just realized you could make this more generic by using a hash to hold these values, then just iterate over the hash in a single update() method.
If you'd rather keep separate methods, please add them before writeiExploderFile() to keep the subroutines alphabetized.
r- to consider all the minor issues here. (I'd like to be able to cq+ the patch when finished. :)
Created attachment 47098 [details]
[iexploder] Automatically update htmltags.in and htmlattrs.in too
(In reply to comment #1)
I should have addressed all comments form above (sorting, naming, bug id).
> I just realized you could make this more generic by using a hash to hold these
> values, then just iterate over the hash in a single update() method.
I'm not sure if I follow here? We are writing three different files so the hash would be the input to the method?
> If you'd rather keep separate methods, please add them before
> writeiExploderFile() to keep the subroutines alphabetized.
I nuked them. And I also append the path for htmldocs and WebCore inside the update method.
> r- to consider all the minor issues here. (I'd like to be able to cq+ the
> patch when finished. :)
I'm fine to land the patch myself. :)
As follow up commits we should rename the file, and I'm working on a commit to automatically append deleted entities to the end of the file.
Attachment 47098 [details] was posted by a committer and has review+, assigning to Holger Freyther for commit.
Landed in r55658. The htmlattrs.in needs another now.