Summary: | Cleanup: Rename CSS list-style-type markers to conform to WebKit Code Style Guidelines | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||
Component: | CSS | Assignee: | Daniel Bates <dbates> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, bdakin, darin, hyatt, ian, mitz, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Daniel Bates
2009-12-20 12:26:16 PST
Created attachment 45288 [details]
Patch
No functionality was changed. So, no tests included in patch.
Attachment 45288 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/css/CSSPrimitiveValueMappings.h:933: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListItem.cpp:54: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/rendering/RenderListMarker.cpp:343: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:574: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:696: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:776: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:793: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:839: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
Total errors found: 8
Created attachment 45289 [details]
Patch with style-bot fixes (maybe we should do these fixes in a separate bug?)
An updated patch with more style corrections to make the style-bot happy.
I think I recall seeing a webkit-dev post about allowing switch indents when there are many cases. Maybe we can disagree with the style-bot (and not use this attachment) since the cleanup requires us to touch some switch statements that have many cases?
Also, if we do decide to go with the style-bot's suggestions then we may want to do these changes as part of a separate bug(s).
Comment on attachment 45288 [details]
Patch
I think that DecimalListStyle would be better than LDecimal and NoneListStyle better than LNone.
Created attachment 45306 [details]
Patch
Updated patch based on Darin's suggestion to substitute DecimalListStyle for LDecimal and NoneListStyle for LNone.
Attachment 45306 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/css/CSSPrimitiveValueMappings.h:939: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:343: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:574: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:696: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:776: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:793: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
WebCore/rendering/RenderListMarker.cpp:839: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
Total errors found: 7
Comment on attachment 45306 [details]
Patch
Maybe more should ahve the ListStyle postfix, but this seems better than what we had before. r=me.
Committed in <http://trac.webkit.org/changeset/52521>. |