[PATCH] clang-format: [js] Support template strings.

Daniel Jasper djasper at google.com
Fri Feb 20 00:09:15 PST 2015


================
Comment at: lib/Format/Format.cpp:796-797
@@ +795,4 @@
+      ++TokenCount;
+      IsMultiline =
+          IsMultiline || I[0]->NewlinesBefore > 0 || I[0]->IsMultiline;
+      if (I[0]->isNot(tok::unknown) || I[0]->TokenText != "`")
----------------
I think, I'd find this (very slightly) easier to read as:

  if (I[0]->IsMultiline || I[0]->NewlinesBefore > 0)
    IsMultiline = true;

================
Comment at: lib/Format/Format.cpp:811
@@ +810,3 @@
+            SourceMgr.getSpellingColumnNumber(End->Tok.getLocation()) + 1;
+        Tokens.back()->ColumnWidth = EndCol - StartCol;
+      }
----------------
Reading the comment for ColumnWidth again, it says:

  /// \brief The width of the non-whitespace parts of the token (or its first
  /// line for multi-line tokens) in columns.

So, if this turns into a multiline token, we need the length of the first line. The reason is this:

  var someLongVariableName = `aaaaaaaaa
          bbbbbbb
          cccccccc`;

While we can't do anything to the content of the template string without affecting the runtime behavior, we need to make the decision of whether or not to break after the "=". For that, we need to know the lengths of the first line of the token.

Should be reasonably easy to reproduce in a test :-).

================
Comment at: lib/Format/Format.cpp:813
@@ +812,3 @@
+      }
+      Tokens.back()->LastLineColumnWidth = LastColumn;
+      Tokens.back()->IsMultiline = IsMultiline;
----------------
I think we need to set LastLineColumnWidth only for MultiLine tokens. It should not have any meaning for single-line tokens. Setting it to LastColumn for non-multiline tokens certainly is confusing.

http://reviews.llvm.org/D7763

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list