<div dir="ltr">Up to you. I am not worried about accidentally breaking it in the foreseeable future.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 16, 2015 at 8:20 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">i.e. should SortIncludes be false for -style=WebKit for now?</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 16, 2015 at 11:19 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Should this be a per-style default? It still only doesn't break webkit files only by accident, right?</div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 16, 2015 at 10:35 AM, Eric Christopher via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">!!!!!!!<div><br></div><div>Awesome :)</div><div><br></div><div>-eric</div></div><div><div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 16, 2015 at 4:41 AM Daniel Jasper via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: djasper<br>
Date: Mon Nov 16 06:38:56 2015<br>
New Revision: 253202<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=253202&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=253202&view=rev</a><br>
Log:<br>
clang-format: Enable #include sorting by default.<br>
<br>
This has seen quite some usage and I am not aware of any issues. Also<br>
add a style option to enable/disable include sorting. The existing<br>
command line flag can from now on be used to override whatever is set<br>
in the style.<br>
<br>
Added:<br>
    cfe/trunk/test/Format/disable-include-sorting.cpp<br>
Modified:<br>
    cfe/trunk/include/clang/Format/Format.h<br>
    cfe/trunk/lib/Format/Format.cpp<br>
    cfe/trunk/tools/clang-format/ClangFormat.cpp<br>
    cfe/trunk/tools/clang-format/clang-format-sublime.py<br>
    cfe/trunk/tools/clang-format/clang-format.el<br>
    cfe/trunk/tools/clang-format/clang-format.py<br>
    cfe/trunk/unittests/Format/FormatTest.cpp<br>
    cfe/trunk/unittests/Format/SortIncludesTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Format/Format.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Format/Format.h (original)<br>
+++ cfe/trunk/include/clang/Format/Format.h Mon Nov 16 06:38:56 2015<br>
@@ -469,6 +469,9 @@ struct FormatStyle {<br>
   /// Pointer and reference alignment style.<br>
   PointerAlignmentStyle PointerAlignment;<br>
<br>
+  /// \brief If true, clang-format will sort #includes.<br>
+  bool SortIncludes;<br>
+<br>
   /// \brief If \c true, a space may be inserted after C style casts.<br>
   bool SpaceAfterCStyleCast;<br>
<br>
<br>
Modified: cfe/trunk/lib/Format/Format.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/Format.cpp (original)<br>
+++ cfe/trunk/lib/Format/Format.cpp Mon Nov 16 06:38:56 2015<br>
@@ -284,6 +284,7 @@ template <> struct MappingTraits<FormatS<br>
     IO.mapOptional("PenaltyExcessCharacter", Style.PenaltyExcessCharacter);<br>
     IO.mapOptional("PenaltyReturnTypeOnItsOwnLine",<br>
                    Style.PenaltyReturnTypeOnItsOwnLine);<br>
+    IO.mapOptional("SortIncludes", Style.SortIncludes);<br>
     IO.mapOptional("PointerAlignment", Style.PointerAlignment);<br>
     IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);<br>
     IO.mapOptional("SpaceBeforeAssignmentOperators",<br>
@@ -507,6 +508,7 @@ FormatStyle getLLVMStyle() {<br>
   LLVMStyle.PenaltyBreakBeforeFirstCallParameter = 19;<br>
<br>
   LLVMStyle.DisableFormat = false;<br>
+  LLVMStyle.SortIncludes = true;<br>
<br>
   return LLVMStyle;<br>
 }<br>
@@ -635,6 +637,7 @@ FormatStyle getGNUStyle() {<br>
 FormatStyle getNoStyle() {<br>
   FormatStyle NoStyle = getLLVMStyle();<br>
   NoStyle.DisableFormat = true;<br>
+  NoStyle.SortIncludes = false;<br>
   return NoStyle;<br>
 }<br>
<br>
@@ -1743,6 +1746,9 @@ tooling::Replacements sortIncludes(const<br>
                                    ArrayRef<tooling::Range> Ranges,<br>
                                    StringRef FileName) {<br>
   tooling::Replacements Replaces;<br>
+  if (!Style.SortIncludes)<br>
+    return Replaces;<br>
+<br>
   unsigned Prev = 0;<br>
   unsigned SearchFrom = 0;<br>
   llvm::Regex IncludeRegex(<br>
<br>
Added: cfe/trunk/test/Format/disable-include-sorting.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/disable-include-sorting.cpp?rev=253202&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/disable-include-sorting.cpp?rev=253202&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Format/disable-include-sorting.cpp (added)<br>
+++ cfe/trunk/test/Format/disable-include-sorting.cpp Mon Nov 16 06:38:56 2015<br>
@@ -0,0 +1,10 @@<br>
+// RUN: clang-format %s | FileCheck %s<br>
+// RUN: clang-format %s -sort-includes -style="{SortIncludes: false}" | FileCheck %s<br>
+// RUN: clang-format %s -sort-includes=false | FileCheck %s -check-prefix=NOT-SORTED<br>
+<br>
+#include <b><br>
+#include <a><br>
+// CHECK: <a><br>
+// CHECK-NEXT: <b><br>
+// NOT-SORTED: <b><br>
+// NOT-SORTED-NEXT: <a><br>
<br>
Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)<br>
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Mon Nov 16 06:38:56 2015<br>
@@ -98,9 +98,11 @@ static cl::opt<unsigned><br>
                     "clang-format from an editor integration"),<br>
            cl::init(0), cl::cat(ClangFormatCategory));<br>
<br>
-static cl::opt<bool> SortIncludes("sort-includes",<br>
-                                  cl::desc("Sort touched include lines"),<br>
-                                  cl::cat(ClangFormatCategory));<br>
+static cl::opt<bool> SortIncludes(<br>
+    "sort-includes",<br>
+    cl::desc("If set, overrides the include sorting behavior determined by the "<br>
+             "SortIncludes style flag"),<br>
+    cl::cat(ClangFormatCategory));<br>
<br>
 static cl::list<std::string> FileNames(cl::Positional, cl::desc("[<file> ...]"),<br>
                                        cl::cat(ClangFormatCategory));<br>
@@ -252,17 +254,14 @@ static bool format(StringRef FileName) {<br>
     return true;<br>
   StringRef AssumedFileName = (FileName == "-") ? AssumeFileName : FileName;<br>
   FormatStyle FormatStyle = getStyle(Style, AssumedFileName, FallbackStyle);<br>
-  Replacements Replaces;<br>
-  std::string ChangedCode;<br>
-  if (SortIncludes) {<br>
-    Replaces =<br>
-        sortIncludes(FormatStyle, Code->getBuffer(), Ranges, AssumedFileName);<br>
-    ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);<br>
-    for (const auto &R : Replaces)<br>
-      Ranges.push_back({R.getOffset(), R.getLength()});<br>
-  } else {<br>
-    ChangedCode = Code->getBuffer().str();<br>
-  }<br>
+  if (SortIncludes.getNumOccurrences() != 0)<br>
+    FormatStyle.SortIncludes = SortIncludes;<br>
+  Replacements Replaces =<br>
+      sortIncludes(FormatStyle, Code->getBuffer(), Ranges, AssumedFileName);<br>
+  std::string ChangedCode =<br>
+      tooling::applyAllReplacements(Code->getBuffer(), Replaces);<br>
+  for (const auto &R : Replaces)<br>
+    Ranges.push_back({R.getOffset(), R.getLength()});<br>
<br>
   bool IncompleteFormat = false;<br>
   Replaces = tooling::mergeReplacements(<br>
<br>
Modified: cfe/trunk/tools/clang-format/clang-format-sublime.py<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-sublime.py?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-sublime.py?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/tools/clang-format/clang-format-sublime.py (original)<br>
+++ cfe/trunk/tools/clang-format/clang-format-sublime.py Mon Nov 16 06:38:56 2015<br>
@@ -32,7 +32,7 @@ class ClangFormatCommand(sublime_plugin.<br>
     if encoding == 'Undefined':<br>
       encoding = 'utf-8'<br>
     regions = []<br>
-    command = [binary, '-sort-includes', '-style', style]<br>
+    command = [binary, '-style', style]<br>
     for region in self.view.sel():<br>
       regions.append(region)<br>
       region_offset = min(region.a, region.b)<br>
<br>
Modified: cfe/trunk/tools/clang-format/clang-format.el<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format.el?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format.el?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/tools/clang-format/clang-format.el (original)<br>
+++ cfe/trunk/tools/clang-format/clang-format.el Mon Nov 16 06:38:56 2015<br>
@@ -126,7 +126,6 @@ is no active region.  If no style is giv<br>
                  nil `(,temp-buffer ,temp-file) nil<br>
<br>
                  "-output-replacements-xml"<br>
-                 "-sort-includes"<br>
                  "-assume-filename" (or (buffer-file-name) "")<br>
                  "-style" style<br>
                  "-offset" (number-to-string start)<br>
<br>
Modified: cfe/trunk/tools/clang-format/clang-format.py<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format.py?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format.py?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/tools/clang-format/clang-format.py (original)<br>
+++ cfe/trunk/tools/clang-format/clang-format.py Mon Nov 16 06:38:56 2015<br>
@@ -72,7 +72,7 @@ def main():<br>
     startupinfo.wShowWindow = subprocess.SW_HIDE<br>
<br>
   # Call formatter.<br>
-  command = [binary, '-style', style, '-cursor', str(cursor), '-sort-includes']<br>
+  command = [binary, '-style', style, '-cursor', str(cursor)]<br>
   if lines != 'all':<br>
     command.extend(['-lines', lines])<br>
   if fallback_style:<br>
<br>
Modified: cfe/trunk/unittests/Format/FormatTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Nov 16 06:38:56 2015<br>
@@ -9563,12 +9563,14 @@ TEST_F(FormatTest, ParsesConfigurationBo<br>
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);<br>
   CHECK_PARSE_BOOL(DerivePointerAlignment);<br>
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");<br>
+  CHECK_PARSE_BOOL(DisableFormat);<br>
   CHECK_PARSE_BOOL(IndentCaseLabels);<br>
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);<br>
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);<br>
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);<br>
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);<br>
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);<br>
+  CHECK_PARSE_BOOL(SortIncludes);<br>
   CHECK_PARSE_BOOL(SpacesInParentheses);<br>
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);<br>
   CHECK_PARSE_BOOL(SpacesInAngles);<br>
<br>
Modified: cfe/trunk/unittests/Format/SortIncludesTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=253202&r1=253201&r2=253202&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=253202&r1=253201&r2=253202&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp Mon Nov 16 06:38:56 2015<br>
@@ -40,6 +40,16 @@ TEST_F(SortIncludesTest, BasicSorting) {<br>
                  "#include \"b.h\"\n"));<br>
 }<br>
<br>
+TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) {<br>
+  Style.SortIncludes = false;<br>
+  EXPECT_EQ("#include \"a.h\"\n"<br>
+            "#include \"c.h\"\n"<br>
+            "#include \"b.h\"\n",<br>
+            sort("#include \"a.h\"\n"<br>
+                 "#include \"c.h\"\n"<br>
+                 "#include \"b.h\"\n"));<br>
+}<br>
+<br>
 TEST_F(SortIncludesTest, MixIncludeAndImport) {<br>
   EXPECT_EQ("#include \"a.h\"\n"<br>
             "#import \"b.h\"\n"<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</div></div><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>