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