[PATCH] D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly moreorthogonal pieces.
Krasimir Georgiev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 10 07:25:25 PST 2017
krasimir added inline comments.
================
Comment at: lib/Format/ContinuationIndenter.cpp:1028
+ unsigned Penalty =
+ handleEndOfLine(Current, State, DryRun, AllowBreak);
----------------
Why `handleEndOfLine`? Is it guaranteed that here we've reached the end of the line?
================
Comment at: lib/Format/ContinuationIndenter.cpp:1284
+ unsigned StartColumn = State.Column - Current.ColumnWidth;
+ auto Delimiter = *getRawStringDelimiter(Current.TokenText);
// The text of a raw string is between the leading 'R"delimiter(' and the
----------------
At this point, we compute `getRawStringDelimiter` twice: once here, and once in `getRawStringStyle` in the caller side. Ideally, we'd only like to compute it once. That's why it was a parameter before.
================
Comment at: lib/Format/ContinuationIndenter.cpp:1408
+std::unique_ptr<BreakableToken> ContinuationIndenter::createBreakableToken(
+ const FormatToken &Current, LineState &State, bool AllowBreak) {
unsigned StartColumn = State.Column - Current.ColumnWidth;
----------------
If `AllowBreak == false`, in which cases will this return a non-`nullptr`?
================
Comment at: unittests/Format/FormatTest.cpp:6322
"#include \"some long include\" // with a comment\n"
- "#include \"some very long include paaaaaaaaaaaaaaaaaaaaaaath\"",
+ "#include \"some very long include path\"\n"
+ "#include <some/very/long/include/path>\n",
----------------
What happened to the old test line?
================
Comment at: unittests/Format/FormatTestComments.cpp:683
" //\\\n"
- " // long 1 2 3 4\n"
- " // 5\n"
+ " // long 1 2 3 4 5\n"
"}",
----------------
Please also add a test case adding a ` 6`, which actually gets broken.
https://reviews.llvm.org/D39900
More information about the cfe-commits
mailing list