[PATCH] D13765: clang-format: [JS] Handle string literals spanning character classes.

[Daniel Jasper via cfe-commits]

djasper added inline comments.

================
Comment at: lib/Format/Format.cpp:755
@@ -754,3 +754,1 @@
-      if (tryMergeEscapeSequence())
-        return;
       if (tryMergeTemplateString())
----------------
I wonder if escape sequences are also relevant to template strings. If yes, we are probably missing a test ;-).

================
Comment at: lib/Format/Format.cpp:828
@@ +827,3 @@
+    // NB: This is not entirely correct, as an r_paren can introduce an operand
+    // location in e.g. `if (foo) /bar/.exec(...);`. That is a rare enough
+    // corner case to not matter in practice, though.
----------------
Maybe add a test to see how this goes wrong and specifically also ensure it doesn't crash.

================
Comment at: lib/Format/Format.cpp:830
@@ +829,3 @@
+    // corner case to not matter in practice, though.
+    return (Tok->isOneOf(tok::period, tok::l_paren, tok::comma, tok::l_brace,
+                         tok::r_brace, tok::l_square, tok::semi, tok::exclaim,
----------------
Remove the ().

================
Comment at: lib/Format/Format.cpp:858
@@ +857,3 @@
+    }
+    if (Prev) {
+      if (Prev->isOneOf(tok::plus, tok::plusplus, tok::minus,
----------------
I'd probably put this into the other two conditions to reduce indentation. More specifically, I'd do:

  if (PrecedesOperand(Prev)) // sink the nullptr check into the function.
    return;

  if (Prev->isOneOf(...)) {
  }

================
Comment at: lib/Format/Format.cpp:896
@@ +895,3 @@
+      case '/':
+        if (!InCharacterClass) {
+          HaveClosingSlash = true;
----------------
No braces.


http://reviews.llvm.org/D13765