[PATCH] D22431: clang-format: [JS] nested and tagged template strings.

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 18 07:00:57 PDT 2016


djasper added inline comments.

================
Comment at: lib/Format/FormatTokenLexer.cpp:231
@@ -230,3 +230,3 @@
 
 void FormatTokenLexer::tryParseTemplateString() {
   FormatToken *BacktickToken = Tokens.back();
----------------
I think, this could now use an elaborate comment on what it is actually trying to parse. And part of that should probably make it into the patch description.

================
Comment at: lib/Format/FormatTokenLexer.cpp:236
@@ -234,1 +235,3 @@
+  else if (!BacktickToken->is(tok::unknown) ||
+             BacktickToken->TokenText != "`")
     return;
----------------
Indent seems off.

================
Comment at: lib/Format/FormatTokenLexer.cpp:246
@@ +245,3 @@
+    if (Offset + 1 < Lex->getBuffer().end() && *Offset == '$' &&
+        *(Offset + 1) == '{') {
+      // '${' introduces an expression interpolation in the template string.
----------------
Maybe use "Offset[1]" instead of "*(Offset + 1)"?

================
Comment at: lib/Format/TokenAnnotator.cpp:1821
@@ -1819,1 +1820,3 @@
+        (Right.is(TT_TemplateString) && Right.TokenText.startswith("}")))
+      return 100;
   }
----------------
mprobst wrote:
> I've chosen 100 by fair dice roll. Please advise on how to pick a reasonable penalty :-)
In what cases would you actually want to put a line break here? Presumably, if otherwise the column limit is violated. Are there any other cases?


https://reviews.llvm.org/D22431





More information about the cfe-commits mailing list