[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:2520
+  /// readability to have the signature indented two levels and to use
+  /// ``OuterScope``. The KJ style guide requires ``OuterScope`.
+  /// `KJ style guide

Here is a ` missing, I'll fix it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG64cf5eba06bd: [clang-format] Add new LambdaBodyIndentation 
option (authored by vlovich, committed by HazardyKnusperkeks).

Changed prior to commit:
  https://reviews.llvm.org/D102706?vs=347152=353753#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18531,6 +18531,7 @@
" aaa;\n"
"});",
getLLVMStyleWithColumns(60));
+
   verifyFormat("SomeFunction({[&] {\n"
"// comment\n"
"  },\n"
@@ -19083,6 +19084,117 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(100);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = "
+"std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) "
+"mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = "
+   "std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }).foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("std::sort(v.begin(), v.end(),\n"
+   "  [](const auto , const auto "
+   ") {\n"
+   "  return someLongArgumentName.someMemberVariable < "
+   "someOtherLongArgumentName.someMemberVariable;\n"
+   "});",
+   Style);
+  verifyFormat("test() {\n"
+   "  (\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  },\n"
+   "  foo, bar)\n"
+   "  .foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  })\n"
+   "  .foo()\n"
+   "  .bar();\n"
+   "}",
+   Style);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = "
+"std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { "
+"result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = "
+   "std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then([this, ] {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { "
+"result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult 

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

I think you can go ahead & commit it for me. I don't know when I'll get commit 
access.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D102706#2823087 , @vlovich wrote:

> I don't think I have permissions. Happy to do it if I'm given permissions 
> (I'm assuming the instructions are the general LLVM ones). Otherwise:
>
> Name: Vitali Lovich
> E-mail: vlov...@gmail.com

You can apply for commit access, yes that's for whole of LLVM, but for me it 
was no problem to get that right.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

I don't know how to commit. Happy to do it if I have the permissions & 
appropriate instructions.

Name: Vitali Lovich
E-mail: vlov...@gmail.com


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D102706#2787550 , 
@HazardyKnusperkeks wrote:

> LGTM, but please wait for more responses.

No one objected, so I declare it ready to commit. Do you need some one to do 
it? If yes please post name and email for the commit.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

LGTM, but please wait for more responses.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich marked an inline comment as done.
vlovich added a comment.

I think all review comments have been addressed. Please let me know if there's 
anything else blocking merge.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I personally like to keep clang-format and clang-format tests clang formatted 
all the time so we have a reasonable set of "clang-format-clean" code we can 
sanity check any change against, the more of LLVM that can be clean the better 
our testing can be.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2790
 
+**LambdaBodyIndentation** (``LambdaBodyIndentationKind``)
+  What is the lambda body indented relative to (default is ``Signature``).

vlovich wrote:
> MyDeveloperDay wrote:
> > Did you generate this by hand or using the clang/doc/tools/dump_style.py?
> By hand. `dump_format_style` generates other changes (although I wasn't aware 
> of it at the time). I'll regenerate just this section using that tool.
it shouldn't, that probably means someone has made a change without keeping 
them consistent..I'll take a look at that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich updated this revision to Diff 347152.
vlovich added a comment.

Regen documentation & clang-format test cases.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17871,6 +17871,7 @@
" aaa;\n"
"});",
getLLVMStyleWithColumns(60));
+
   verifyFormat("SomeFunction({[&] {\n"
"// comment\n"
"  },\n"
@@ -18423,6 +18424,117 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(100);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = "
+"std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) "
+"mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = "
+   "std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }).foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("std::sort(v.begin(), v.end(),\n"
+   "  [](const auto , const auto "
+   ") {\n"
+   "  return someLongArgumentName.someMemberVariable < "
+   "someOtherLongArgumentName.someMemberVariable;\n"
+   "});",
+   Style);
+  verifyFormat("test() {\n"
+   "  (\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  },\n"
+   "  foo, bar)\n"
+   "  .foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  })\n"
+   "  .foo()\n"
+   "  .bar();\n"
+   "}",
+   Style);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = "
+"std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { "
+"result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = "
+   "std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then([this, ] {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { "
+"result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, ] {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) 

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

In D102706#2774010 , @MyDeveloperDay 
wrote:

> Is ```Signature``` effectively as is?

Yes. I noted that it's the default behavior. Please let me know if I can 
clarify the documentation wording somehow (or if you have preferred wording you 
want me to use)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

In D102706#2774018 , @MyDeveloperDay 
wrote:

> Could you clang-format the tests please so I can more easily read them.

Sure. I was just taking my queue from the rest of the file which appeared to 
intentionally not clang-format the test strings.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2790
 
+**LambdaBodyIndentation** (``LambdaBodyIndentationKind``)
+  What is the lambda body indented relative to (default is ``Signature``).

MyDeveloperDay wrote:
> Did you generate this by hand or using the clang/doc/tools/dump_style.py?
By hand. `dump_format_style` generates other changes (although I wasn't aware 
of it at the time). I'll regenerate just this section using that tool.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you clang-format the tests please so I can more easily read them.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2790
 
+**LambdaBodyIndentation** (``LambdaBodyIndentationKind``)
+  What is the lambda body indented relative to (default is ``Signature``).

Did you generate this by hand or using the clang/doc/tools/dump_style.py?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Is ```Signature``` effectively as is?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich updated this revision to Diff 346869.
vlovich added a comment.
Herald added a subscriber: mgrang.

Review response

- Add examples to documentation + links to KJ style guide.
- Add better header doc
- Add more test cases
- Add release notes
- Note current "problem" test with a TODO (corner case that's not critical for 
KJ).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17871,6 +17871,7 @@
" aaa;\n"
"});",
getLLVMStyleWithColumns(60));
+
   verifyFormat("SomeFunction({[&] {\n"
"// comment\n"
"  },\n"
@@ -18423,6 +18424,105 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(100);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, ](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }).foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("std::sort(v.begin(), v.end(),\n"
+   "  [](const auto , const auto ) {\n"
+   "  return someLongArgumentName.someMemberVariable < someOtherLongArgumentName.someMemberVariable;\n"
+   "});",
+   Style);
+  verifyFormat("test() {\n"
+   "  (\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  },\n"
+   "  foo, bar)\n"
+   "  .foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  })\n"
+   "  .foo()\n"
+   "  .bar();\n"
+   "}",
+   Style);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, ](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then([this, ] {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, ] {\n"
+   "return someObject.startAsyncAction().then([this, ](AsyncActionResult result) mutable {\n"
+  

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

> verifyFormat("test() {\n"
>
>   "  ([]() -> {\n"
>   "int b = 32;\n"
>   "return 3;\n"
>   "  }).foo();\n"
>   "}",
>   Style);
>
>   There you have parenthesis around the lambda, how about without?
>   Maybe just something like
>
> std::sort(v.begin(), v.end(), [](const auto& lhs, const auto& rhs) { return 
> lhs.Foo < rhs.Foo; });
>
>   

Ah. Ok. What I had done was run all the tests with this flag & note the ones 
where the formatting changed & copied those. The one without parenthesis didn't 
do anything interesting which is why I omitted it originally. I'll add it back 
+ your sort example too. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D102706#2766871 , @vlovich wrote:

> In D102706#2766680 , 
> @HazardyKnusperkeks wrote:
>
>> Maybe a bit more test cases with smaller lambdas? Or without the outer 
>> parenthesis?
>
> I'm not sure I understand this comment. Which test case are you referring to 
> by "or without the outer parenthesis"?



  verifyFormat("test() {\n"
 "  ([]() -> {\n"
 "int b = 32;\n"
 "return 3;\n"
 "  }).foo();\n"
 "}",
 Style);

There you have parenthesis around the lambda, how about without?
Maybe just something like

  std::sort(v.begin(), v.end(), [](const auto& lhs, const auto& rhs) { return 
lhs.Foo < rhs.Foo; });


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

In D102706#2766680 , 
@HazardyKnusperkeks wrote:

> Maybe a bit more test cases with smaller lambdas? Or without the outer 
> parenthesis?

I'm not sure I understand this comment. Which test case are you referring to by 
"or without the outer parenthesis"?




Comment at: clang/include/clang/Format/Format.h:2456
+  enum LambdaBodyIndentationKind : unsigned char {
+/// Align lambda body relative to the lambda signature. This is the 
default.
+LBI_Signature,

HazardyKnusperkeks wrote:
> I think you should add examples, because for me it is not 100% what's the 
> difference is.
Good idea. Meant to do that & forgot. In terms of an example, do  you find the 
test case or description in the phabricator message useful or are you looking 
for something else?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Maybe a bit more test cases with smaller lambdas? Or without the outer 
parenthesis?




Comment at: clang/include/clang/Format/Format.h:2456
+  enum LambdaBodyIndentationKind : unsigned char {
+/// Align lambda body relative to the lambda signature. This is the 
default.
+LBI_Signature,

I think you should add examples, because for me it is not 100% what's the 
difference is.



Comment at: clang/include/clang/Format/Format.h:2462
+
+  LambdaBodyIndentationKind LambdaBodyIndentation;
+

Here should also a comment be added.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

I'm not 100% certain if the implementation is absolutely correct. For example, 
the `#define A` test case is broken (the closing brace is on the wrong line). 
Additionally, the `pop_back` feels weird but seems to fix all the other cases 
to make the closing brace aligned properly. Maybe some/all of the 
implementation belongs in ContinuationIndenter.cpp instead but I couldn't 
really follow where the best place to do that might be. I can also just leave 
the preprocessor macro case as a known issue for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102706/new/

https://reviews.llvm.org/D102706

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich created this revision.
vlovich added reviewers: MyDeveloperDay, curdeius.
vlovich added a project: clang-format.
vlovich requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently the lambda body indents relative to where the lambda signature is 
located. This instead lets the user
choose to align the lambda body relative to the parent scope that contains the 
lambda declaration. Thus:

  someFunction([] {
lambdaBody();
  });

will always have the same indentation of the body even when the lambda 
signature goes on a new line:

  someFunction(
  [] {
lambdaBody();
  });

whereas before `lambdaBody` would be indented 6 spaces.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102706

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17871,6 +17871,7 @@
" aaa;\n"
"});",
getLLVMStyleWithColumns(60));
+
   verifyFormat("SomeFunction({[&] {\n"
"// comment\n"
"  },\n"
@@ -18423,6 +18424,90 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(100);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, ](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }).foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  (\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  },\n"
+   "  foo, bar)\n"
+   "  .foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  })\n"
+   "  .foo()\n"
+   "  .bar();\n"
+   "}",
+   Style);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, ](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then([this, ] {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, ] {\n"
+   "return someObject.startAsyncAction().then([this, ](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+