Bug 55253

Summary: check-webkit-style should check for spaces after periods in comments
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, darin, levin, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 55272    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Eric Seidel (no email)
Reported 2011-02-25 13:12:09 PST
check-webkit-style should check for spaces after periods in comments I feel like every few patches someone tells me my spaces after periods are wrong. My typing teacher taught me 2, but I don't really care which. But I don't want to have my patches cq-'d due to lack of "proper" spacing. :) I'd rather have check-webkit-style tell me what to do (assuming we even want to bother to specify it). CCing folks who I believe care about this.
Attachments
Patch (4.67 KB, patch)
2011-02-25 17:40 PST, David Levin 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2011-02-25 13:12:53 PST
We should also add a rule to http://www.webkit.org/coding/coding-style.html if we're going to enforce this.
Eric Seidel (no email)
Comment 2 2011-02-25 13:14:20 PST
My tone in comment 0 sounds more angry than I mean. :) I mean more to sound ignorant and willing to do what I"m told. :)
David Levin
Comment 3 2011-02-25 14:16:21 PST
(In reply to comment #2) > My tone in comment 0 sounds more angry than I mean. :) I mean more to sound ignorant and willing to do what I"m told. :) I don't really care. I simply tell people because that is how I was brought up to do things in WebKit (per ap). It is part of my way outstanding work to put this in the style guide as it is one of the items in https://lists.webkit.org/pipermail/webkit-dev/2009-September/009807.html I very much need to add all of that to the style guide (and to the style checker) -- I'll try to get to this (unfortunately these items are my side items to handle).
David Levin
Comment 4 2011-02-25 14:17:43 PST
Added ap because he cares/often notes it as well (but I don't think there is much debate here -- as this was already brought up on the alias and just hasn't made it into the style guide yet b/c no one like me has put it there).
Darin Adler
Comment 5 2011-02-25 14:53:37 PST
I do care about consistency in this. And we have chosen single spaces. So yes, lets put it in the style guide and the checker script.
Alexey Proskuryakov
Comment 6 2011-02-25 15:50:20 PST
I've been trained so by Darin (and I also prefer it because Russian typography uses uniform spacing, but that's completely irrelevant for WebKit).
David Levin
Comment 7 2011-02-25 17:40:03 PST
Created attachment 83906 [details] Patch
WebKit Commit Bot
Comment 8 2011-02-26 07:27:17 PST
The commit-queue encountered the following flaky tests while processing attachment 83906 [details]: inspector/debugger/debugger-pause-on-exception.html bug 55298 (authors: loislo@chromium.org and pfeldman@chromium.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 9 2011-02-26 07:30:06 PST
Comment on attachment 83906 [details] Patch Clearing flags on attachment: 83906 Committed r79787: <http://trac.webkit.org/changeset/79787>
WebKit Commit Bot
Comment 10 2011-02-26 07:30:10 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.