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

Martin Probst via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 20 18:25:14 PDT 2016


mprobst marked an inline comment as done.

================
Comment at: lib/Format/FormatTokenLexer.cpp:231
@@ -230,3 +230,3 @@
 
 void FormatTokenLexer::tryParseTemplateString() {
   FormatToken *BacktickToken = Tokens.back();
----------------
djasper wrote:
> 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.
Done. Also, it turns out my attempt here was overly optimistic, I actually need a proper state stack to handle this correctly.

================
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.
----------------
djasper wrote:
> Maybe use "Offset[1]" instead of "*(Offset + 1)"?
Is somePointer[1] idiomatic C for "the next element after this pointer"? To me `+ 1` seemed like a better expression of intent, if a bit more wordy. Done in any case.

================
Comment at: lib/Format/TokenAnnotator.cpp:1821
@@ -1819,1 +1820,3 @@
+        (Right.is(TT_TemplateString) && Right.TokenText.startswith("}")))
+      return 100;
   }
----------------
djasper wrote:
> 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?
No, I think breaking in the template string should probably be a last resort. It's IMHO conceptually tighter than string concatenation using `+`. That being said, I have no idea what number that should translate into :-)


https://reviews.llvm.org/D22431





More information about the cfe-commits mailing list