Bug 5877 - Remove remaining APPLE_CHANGES and KHTML_NO_CPP_DOM blocks from WebCore
Summary: Remove remaining APPLE_CHANGES and KHTML_NO_CPP_DOM blocks from WebCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P4 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 5929 5931
  Show dependency treegraph
 
Reported: 2005-11-29 04:43 PST by Eric Seidel (no email)
Modified: 2005-12-04 03:34 PST (History)
0 users

See Also:


Attachments
The patch (168.02 KB, patch)
2005-11-29 04:46 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
cleanup.pl script (383 bytes, text/x-perl-script)
2005-11-29 04:46 PST, Eric Seidel (no email)
no flags Details
A hacked version of the partial preprocessor (partpp.pl) (17.66 KB, text/x-perl-script)
2005-11-29 04:47 PST, Eric Seidel (no email)
no flags Details
Responding to Darin's comments. (216.53 KB, patch)
2005-11-29 14:59 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
A newer hacked version of partpp.pl which has whitespace removal heuristics (17.98 KB, text/x-perl-script)
2005-12-04 02:27 PST, Eric Seidel (no email)
no flags Details
Diff from the original (http://www.phost.de/~stefan/Files/partpp.pl) partpp.pl (for the curious) (4.60 KB, patch)
2005-12-04 02:28 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
SpacingHeuristics.pm perl module (1.67 KB, text/x-perl-script)
2005-12-04 02:28 PST, Eric Seidel (no email)
no flags Details
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace) (162.13 KB, patch)
2005-12-04 02:29 PST, Eric Seidel (no email)
mjs: review+
Details | Formatted Diff | Diff
newer cleanup.pl (trivial partpp.pl wrapper) (361 bytes, text/x-perl-script)
2005-12-04 02:33 PST, Eric Seidel (no email)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2005-11-29 04:43:10 PST
Remove remaining APPLE_CHANGES and KHTML_NO_CPP_DOM blocks from WebCore

The previous bug:
http://bugzilla.opendarwin.org/show_bug.cgi?id=5711
only removed APPLE_CHANGES from WebCore/khtml

This removes all the rest from WebCore (kwq mostly), with the exception of SVG, and additionally removes 
all KHTML_NO_CPP_DOM blocks.
Comment 1 Eric Seidel (no email) 2005-11-29 04:46:15 PST
Created attachment 4847 [details]
The patch
Comment 2 Eric Seidel (no email) 2005-11-29 04:46:54 PST
Created attachment 4848 [details]
cleanup.pl script
Comment 3 Eric Seidel (no email) 2005-11-29 04:47:31 PST
Created attachment 4849 [details]
A hacked version of the partial preprocessor (partpp.pl)
Comment 4 Eric Seidel (no email) 2005-11-29 04:54:54 PST
Comment on attachment 4847 [details]
The patch

This also removes all #if 0 blocks (which may or may not be a good thing).
Comment 5 Darin Adler 2005-11-29 07:36:20 PST
Comment on attachment 4847 [details]
The patch

This patch shows lots of cases where it leaves stray multiple blank lines. So
it's going to leave formatting a little messy. I'd like to see a version that
has a heuristic for nicely eating the excess blank lines.

I also see cases where there's a comment just before an #if 0 section. This
patch is going to lave those comments stranded. An example is in DOMHTML.mm.

Some of this already probably took place with the last "remove ifdefs" patch,
which I did not review in detail.
Comment 6 Eric Seidel (no email) 2005-11-29 14:59:21 PST
Created attachment 4855 [details]
Responding to Darin's comments.
Comment 7 Eric Seidel (no email) 2005-11-29 14:59:30 PST
Comment on attachment 4855 [details]
Responding to Darin's comments.

To address Darin's concerns, I did a couple things:

1.  I did a search throughout the entire project looking for "excessive"
newlines and removed them.
2.  I did a manual review of each of the previously changed files and removed
any extra comments, newlines.
3.  I removed dom_misc.* entirely, and moved the remaining DOM::EventListener
and DOM::NodeFilterCondition onto khtml::Shared<T>
4.  As part of my manual walk through khtml_part.cpp I removed
DIRECT_LINKAGE_TO_ECMA as well.
Comment 8 Eric Seidel (no email) 2005-11-29 15:00:04 PST
Comment on attachment 4847 [details]
The patch

Retracting review on this one for now.	See the new larger patch instead.
Comment 9 Darin Adler 2005-12-03 11:10:29 PST
Comment on attachment 4855 [details]
Responding to Darin's comments.

Here's an example of the kind of blank line thing I'm not so happy about:

---------

 #include "khtmlpart_p.h"

-#if !KHTML_NO_CPLUSPLUS_DOM
-#include "dom/html_document.h"
-#endif

 #include <CoreServices/CoreServices.h>

---------

and here's another:

---------

   cancelRedirection();

-#if !KHTML_NO_CPLUSPLUS_DOM
-  // null node activated.
-  emit nodeActivated(Node());
-#endif
 }

---------

Otherwise, this patch looks fine.
Comment 10 Eric Seidel (no email) 2005-12-04 02:26:11 PST
Comment on attachment 4855 [details]
Responding to Darin's comments.

Retracting review, posting a simpler patch instead.
Comment 11 Eric Seidel (no email) 2005-12-04 02:27:22 PST
Created attachment 4926 [details]
A newer hacked version of partpp.pl which has whitespace removal heuristics
Comment 12 Eric Seidel (no email) 2005-12-04 02:28:00 PST
Created attachment 4927 [details]
Diff from the original (http://www.phost.de/~stefan/Files/partpp.pl) partpp.pl (for the curious)
Comment 13 Eric Seidel (no email) 2005-12-04 02:28:48 PST
Created attachment 4928 [details]
SpacingHeuristics.pm perl module
Comment 14 Eric Seidel (no email) 2005-12-04 02:29:36 PST
Created attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)
Comment 15 Eric Seidel (no email) 2005-12-04 02:32:23 PST
Comment on attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)

This is a simpler patch which is entirely script generated and addresses
darin's whitespace concerns.  Script is also attached to the bug.  This should
be a trivial review which any reviewer should feel comfortable with.
Comment 16 Eric Seidel (no email) 2005-12-04 02:33:29 PST
Created attachment 4930 [details]
newer cleanup.pl (trivial partpp.pl wrapper)
Comment 17 Maciej Stachowiak 2005-12-04 03:21:30 PST
Comment on attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)

rs=me