[PATCH] D80031: [clang-format] [NFC] release note placed in the wrong location and other rst linting errors

[Kanglei Fang via Phabricator via cfe-commits]

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

Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80031



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

[PATCH] D77039: [clang-format] Don't break multi block parameters on ObjCBreakBeforeNestedBlockParam

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

In D77039#1967224 , @MyDeveloperDay 
wrote:

> what is changing in clang/docs/ClangFormatStyleOptions.rst? if nothing then 
> remove from the diff


there's an indentation issue, this changes the example of the doc from:

  - (void)_aMethod
   {
  [self.test1 t:self
   w:self
  callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
  u = c;
   }]
  }

to

  - (void)_aMethod
  {
 [self.test1 t:self
 w:self
  callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
  u = c;
  }]
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77039



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

[PATCH] D77039: [clang-format] Don't break multi block parameters on ObjCBreakBeforeNestedBlockParam

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

@jolesiak I don't yet have the commit access, do you mind help merging this 
diff? Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77039



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

[PATCH] D77039: [clang-format] Don't break multi block parameters on ObjCBreakBeforeNestedBlockParam

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 254015.
ghvg1313 added a comment.

reset diff's base


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77039

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1420,6 +1420,10 @@
"*b, NSNumber *c) {\n"
"  b = c;\n"
"}]");
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "} z:self]");
 
   Style.ColumnLimit = 80;
   verifyFormat(
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -342,6 +342,7 @@
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
 return true;
   if (Style.Language == FormatStyle::LK_ObjC &&
+  Style.ObjCBreakBeforeNestedBlockParam &&
   Current.ObjCSelectorNameParts > 1 &&
   Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
 return true;
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2072,8 +2072,8 @@
  - (void)_aMethod
  {
 [self.test1 t:self
- w:self
-callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+w:self
+ callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
  u = c;
  }]
  }


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1420,6 +1420,10 @@
"*b, NSNumber *c) {\n"
"  b = c;\n"
"}]");
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "} z:self]");
 
   Style.ColumnLimit = 80;
   verifyFormat(
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -342,6 +342,7 @@
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
 return true;
   if (Style.Language == FormatStyle::LK_ObjC &&
+  Style.ObjCBreakBeforeNestedBlockParam &&
   Current.ObjCSelectorNameParts > 1 &&
   Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
 return true;
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2072,8 +2072,8 @@
  - (void)_aMethod
  {
 [self.test1 t:self
- w:self
-callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+w:self
+ callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
  u = c;
  }]
  }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77039: [clang-format] Don't break multi block parameters on ObjCBreakBeforeNestedBlockParam

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 254014.
ghvg1313 added a comment.

- Fix documentation formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77039

Files:
  clang/docs/ClangFormatStyleOptions.rst


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2072,8 +2072,8 @@
  - (void)_aMethod
  {
 [self.test1 t:self
- w:self
-callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+w:self
+ callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
  u = c;
  }]
  }


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2072,8 +2072,8 @@
  - (void)_aMethod
  {
 [self.test1 t:self
- w:self
-callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+w:self
+ callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
  u = c;
  }]
  }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77039: [clang-format] Don't break multi block parameters on ObjCBreakBeforeNestedBlockParam

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 created this revision.
ghvg1313 added reviewers: jolesiak, benhamilton.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

While the original diff  makes a lot of sense, 
and multiple inline block parameter/trailing paramemter after inline block 
paramemter should be discouraged, the formatting result is different than what 
xcode does by default
For the exact same example provided in the original diff:

  [object
blockArgument:^{
  a = 42;
}
   anotherArg:42];

The code is hard to read and not very visually pleasing

This diff uses `ObjCBreakBeforeNestedBlockParam` to shield from the formatting
When it's set to false, don't allign the inline block paramemters.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77039

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1420,6 +1420,10 @@
"*b, NSNumber *c) {\n"
"  b = c;\n"
"}]");
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "} z:self]");
 
   Style.ColumnLimit = 80;
   verifyFormat(
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -342,6 +342,7 @@
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
 return true;
   if (Style.Language == FormatStyle::LK_ObjC &&
+  Style.ObjCBreakBeforeNestedBlockParam &&
   Current.ObjCSelectorNameParts > 1 &&
   Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
 return true;


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1420,6 +1420,10 @@
"*b, NSNumber *c) {\n"
"  b = c;\n"
"}]");
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "} z:self]");
 
   Style.ColumnLimit = 80;
   verifyFormat(
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -342,6 +342,7 @@
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
 return true;
   if (Style.Language == FormatStyle::LK_ObjC &&
+  Style.ObjCBreakBeforeNestedBlockParam &&
   Current.ObjCSelectorNameParts > 1 &&
   Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 240260.
ghvg1313 marked an inline comment as done.
ghvg1313 added a comment.



- rebase master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1398,6 +1398,37 @@
   "}");
 }
 
+TEST_F(FormatTestObjC, BreakLineBeforeNestedBlockParam) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, NSNumber *u, "
+   "NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = c;\n"
+   "} w:self callback2:^(typeof(self) self, NSNumber *a, NSNumber "
+   "*b, NSNumber *c) {\n"
+   "  b = c;\n"
+   "}]");
+
+  Style.ColumnLimit = 80;
+  verifyFormat(
+  "[self.test_method a:self b:self\n"
+  "   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+  " u = v;\n"
+  "   }]");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -498,6 +498,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -796,6 +798,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -861,8 +861,10 @@
   // Any break on this level means that the parent level has been broken
   // and we need to avoid bin packing there.
   bool NestedBlockSpecialCase =
-  !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
-  State.Stack[State.Stack.size() - 2].NestedBlockInlined;
+  (!Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
+   State.Stack[State.Stack.size() - 2].NestedBlockInlined) ||
+  (Style.Language == FormatStyle::LK_ObjC && Current.is(tok::r_brace) &&
+   State.Stack.size() > 1 && !Style.ObjCBreakBeforeNestedBlockParam);
   if (!NestedBlockSpecialCase)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
@@ -1380,7 +1382,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1669,6 +1669,29 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  /// \code
+  ///   false:
+  ///- (void)_aMethod
+  ///{
+  ///[self.test1 t:self w:self 

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 marked 2 inline comments as done.
ghvg1313 added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:164
+
+- Clang-format now supports JavaScript null operators.
+

MyDeveloperDay wrote:
> is this line part of this patch?
No, rebased again and got rid of it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 239458.
ghvg1313 added a comment.

- rebase master
- formatting code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1398,6 +1398,37 @@
   "}");
 }
 
+TEST_F(FormatTestObjC, BreakLineBeforeNestedBlockParam) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, NSNumber *u, "
+   "NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = c;\n"
+   "} w:self callback2:^(typeof(self) self, NSNumber *a, NSNumber "
+   "*b, NSNumber *c) {\n"
+   "  b = c;\n"
+   "}]");
+
+  Style.ColumnLimit = 80;
+  verifyFormat(
+  "[self.test_method a:self b:self\n"
+  "   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+  " u = v;\n"
+  "   }]");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -498,6 +498,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -796,6 +798,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -861,8 +861,10 @@
   // Any break on this level means that the parent level has been broken
   // and we need to avoid bin packing there.
   bool NestedBlockSpecialCase =
-  !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
-  State.Stack[State.Stack.size() - 2].NestedBlockInlined;
+  (!Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
+   State.Stack[State.Stack.size() - 2].NestedBlockInlined) ||
+  (Style.Language == FormatStyle::LK_ObjC && Current.is(tok::r_brace) &&
+   State.Stack.size() > 1 && !Style.ObjCBreakBeforeNestedBlockParam);
   if (!NestedBlockSpecialCase)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
@@ -1380,7 +1382,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1669,6 +1669,29 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  /// \code
+  ///   false:
+  ///- (void)_aMethod
+  ///{
+  ///[self.test1 t:self w:self callback:^(typeof(self) self, 

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

@MyDeveloperDay gentle ping for help merging


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

In D70926#1790848 , @MyDeveloperDay 
wrote:

> Thanks for the patch.


Happy new year!
Could you help landing this diff? I don't know exactly what to do from here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 marked 2 inline comments as done.
ghvg1313 added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:869
   State.Stack[i].BreakBeforeParameter = true;
 
   if (PreviousNonComment &&

ghvg1313 wrote:
> MyDeveloperDay wrote:
> > I'm a little confused by the code, this feels to me like its covering 
> > something more than a specific than just the ObjectiveC case, what is meant 
> > here by a nestedblockspecialcase? this feels like we are turning off 
> > BreakBeforeParameter by another method?
> > 
> > I get the code below, just wondering why this is also needed.
> I think you are right, bin packing is already avoided as the comment says so 
> I don't have to do anything extra.
> 
> Was trying to handle linebreaking within block as well. Not needed indeed.
Notice I get this back again, it's actually to avoid parameter break in between 
blocks, so we don't get inconsistent parameter break between blocks, for 
example: 
```
- (void)_aMethod
{
[self.test1 t:self
w:self
 callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
 u = c;
 } x:self callback2:^(typeof(self) self, NSNumber *u, NSNumber *v) {
 u = c;
 }]
}
```

Originally, the second block will trigger the parameter break in between itself 
and the previous block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 234621.
ghvg1313 added a comment.

- Handle parameter break between blocks
- Fix test cases and move them to objcTest
- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1398,6 +1398,32 @@
   "}");
 }
 
+TEST_F(FormatTestObjC, BreakLineBeforeNestedBlockParam) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+   "  u = c;\n"
+   "} w:self callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, NSNumber *c) {\n"
+   "  b = c;\n"
+   "}]");
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n"
+   "   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+   " u = v;\n"
+   "   }]");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -794,6 +796,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -861,8 +861,10 @@
   // Any break on this level means that the parent level has been broken
   // and we need to avoid bin packing there.
   bool NestedBlockSpecialCase =
-  !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
-  State.Stack[State.Stack.size() - 2].NestedBlockInlined;
+  (!Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
+  State.Stack[State.Stack.size() - 2].NestedBlockInlined) ||
+  (Style.Language == FormatStyle::LK_ObjC && Current.is(tok::r_brace)
+   && State.Stack.size() > 1 && !Style.ObjCBreakBeforeNestedBlockParam);
   if (!NestedBlockSpecialCase)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
@@ -1380,7 +1382,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1646,6 +1646,28 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  /// \code
+  ///   false:
+  ///- (void)_aMethod
+  ///{
+  ///[self.test1 t:self w:self callback:^(typeof(self) self, 

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

@MyDeveloperDay Anything I'll need to do on my end to start the landing process?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

In D70926#1769437 , @MyDeveloperDay 
wrote:

> LGTM
>
> A little advice as you fix each comment check the "Done" button.
>
> I notice you are a new user, will you need help landing it?


Thanks for the review!
And yes I do need help landing it, quite new to the community, not sure if I 
should head to github and dupe the changes or something else...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 232177.
ghvg1313 marked an inline comment as done.
ghvg1313 added a comment.

- Move documents from release notes to header and proper rst, register test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12548,6 +12548,7 @@
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
+  CHECK_PARSE_BOOL(ObjCBreakBeforeNestedBlockParam);
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);
   CHECK_PARSE_BOOL(ReflowComments);
   CHECK_PARSE_BOOL(SortIncludes);
@@ -1,6 +14445,29 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, DoNotBreakLineBeforeNestedBlockParam) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = c;\n} w:self " \
+   "callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, " \
+   "NSNumber *c) {\n  b = c;\n}]", Style);
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n" \
+   "   callback:^(typeof(self) self, NSNumber *u, " \
+   "NSNumber *v) {\n" \
+   " u = v;\n" \"   }]", Style);
+}
+
 TEST_F(FormatTest, ArrayOfTemplates) {
   EXPECT_EQ("auto a = new unique_ptr[10];",
 format("auto a = new unique_ptr [ 10];"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -792,6 +794,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1380,7 +1380,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1646,6 +1646,28 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  /// \code
+  ///   false:
+  ///- (void)_aMethod
+  ///{
+  ///[self.test1 t:self w:self callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+  ///u = c;
+  ///}]
+  ///}
+  ///true:
+  ///- (void)_aMethod
+  ///{
+  ///   [self.test1 t:self
+  ///w:self
+  ///   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+  ///u = c;
+  ///

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 231971.
ghvg1313 marked an inline comment as done.
ghvg1313 added a comment.

- rename option, remove handlers for nested block itself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1,6 +1,29 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, DoNotBreakLineBeforeNestedBlockParam) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = c;\n} w:self " \
+   "callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, " \
+   "NSNumber *c) {\n  b = c;\n}]", Style);
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n" \
+   "   callback:^(typeof(self) self, NSNumber *u, " \
+   "NSNumber *v) {\n" \
+   " u = v;\n" \"   }]", Style);
+}
+
 TEST_F(FormatTest, ArrayOfTemplates) {
   EXPECT_EQ("auto a = new unique_ptr[10];",
 format("auto a = new unique_ptr [ 10];"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -792,6 +794,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1380,7 +1380,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1646,6 +1646,10 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// \brief Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  bool ObjCBreakBeforeNestedBlockParam;
+
   /// Add a space in front of an Objective-C protocol list, i.e. use
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
@@ -2126,6 +2130,7 @@
NamespaceMacros == R.NamespaceMacros &&
ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
+   ObjCBreakBeforeNestedBlockParam == R.ObjCBreakBeforeNestedBlockParam &&
ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
PenaltyBreakAssignment == R.PenaltyBreakAssignment &&
Index: clang/docs/ReleaseNotes.rst
===
--- 

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 marked an inline comment as done.
ghvg1313 added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:869
   State.Stack[i].BreakBeforeParameter = true;
 
   if (PreviousNonComment &&

MyDeveloperDay wrote:
> I'm a little confused by the code, this feels to me like its covering 
> something more than a specific than just the ObjectiveC case, what is meant 
> here by a nestedblockspecialcase? this feels like we are turning off 
> BreakBeforeParameter by another method?
> 
> I get the code below, just wondering why this is also needed.
I think you are right, bin packing is already avoided as the comment says so I 
don't have to do anything extra.

Was trying to handle linebreaking within block as well. Not needed indeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 231790.
ghvg1313 added a comment.

- Update documents


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1,6 +1,29 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, DoNotBreakLineBeforeNestedBlockParam) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCDontBreakBeforeNestedBlockParam = true;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = c;\n} w:self " \
+   "callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, " \
+   "NSNumber *c) {\n  b = c;\n}]", Style);
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n" \
+   "   callback:^(typeof(self) self, NSNumber *u, " \
+   "NSNumber *v) {\n" \
+   " u = v;\n" \"   }]", Style);
+}
+
 TEST_F(FormatTest, ArrayOfTemplates) {
   EXPECT_EQ("auto a = new unique_ptr[10];",
 format("auto a = new unique_ptr [ 10];"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCDontBreakBeforeNestedBlockParam",
+   Style.ObjCDontBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -791,7 +793,7 @@
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
-  LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCDontBreakBeforeNestedBlockParam = false;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -863,7 +863,7 @@
   bool NestedBlockSpecialCase =
   !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
   State.Stack[State.Stack.size() - 2].NestedBlockInlined;
-  if (!NestedBlockSpecialCase)
+  if (!NestedBlockSpecialCase && !Style.ObjCDontBreakBeforeNestedBlockParam)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
 
@@ -1380,7 +1380,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+!Style.ObjCDontBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1646,6 +1646,10 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// \brief Dont break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  bool ObjCDontBreakBeforeNestedBlockParam;
+
   /// Add a space in front of an Objective-C protocol list, i.e. use
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
@@ -2126,6 +2130,7 @@
NamespaceMacros == R.NamespaceMacros &&
ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
ObjCBlockIndentWidth == 

[PATCH] D70926: Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 created this revision.
ghvg1313 added a reviewer: klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

>From `clang-format` version 3.7.0 and up, , there is no way to keep following 
>format of ObjectiveC block:

  - (void)_aMethod
  {
  [self.test1 t:self w:self callback:^(typeof(self) self, NSNumber *u, 
NSNumber *v) {
  u = c;
  }]
  }

Regardless of the change in `.clang-format` configuration file, all parameters 
will be lined up so that colons will be on the same column, like following:

  - (void)_aMethod
  {
  [self.test1 t:self
  w:self
   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
   u = c;
   }]
  }

Considering with ObjectiveC, the first code style is cleaner & more readable 
for some people, I've added a config option: 
`ObjCDontBreakBeforeNestedBlockParam` (boolean) so that if it is enable, the 
first code style will be favored.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70926

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1,6 +1,29 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, DoNotBreakLineBeforeNestedBlockParam) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCDontBreakBeforeNestedBlockParam = true;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = c;\n} w:self " \
+   "callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, " \
+   "NSNumber *c) {\n  b = c;\n}]", Style);
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n" \
+   "   callback:^(typeof(self) self, NSNumber *u, " \
+   "NSNumber *v) {\n" \
+   " u = v;\n" \"   }]", Style);
+}
+
 TEST_F(FormatTest, ArrayOfTemplates) {
   EXPECT_EQ("auto a = new unique_ptr[10];",
 format("auto a = new unique_ptr [ 10];"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCDontBreakBeforeNestedBlockParam",
+   Style.ObjCDontBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -791,7 +793,7 @@
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
-  LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCDontBreakBeforeNestedBlockParam = false;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -863,7 +863,7 @@
   bool NestedBlockSpecialCase =
   !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
   State.Stack[State.Stack.size() - 2].NestedBlockInlined;
-  if (!NestedBlockSpecialCase)
+  if (!NestedBlockSpecialCase && !Style.ObjCDontBreakBeforeNestedBlockParam)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
 
@@ -1380,7 +1380,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+!Style.ObjCDontBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h