krasimir added inline comments.
================
Comment at: lib/Format/Format.cpp:1823
+ unsigned LongestMatchLength = 0;
+ for (unsigned i = 0; i < Style.JavaImportGroups.size(); i++) {
+ std::string GroupPrefix = Style.JavaImportGroups[i];
----------------
nit: LLVM Style uses CamelCase for variables, please use `I`.
================
Comment at: lib/Format/Format.cpp:1834
+
+// Sorts and deduplicate a block of includes given by 'Imports' based on
+// JavaImportGroups, then adding the necessary replacement to 'Replaces'.
----------------
nit: Sorts and deduplicates
================
Comment at: lib/Format/Format.cpp:1836
+// JavaImportGroups, then adding the necessary replacement to 'Replaces'.
+// import declarations with the same text will be deduplicated. Between each
+// import group, a newline is inserted, and within each import group, a
----------------
nit: Import declarations...
================
Comment at: unittests/Format/SortImportsTestJava.cpp:99
+ "import org.a;\n"));
+}
+
----------------
Add a test where there are existing blocks of import statements separated by
empty lines. I'd expect that a newly added import gets added to the right
already existing block, right?
================
Comment at: unittests/Format/SortImportsTestJava.cpp:136
+ EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
+ "import org.a;\n"));
+}
----------------
interested does the newline get taken into account? e.g. what happens with
```
sort("import org.a;\n"
"import org.b;");
```
Repository:
rC Clang
https://reviews.llvm.org/D52800
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
