[PATCH] D67843: DisableFormat also now disables SortIncludes

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 21 10:31:55 PDT 2019


MyDeveloperDay added a comment.

I assume the intention was that users could have DisableFormat=true and SortIncludes=true when they want to sort the includes but not perform any additional formatting in the code.

I think by making this change you make it impossible to run clang-format through a codebase with the sole intention of just sorting the headers. (which I could see as potentially useful isolated functionality)..

If SortIncludes is false by default? (which you are making it not for no style so I'm unclear what it would be now if you running without a BasedOnStyle setting (uninitialized?)) then you don't need to supply both unless you are using LLVM style or one of the other styles that turn it on.

Are you sure this is the right change?



================
Comment at: clang/lib/Format/Format.cpp:2131
   tooling::Replacements Replaces;
-  if (!Style.SortIncludes)
+  if (!Style.SortIncludes || Style.DisableFormat)
     return Replaces;
----------------
this no longer allows DisableFormat=true but SortInclude=true... i.e. running ClangFormat to just sort the headers.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67843/new/

https://reviews.llvm.org/D67843





More information about the cfe-commits mailing list