Bug 27557 - Case-insensitive comparison of include file sorting for cpplint
Summary: Case-insensitive comparison of include file sorting for cpplint
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-22 12:58 PDT by Jakob Petsovits
Modified: 2009-07-22 13:34 PDT (History)
4 users (show)

See Also:


Attachments
Case-insensitive comparison of include file order for cpplint (2.91 KB, patch)
2009-07-22 13:01 PDT, Jakob Petsovits
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakob Petsovits 2009-07-22 12:58:56 PDT
Quoting my test case:

#include "a.h"
'#include "B.h"
'#include "c.h"

That thing currently fails. The fix is straightforward, patch goes below.
Comment 1 Jakob Petsovits 2009-07-22 13:01:06 PDT
Created attachment 33284 [details]
Case-insensitive comparison of include file order for cpplint
Comment 2 Darin Adler 2009-07-22 13:02:42 PDT
Comment on attachment 33284 [details]
Case-insensitive comparison of include file order for cpplint

The order is supposed to be case-sensitive, as if done by the command line "sort" tool. Did someone say it should be case-insensitive?
Comment 3 Darin Adler 2009-07-22 13:05:01 PDT
Comment on attachment 33284 [details]
Case-insensitive comparison of include file order for cpplint

Setting to review- for now. I believe the sort is supposed to be case sensitive, but if there is consensus otherwise, then this is a good change to make.
Comment 4 Adam Treat 2009-07-22 13:06:47 PDT
Oops.  I committed it already.  Should I back out until we find consensus?
Comment 5 Darin Adler 2009-07-22 13:15:45 PDT
(In reply to comment #4)
> Oops.  I committed it already.  Should I back out until we find consensus?

Yes, I think so. I'm almost certain our standard was case-sensitive as done by the "sort" tool, and I believe it would require discussion to change that.
Comment 6 Jakob Petsovits 2009-07-22 13:19:45 PDT
Then I'm probably adding a test case to clarify that way of working, right?
Comment 7 Adam Treat 2009-07-22 13:22:46 PDT
Reverted with r46235.
 
Sorry guys.  It just doesn't seem very intuitive to me to sort alphabetically with case-sensitivity.  But if thems the rules, thems the rules.
Comment 8 George Staikos 2009-07-22 13:31:40 PDT
sort -f?

All in all the concept of sorting headers alphabetically as a rule seems like a scary and arbitrary decision, though I'm really not interested in debating it.
Comment 9 Darin Adler 2009-07-22 13:34:34 PDT
Why do we sort at all?

One reason is to make it less likely we will end up with duplicate includes. Another is to ensure we don't get a variety of different "logical" sorting orders invented by different well-meaning programmers in different source files.

Case-sensitive vs. case-insensitive is entirely arbitrary, and we chose case-sensitive long ago. I'd be open to considering a change.