[PATCH] D98214: [clang-format] Fix aligning with linebreaks

[Marek Kurdej via Phabricator via cfe-commits]

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