MyDeveloperDay added inline comments.
================ Comment at: clang/docs/ClangFormatStyleOptions.rst:1970 + but this behavior is changed by another option, + ``JavaStaticImportAfterImport``. ---------------- Can you add a test that shows if the sorting is still in the groups, i.e. I can't tell if ``` import com.test.a; import static org.a; import com.test.b; import static org.b; ``` becomes ``` import static org.a; import static org.b; import com.test.a; import com.test.b; ``` or ``` import static org.a; import static org.b; import com.test.a; import com.test.b; ``` or ``` import static org.a; import com.test.a; import static org.b; import com.test.b; ``` ================ Comment at: clang/include/clang/Format/Format.h:1708 + /// \endcode + bool JavaStaticImportAfterImport; + ---------------- Can we consider changing the name or the option to make it clearer what its for? `SortJavaStaticImport` and make it an enumeration rather than true/false `Before,After,Never` There need to be a "don't touch it option (.e.g. Never)" that does what it does today (and that should be the default, otherwise we end up causing clang-format to change ALL java code" which could be 100's of millions of lines+ ================ Comment at: clang/lib/Format/Format.cpp:906 LLVMStyle.JavaScriptWrapImports = true; + LLVMStyle.JavaStaticImportAfterImport = false; LLVMStyle.TabWidth = 8; ---------------- We can't have a default that does will cause a change ================ Comment at: clang/unittests/Format/SortImportsTestJava.cpp:253 +TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) { + FmtStyle.JavaStaticImportAfterImport = true; ---------------- please test all options You are also missing a test for checking the PARSING of the options Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87201/new/ https://reviews.llvm.org/D87201 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits