[PATCH] D20798: clang-format: [JS] Sort imported symbols.

[Daniel Jasper via cfe-commits]

djasper added a comment.

In general, this is lacking test cases for imports that are wrapped over multiple lines to start with. Should probably add those both for the old and for the new behavior.


================
Comment at: lib/Format/Format.cpp:1229
@@ -1227,9 +1228,3 @@
   // the entire block. Otherwise, no replacement is generated.
-  bool OutOfOrder = false;
-  for (unsigned i = 1, e = Indices.size(); i != e; ++i) {
-    if (Indices[i] != i) {
-      OutOfOrder = true;
-      break;
-    }
-  }
-  if (!OutOfOrder)
+  bool InOrder = std::is_sorted(Indices.begin(), Indices.end());
+  if (InOrder)
----------------
I think it's not even worth storing this variable now.

================
Comment at: lib/Format/SortJavaScriptImports.cpp:46
@@ +45,3 @@
+
+  bool operator==(const JsImportedSymbol &RHS) const {
+    return Symbol == RHS.Symbol && Alias == RHS.Alias;
----------------
Maybe add a comment on why you are ignoring 'Range' for comparison.

================
Comment at: lib/Format/SortJavaScriptImports.cpp:168
@@ +167,3 @@
+
+    if (ReferencesInOrder && SymbolsInOrder) {
+      return Result;
----------------
nit: no braces

================
Comment at: lib/Format/SortJavaScriptImports.cpp:232
@@ +231,3 @@
+    std::stable_sort(Symbols.begin(), Symbols.end(),
+                     [&](JsImportedSymbol LHS, JsImportedSymbol RHS) {
+                       return LHS.Symbol < RHS.Symbol;
----------------
I think it's safe to pass the symbols as const reference here.


http://reviews.llvm.org/D20798