Hi all- I've been attempting to do some code cleanup in WebCore as of late. I've removed several dead (and rotted) sections of #ifdef'd code, and am about make patches to remove some more #ifdef'd sections and I wanted to make sure there were no complaints. CACHE_DEBUG PARSER_DEBUG TOKEN_DEBUG DEBUG_LAYOUT FORMS_DEBUG KJS_VERBOSE CSS_DEBUG CSS_STYLESHEET_DEBUG is the list of ifdefs (and associated code) which I plan to post patches to remove. kdWarning, kdDebug, kdError are all noops in WebCore, thus these #ifdefs are useless (without defining your own local kdDebug, etc.) Any complaints? Any ifdefs which I haven't listed there which you'd like to make sure we don't remove? -eric
Hi! On Sun, 4 Dec 2005 05:31:10 -0800 Eric Seidel <eseidel@apple.com> wrote:
Any complaints?
Any ifdefs which I haven't listed there which you'd like to make sure we don't remove?
I'm using DECODER_DEBUG all the time, and have used TOKEN_DEBUG once. So, I would prefer to have at least the former preserved (changing kdDebug to fprintf or cerr is OK with me, though). Hard to comment about other ifdefs. Guess it depends on how rotten the code is... After all, the logs are what helped to write this code in the first place, so they are supposed to be positioned better than something one can add ad hoc :) - WBR, Alexey Proskuryakov
On Dec 4, 2005, at 7:31 AM, Alexey Proskuryakov wrote:
Hi!
On Sun, 4 Dec 2005 05:31:10 -0800 Eric Seidel <eseidel@apple.com> wrote:
Any complaints? Any ifdefs which I haven't listed there which you'd like to make sure we don't remove?
I'm using DECODER_DEBUG all the time, and have used TOKEN_DEBUG once. So, I would prefer to have at least the former preserved (changing kdDebug to fprintf or cerr is OK with me, though).
Hard to comment about other ifdefs. Guess it depends on how rotten the code is... After all, the logs are what helped to write this code in the first place, so they are supposed to be positioned better than something one can add ad hoc :)
This is exactly why I sent out this note! If folks are still using these, then we should keep them. We're slowly removing them anyway, but perhaps we could replace them with something smaller and cleaner? Perhaps a define like DECODER_DEUBG("my great message %s", myGreatCharStar); There are a couple problems with the existing approach: 1. It's big and ugly. (3 lines for a little log) 2. If you don't use the #ifdefs, due to the c++ stream syntax, there is no way to no have everything right of << not executed at runtime, for example: kdDebug(123) << doSomethingReallySlowly() << touchABunchOfMemory() <<endl; These are some of the reasons why we've been slowly removing these blocks (both the ifdefs and the raw kdDebug(123) stuff). -eric
On Sunday 04 December 2005 08:31, Eric Seidel wrote:
Hi all-
I've been attempting to do some code cleanup in WebCore as of late. I've removed several dead (and rotted) sections of #ifdef'd code, and am about make patches to remove some more #ifdef'd sections and I wanted to make sure there were no complaints.
CACHE_DEBUG PARSER_DEBUG TOKEN_DEBUG DEBUG_LAYOUT FORMS_DEBUG KJS_VERBOSE CSS_DEBUG CSS_STYLESHEET_DEBUG
is the list of ifdefs (and associated code) which I plan to post patches to remove. kdWarning, kdDebug, kdError are all noops in WebCore, thus these #ifdefs are useless (without defining your own local kdDebug, etc.)
Any complaints?
Any ifdefs which I haven't listed there which you'd like to make sure we don't remove?
I object as well. This is just making it even harder to merge things with our tree, and we definitely use them. -- George Staikos KDE Developer http://www.kde.org/ Staikos Computing Services Inc. http://www.staikos.net/
On Dec 4, 2005, at 11:39 AM, George Staikos wrote:
On Sunday 04 December 2005 08:31, Eric Seidel wrote:
Hi all-
I've been attempting to do some code cleanup in WebCore as of late. I've removed several dead (and rotted) sections of #ifdef'd code, and am about make patches to remove some more #ifdef'd sections and I wanted to make sure there were no complaints.
CACHE_DEBUG PARSER_DEBUG TOKEN_DEBUG DEBUG_LAYOUT FORMS_DEBUG KJS_VERBOSE CSS_DEBUG CSS_STYLESHEET_DEBUG
is the list of ifdefs (and associated code) which I plan to post patches to remove. kdWarning, kdDebug, kdError are all noops in WebCore, thus these #ifdefs are useless (without defining your own local kdDebug, etc.)
Any complaints?
Any ifdefs which I haven't listed there which you'd like to make sure we don't remove?
I object as well. This is just making it even harder to merge things with our tree, and we definitely use them.
As I mentioned in my previous email to Alexey, I think there are smaller, cleaner ways of achieving the same goal. Unfortunately the "it's hard to merge between khtml" isn't a very strong argument anymore, given how far we've diverged in the last 4.5 years. (RefPtr, #APPLE_CHANGES, JSC gc changes, QualifiedName being just a few on a long list). The goal here is to make the source as clean and readable as possible. Perhaps then merging at a file-level (as we're currently attempting with kdom/ksvg2) will be a more viable approach. -eric
On Tuesday 06 December 2005 17:10, Eric Seidel wrote:
As I mentioned in my previous email to Alexey, I think there are smaller, cleaner ways of achieving the same goal.
Unfortunately the "it's hard to merge between khtml" isn't a very strong argument anymore, given how far we've diverged in the last 4.5 years. (RefPtr, #APPLE_CHANGES, JSC gc changes, QualifiedName being just a few on a long list).
The goal here is to make the source as clean and readable as possible. Perhaps then merging at a file-level (as we're currently attempting with kdom/ksvg2) will be a more viable approach.
I don't doubt there's something better. It's just frustrating that we have an initiative to merge things underway and the diff is simultaneously getting larger and larger. -- George Staikos KDE Developer http://www.kde.org/ Staikos Computing Services Inc. http://www.staikos.net/
Err...wasn't "merging between khtml" one of the main reasons why WebKit was made available as open source, or did I miss something at WWDC 2005? Dave On Tue, Dec 06, 2005 at 02:10:02PM -0800, Eric Seidel wrote:
As I mentioned in my previous email to Alexey, I think there are smaller, cleaner ways of achieving the same goal.
Unfortunately the "it's hard to merge between khtml" isn't a very strong argument anymore, given how far we've diverged in the last 4.5 years. (RefPtr, #APPLE_CHANGES, JSC gc changes, QualifiedName being just a few on a long list).
The goal here is to make the source as clean and readable as possible. Perhaps then merging at a file-level (as we're currently attempting with kdom/ksvg2) will be a more viable approach.
-eric
On Wednesday 07 December 2005 12:24, David D. Kilzer wrote:
Err...wasn't "merging between khtml" one of the main reasons why WebKit was made available as open source, or did I miss something at WWDC 2005?
No, KDE doesn't use WebKit. WebCore and JSCore are based on KHTML and KJS. We're trying to bring them back in line again. KDE is very close to taking on JSCore as-is in the Apple tree right now. There are just a few issues to resolve I think. WebCore/KHTML is a much more complex issue though. We're working on a plan for that. -- George Staikos KDE Developer http://www.kde.org/ Staikos Computing Services Inc. http://www.staikos.net/
The patches in question are attached to this bug: http://bugzilla.opendarwin.org/show_bug.cgi?id=5931 -eric On Dec 4, 2005, at 5:31 AM, Eric Seidel wrote:
Hi all-
I've been attempting to do some code cleanup in WebCore as of late. I've removed several dead (and rotted) sections of #ifdef'd code, and am about make patches to remove some more #ifdef'd sections and I wanted to make sure there were no complaints.
CACHE_DEBUG PARSER_DEBUG TOKEN_DEBUG DEBUG_LAYOUT FORMS_DEBUG KJS_VERBOSE CSS_DEBUG CSS_STYLESHEET_DEBUG
is the list of ifdefs (and associated code) which I plan to post patches to remove. kdWarning, kdDebug, kdError are all noops in WebCore, thus these #ifdefs are useless (without defining your own local kdDebug, etc.)
Any complaints?
Any ifdefs which I haven't listed there which you'd like to make sure we don't remove?
-eric _______________________________________________ webkit-dev mailing list webkit-dev@opendarwin.org http://www.opendarwin.org/mailman/listinfo/webkit-dev
participants (4)
-
Alexey Proskuryakov
-
ddkilzer@kilzer.net
-
Eric Seidel
-
George Staikos