Bug 27557

Summary:

Case-insensitive comparison of include file sorting for cpplint

Product:

WebKit

Reporter:

Jakob Petsovits <jpetsovits>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

darin, manyoso, mitz, staikos

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
Case-insensitive comparison of include file order for cpplint	darin: review-

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.