Bug 12228

Summary: Cleanup assorted CSS classes
Product: WebKit Reporter: Sam Weinig <sam>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
cleanup patch
darin: review+
updated patch
mitz: review+
patch without white space changes none

Sam Weinig
Reported 2007-01-12 12:15:40 PST
Patch forthcoming does cleanup of about half of the css files.
Attachments
cleanup patch (191.03 KB, patch)
2007-01-12 12:21 PST, Sam Weinig 		darin: review+
DetailsFormatted DiffDiff
updated patch (217.65 KB, patch)
2007-01-13 12:18 PST, Sam Weinig 		mitz: review+
DetailsFormatted DiffDiff
patch without white space changes (104.64 KB, patch)
2007-01-13 12:20 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2007-01-12 12:21:46 PST
Created attachment 12395 [details] cleanup patch
Darin Adler
Comment 2 2007-01-13 07:25:17 PST
Comment on attachment 12395 [details] cleanup patch The word "Hand" really shouldn't be capitalized in "shorthand". Do we really want braces for the for statements that don't need them? I wish our style guidelines were clear on that. If anything I'd like to see you applying even more of the m_ prefix. Hyatt was just talking about how it's hard to work on certain classes because it's so hard to tell the members apart from local variables. Fine to land this. r=me
Sam Weinig
Comment 3 2007-01-13 12:18:59 PST
Created attachment 12419 [details] updated patch This patch updates the previous version to address Darin's comments. Specifically, the member variables in CSSSelector have been prefixed with m_ and uses of ShortHand have been replaced with Shorthand.
Sam Weinig
Comment 4 2007-01-13 12:20:40 PST
Created attachment 12420 [details] patch without white space changes This version is made using svn diff -x -w for easier reviewing.
Dave Hyatt
Comment 5 2007-01-13 13:11:39 PST
Comment on attachment 12395 [details] cleanup patch Yeah, please make sure not to capitalize the H in shorthand.
mitz
Comment 6 2007-01-13 14:49:21 PST
Comment on attachment 12420 [details] patch without white space changes Misplaced *: + DashboardRegion *getDashboardRegionValue () const { return m_type != CSS_DASHBOARD_REGION ? 0 : m_value.region; } r=me
Sam Weinig
Comment 7 2007-01-14 17:26:21 PST
Landed in r18850.
Note You need to log in before you can comment on or make changes to this bug.