Bug 32799

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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch with style-bot fixes (maybe we should do these fixes in a separate bug?)	none
Patch	eric: review+

Daniel Bates

Reported 2009-12-20 12:26:16 PST

Following up on a suggestion by Darin <https://bugs.webkit.org/show_bug.cgi?id=32138#c5>, we should rename the enums in RenderStyleConstants::EListStyleType to conform to the WebKit Code Style Guidelines.

Attachments
Patch (18.48 KB, patch) 2009-12-20 12:41 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Patch with style-bot fixes (maybe we should do these fixes in a separate bug?) (28.89 KB, patch) 2009-12-20 12:55 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Patch (18.74 KB, patch) 2009-12-20 22:01 PST, Daniel Bates	eric: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Daniel Bates

Comment 1 2009-12-20 12:41:09 PST

Created attachment 45288 [details] Patch No functionality was changed. So, no tests included in patch.

WebKit Review Bot

Comment 2 2009-12-20 12:41:34 PST

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

Daniel Bates

Comment 3 2009-12-20 12:55:12 PST

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).

Darin Adler

Comment 4 2009-12-20 21:06:21 PST

Comment on attachment 45288 [details] Patch I think that DecimalListStyle would be better than LDecimal and NoneListStyle better than LNone.

Daniel Bates

Comment 5 2009-12-20 22:01:44 PST

Created attachment 45306 [details] Patch Updated patch based on Darin's suggestion to substitute DecimalListStyle for LDecimal and NoneListStyle for LNone.

WebKit Review Bot

Comment 6 2009-12-20 22:02:55 PST

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

Eric Seidel (no email)

Comment 7 2009-12-20 22:39:38 PST

Comment on attachment 45306 [details] Patch Maybe more should ahve the ListStyle postfix, but this seems better than what we had before. r=me.

Daniel Bates

Comment 8 2009-12-22 23:31:04 PST

Committed in <http://trac.webkit.org/changeset/52521>.

Note You need to log in before you can comment on or make changes to this bug.