Summary: | Remove remaining APPLE_CHANGES and KHTML_NO_CPP_DOM blocks from WebCore | ||
---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> |
Component: | New Bugs | Assignee: | Eric Seidel (no email) <eric> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | ||
Priority: | P4 | ||
Version: | 420+ | ||
Hardware: | Mac | ||
OS: | OS X 10.4 | ||
Bug Depends on: | |||
Bug Blocks: | 5929, 5931 | ||
Attachments: |
Description
Eric Seidel (no email)
2005-11-29 04:43:10 PST
Created attachment 4847 [details]
The patch
Created attachment 4848 [details]
cleanup.pl script
Created attachment 4849 [details]
A hacked version of the partial preprocessor (partpp.pl)
Comment on attachment 4847 [details]
The patch
This also removes all #if 0 blocks (which may or may not be a good thing).
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.
Created attachment 4855 [details]
Responding to Darin's comments.
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 on attachment 4847 [details]
The patch
Retracting review on this one for now. See the new larger patch instead.
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 on attachment 4855 [details]
Responding to Darin's comments.
Retracting review, posting a simpler patch instead.
Created attachment 4926 [details]
A newer hacked version of partpp.pl which has whitespace removal heuristics
Created attachment 4927 [details] Diff from the original (http://www.phost.de/~stefan/Files/partpp.pl) partpp.pl (for the curious) Created attachment 4928 [details]
SpacingHeuristics.pm perl module
Created attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)
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.
Created attachment 4930 [details]
newer cleanup.pl (trivial partpp.pl wrapper)
Comment on attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)
rs=me
|