================ 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 != "`") ---------------- djasper wrote: > I think, I'd find this (very slightly) easier to read as: > > if (I[0]->IsMultiline || I[0]->NewlinesBefore > 0) > IsMultiline = true; Done.
================ Comment at: lib/Format/Format.cpp:798 @@ +797,3 @@ + IsMultiline || I[0]->NewlinesBefore > 0 || I[0]->IsMultiline; + if (I[0]->isNot(tok::unknown) || I[0]->TokenText != "`") + continue; ---------------- djasper wrote: > Can you add a test with two template strings? I think that might do the wrong > thing as you need to abort when you find a TT_TemplateString. It's not strictly needed - if there was a preceding template string, it cannot just equals "`", it must have at least opener and closer "``". But it's a reasonable optimization and IMHO adds clarity. ================ Comment at: lib/Format/Format.cpp:811 @@ +810,3 @@ + SourceMgr.getSpellingColumnNumber(End->Tok.getLocation()) + 1; + Tokens.back()->ColumnWidth = EndCol - StartCol; + } ---------------- djasper wrote: > 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 :-). As discussed offline, this changed quite a bit. PTAL. ================ Comment at: lib/Format/Format.cpp:813 @@ +812,3 @@ + } + Tokens.back()->LastLineColumnWidth = LastColumn; + Tokens.back()->IsMultiline = IsMultiline; ---------------- djasper wrote: > 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. Done. http://reviews.llvm.org/D7763 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
