[PATCH] D98214: [clang-format] Fix aligning with linebreaks
Marek Kurdej via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 9 00:00:19 PST 2021
curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.
This looks really good. Just a few rather minor remarks.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:280-281
// In the above example, we need to take special care to ensure that
- // 'double z' is indented along with it's owning function 'b'.
+ // 'double z' is indented along with it's owning function 'b'. The same holds
+ // for calling a function:
+ // double a = foo(x);
----------------
Nit: I'd break the line before the new phrase.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:332-340
if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
(ScopeStart > Start + 1 &&
Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)) ||
+ (ScopeStart > Start + 1 &&
+ Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+ Changes[ScopeStart - 1].Tok->is(tok::l_paren)) ||
Changes[i].Tok->is(TT_ConditionalExpr) ||
----------------
Would it be possible to break up this condition and name it (or name its parts)? It's getting hard to follow.
Suggestion according to my understanding, which might be wrong.
================
Comment at: clang/unittests/Format/FormatTest.cpp:14305-14311
+ verifyFormat("void foo() {\n"
+ " int myVar = 5;\n"
+ " double x = 3.14;\n"
+ " auto str = \"Hello \"\n"
+ " \"World\";\n"
+ "}",
+ Style);
----------------
Nice tests. I'd like to see however the behaviour when there's another assignment/declaration after a multi-line string, maybe even mixing multi-line strings and multi-line function calls. You already do test multiple multi-line function calls (with `newEntry` and `newEntry2`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98214/new/
https://reviews.llvm.org/D98214
More information about the cfe-commits
mailing list