[PATCH] D27382: Add a new clang-format style option ObjCBlockResetsIndent.
yixiang added a reviewer: djasper. yixiang added a subscriber: cfe-commits. yixiang updated this revision to Diff 80174. https://reviews.llvm.org/D27382 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -322,6 +322,7 @@ IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep); IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation); IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth); +IO.mapOptional("ObjCBlockResetsIndent", Style.ObjCBlockResetsIndent); IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty); IO.mapOptional("ObjCSpaceBeforeProtocolList", Style.ObjCSpaceBeforeProtocolList); Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -1037,6 +1037,13 @@ } void ContinuationIndenter::moveStateToNewBlock(LineState &State) { + // For ObjC blocks, reset the indent to that of the owner block if the style + // tells us to do so. + if (Style.ObjCBlockResetsIndent && State.NextToken->is(TT_ObjCBlockLBrace)) { +State.Stack.back().Indent = State.Stack.front().Indent; +State.Stack.back().NestedBlockIndent = +State.Stack.front().NestedBlockIndent; + } unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent; // ObjC block sometimes follow special indentation rules. unsigned NewIndent = Index: include/clang/Format/Format.h === --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -500,6 +500,9 @@ /// \brief The number of characters to use for indentation of ObjC blocks. unsigned ObjCBlockIndentWidth; + /// \brief Whether ObjC blocks resets the indent to that of its owner block. + bool ObjCBlockResetsIndent = false; + /// \brief Add a space after ``@property`` in Objective-C, i.e. use /// ``@property (readonly)`` instead of ``@property(readonly)``. bool ObjCSpaceAfterProperty; Index: docs/ClangFormatStyleOptions.rst === --- docs/ClangFormatStyleOptions.rst +++ docs/ClangFormatStyleOptions.rst @@ -618,6 +618,9 @@ **ObjCBlockIndentWidth** (``unsigned``) The number of characters to use for indentation of ObjC blocks. +**ObjCBlockResetsIndent** (``bool``) + Whether ObjC blocks resets the indent to that of its owner block. + **ObjCSpaceAfterProperty** (``bool``) Add a space after ``@property`` in Objective-C, i.e. use ``@property (readonly)`` instead of ``@property(readonly)``. Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -322,6 +322,7 @@ IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep); IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation); IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth); +IO.mapOptional("ObjCBlockResetsIndent", Style.ObjCBlockResetsIndent); IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty); IO.mapOptional("ObjCSpaceBeforeProtocolList", Style.ObjCSpaceBeforeProtocolList); Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -1037,6 +1037,13 @@ } void ContinuationIndenter::moveStateToNewBlock(LineState &State) { + // For ObjC blocks, reset the indent to that of the owner block if the style + // tells us to do so. + if (Style.ObjCBlockResetsIndent && State.NextToken->is(TT_ObjCBlockLBrace)) { +State.Stack.back().Indent = State.Stack.front().Indent; +State.Stack.back().NestedBlockIndent = +State.Stack.front().NestedBlockIndent; + } unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent; // ObjC block sometimes follow special indentation rules. unsigned NewIndent = Index: include/clang/Format/Format.h === --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -500,6 +500,9 @@ /// \brief The number of characters to use for indentation of ObjC blocks. unsigned ObjCBlockIndentWidth; + /// \brief Whether ObjC blocks resets the indent to that of its owner block. + bool ObjCBlockResetsIndent = false; + /// \brief Add a space after ``@property`` in Objective-C, i.e. use /// ``@property (readonly)`` instead of ``@property(readonly)``. bool ObjCSpaceAfterProperty; Index: docs/ClangFormatStyleOptions.rst
[PATCH] D27383: Add a new clang-format style option ObjCBlockResetsIndent.
yixiang created this revision. yixiang added a reviewer: djasper. yixiang added subscribers: cfe-commits, klimek. Add a new clang-format style option ObjCBlockResetsIndent of type bool. It defaults to false. When true, ObjC blocks resets indentation to that of its owner block. The resetting behavior is often desirable, as it leaves more columns for the content of the ObjC block. E.g. for an unformatted file test.m like this: - (void)test { [self callAVeryVeryVeryVeryVeryVeryLongAsyncMethodWith:@"Some param" completionHandler:^() { [self callAnotherLongMethodHereWith:@"param"]; }]; } The formatted result with ObjCBlockResetsIndent=false (or omitted) is this: // clang-format -style='{BasedOnStyle: Google, ObjCBlockResetsIndent: false}' test.m // OR // clang-format -style='{BasedOnStyle: Google}' test.m - (void)test { [self callAVeryVeryVeryVeryVeryVeryLongAsyncMethodWith:@"Some param" completionHandler:^() { [self callAnotherLongMethodHereWith: @"param"]; }]; } The formatted result with ObjCBlockResetsIndent=true is this: // clang-format -style='{BasedOnStyle: Google, ObjCBlockResetsIndent: true}' test.m - (void)test { [self callAVeryVeryVeryVeryVeryVeryLongAsyncMethodWith:@"Some param" completionHandler:^() { [self callAnotherLongMethodHereWith:@"param"]; }]; } https://reviews.llvm.org/D27383 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -322,6 +322,7 @@ IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep); IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation); IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth); +IO.mapOptional("ObjCBlockResetsIndent", Style.ObjCBlockResetsIndent); IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty); IO.mapOptional("ObjCSpaceBeforeProtocolList", Style.ObjCSpaceBeforeProtocolList); Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -1037,6 +1037,13 @@ } void ContinuationIndenter::moveStateToNewBlock(LineState &State) { + // For ObjC blocks, reset the indent to that of the owner block if the style + // tells us to do so. + if (Style.ObjCBlockResetsIndent && State.NextToken->is(TT_ObjCBlockLBrace)) { +State.Stack.back().Indent = State.Stack.front().Indent; +State.Stack.back().NestedBlockIndent = +State.Stack.front().NestedBlockIndent; + } unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent; // ObjC block sometimes follow special indentation rules. unsigned NewIndent = Index: include/clang/Format/Format.h === --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -500,6 +500,9 @@ /// \brief The number of characters to use for indentation of ObjC blocks. unsigned ObjCBlockIndentWidth; + /// \brief Whether ObjC blocks resets the indent to that of its owner block. + bool ObjCBlockResetsIndent = false; + /// \brief Add a space after ``@property`` in Objective-C, i.e. use /// ``@property (readonly)`` instead of ``@property(readonly)``. bool ObjCSpaceAfterProperty; Index: docs/ClangFormatStyleOptions.rst === --- docs/ClangFormatStyleOptions.rst +++ docs/ClangFormatStyleOptions.rst @@ -618,6 +618,9 @@ **ObjCBlockIndentWidth** (``unsigned``) The number of characters to use for indentation of ObjC blocks. +**ObjCBlockResetsIndent** (``bool``) + Whether ObjC blocks resets the indent to that of its owner block. + **ObjCSpaceAfterProperty** (``bool``) Add a space after ``@property`` in Objective-C, i.e. use ``@property (readonly)`` instead of ``@property(readonly)``. Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -322,6 +322,7 @@ IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep); IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation); IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth); +IO.mapOptional("ObjCBlockResetsIndent", Style.ObjCBlockResetsIndent); IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty); IO.mapOptional("ObjCSpaceBeforeProtocolList", Style.ObjCSpaceBeforeProtocolList); Index: l
[PATCH] D27382: [OBSOLETE] Add a new clang-format style option ObjCBlockResetsIndent.
yixiang abandoned this revision. yixiang added a comment. Sending as another diff using arcanist. This one doesn't have proper file context for reviewing. https://reviews.llvm.org/D27382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27383: Add a new clang-format style option ObjCBlockResetsIndent.
yixiang updated this revision to Diff 80176. yixiang added a comment. - Added unit test for ObjCBlockResetsIndent. https://reviews.llvm.org/D27383 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -11263,6 +11263,128 @@ FourIndent); } +TEST_F(FormatTest, FormatsBlocksWithIndentResetting) { + FormatStyle ShortBlocks = getLLVMStyle(); + ShortBlocks.AllowShortBlocksOnASingleLine = true; + ShortBlocks.ObjCBlockResetsIndent = true; + + verifyFormat("int (^Block)(int, int);", ShortBlocks); + verifyFormat("int (^Block1)(int, int) = ^(int i, int j)", ShortBlocks); + verifyFormat("void (^block)(int) = ^(id test) { int i; };", ShortBlocks); + verifyFormat("void (^block)(int) = ^(int test) { int i; };", ShortBlocks); + verifyFormat("void (^block)(int) = ^id(int test) { int i; };", ShortBlocks); + verifyFormat("void (^block)(int) = ^int(int test) { int i; };", ShortBlocks); + + verifyFormat("foo(^{ bar(); });", ShortBlocks); + verifyFormat("foo(a, ^{ bar(); });", ShortBlocks); + verifyFormat("{ void (^block)(Object *x); }", ShortBlocks); + + verifyFormat("[operation setCompletionBlock:^{\n" + " [self onOperationDone];\n" + "}];"); + verifyFormat("int i = {[operation setCompletionBlock:^{\n" + " [self onOperationDone];\n" + "}]};"); + + verifyFormat("[operation setCompletionBlock:^(int *i) {\n" + " f();\n" + "}];"); + + verifyFormat("int a = [operation block:^int(int *i) {\n" + " return 1;\n" + "}];"); + + FormatStyle ResetsIndent = getLLVMStyle(); + ResetsIndent.ObjCBlockResetsIndent = true; + verifyFormat("[myObject doSomethingWith:arg1\n" + " aaa:^int(int *a) {\n" + " return 1;\n" + "}\n" + " bbb:f(a * )];", + ResetsIndent); + + verifyFormat("[operation setCompletionBlock:^{\n" + " [self.delegate newDataAvailable];\n" + "}];", + ResetsIndent); + verifyFormat("dispatch_async(_fileIOQueue, ^{\n" + " NSString *path = [self sessionFilePath];\n" + " if (path) {\n" + "// ...\n" + " }\n" + "});", + ResetsIndent); + verifyFormat("[[SessionService sharedService]\n" + "loadWindowWithCompletionBlock:^(SessionWindow *window) {\n" + " if (window) {\n" + "[self windowDidLoad:window];\n" + " } else {\n" + "[self errorLoadingWindow];\n" + " }\n" + "}];", + ResetsIndent); + verifyFormat("void (^largeBlock)(void) = ^{\n" + " // ...\n" + "};\n", + ResetsIndent); + + FormatStyle ResetsIndent60 = getLLVMStyleWithColumns(60); + ResetsIndent60.ObjCBlockResetsIndent = true; + verifyFormat("[[SessionService sharedService]\n" + "loadWindowWithCompletionBlock: //\n" + "^(SessionWindow *window) {\n" + " if (window) {\n" + "[self windowDidLoad:window];\n" + " } else {\n" + "[self errorLoadingWindow];\n" + " }\n" + "}];", + ResetsIndent60); + + verifyFormat("[myObject doSomethingWith:arg1\n" + "firstBlock:^(Foo *a) {\n" + " // ...\n" + " int i;\n" + "}\n" + "secondBlock:^(Bar *b) {\n" + " // ...\n" + " int i;\n" + "}\n" + "thirdBlock:^Foo(Bar *b) {\n" + " // ...\n" + " int i;\n" + "}];", + ResetsIndent); + verifyFormat("[myObject doSomethingWith:arg1\n" + " firstBlock:-1\n" + " secondBlock:^(Bar *b) {\n" + " // ...\n" + " int i;\n" + "}];", + ResetsIndent); + + verifyFormat("f(^{\n" + " @autoreleasepool {\n" + "if (a) {\n" + " g();\n" + "}\n" + " }\n" + "});", + ResetsIndent); + verifyFormat("Block b = ^int *(A *a, B *b) {}", ResetsIndent); + verifyFormat("BOOL (^aaa)(void) = ^BOOL {\n" + "};", + ResetsIndent); + + FormatStyle FourIndent = getLLVMStyle(); + FourIndent.ObjCBlockIndentWidth = 4; + FourIndent.ObjCBlockResetsIndent = true; + verifyFormat("[operation setComp
[PATCH] D27383: Add a new clang-format style option ObjCBlockResetsIndent.
yixiang added a comment. > Can you add tests in unittests/Format/FormatTest.cpp (probably next to > TEST_F(FormatTest, FormatsBlocks) {..})? > What's the behavior when there is more than one block? Unit tests added. > Also note that we have some requirements for additional options (mainly to > ensure long-term maintainability): > > http://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options Thanks for pointing that out to me. https://reviews.llvm.org/D27383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27383: Add a new clang-format style option ObjCBlockResetsIndent.
yixiang updated this revision to Diff 80192. yixiang added a comment. - Don't reset indent for statements with multiple blocks. https://reviews.llvm.org/D27383 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -11263,6 +11263,128 @@ FourIndent); } +TEST_F(FormatTest, FormatsBlocksWithIndentResetting) { + FormatStyle ShortBlocks = getLLVMStyle(); + ShortBlocks.AllowShortBlocksOnASingleLine = true; + ShortBlocks.ObjCBlockResetsIndent = true; + + verifyFormat("int (^Block)(int, int);", ShortBlocks); + verifyFormat("int (^Block1)(int, int) = ^(int i, int j)", ShortBlocks); + verifyFormat("void (^block)(int) = ^(id test) { int i; };", ShortBlocks); + verifyFormat("void (^block)(int) = ^(int test) { int i; };", ShortBlocks); + verifyFormat("void (^block)(int) = ^id(int test) { int i; };", ShortBlocks); + verifyFormat("void (^block)(int) = ^int(int test) { int i; };", ShortBlocks); + + verifyFormat("foo(^{ bar(); });", ShortBlocks); + verifyFormat("foo(a, ^{ bar(); });", ShortBlocks); + verifyFormat("{ void (^block)(Object *x); }", ShortBlocks); + + verifyFormat("[operation setCompletionBlock:^{\n" + " [self onOperationDone];\n" + "}];"); + verifyFormat("int i = {[operation setCompletionBlock:^{\n" + " [self onOperationDone];\n" + "}]};"); + + verifyFormat("[operation setCompletionBlock:^(int *i) {\n" + " f();\n" + "}];"); + + verifyFormat("int a = [operation block:^int(int *i) {\n" + " return 1;\n" + "}];"); + + FormatStyle ResetsIndent = getLLVMStyle(); + ResetsIndent.ObjCBlockResetsIndent = true; + verifyFormat("[myObject doSomethingWith:arg1\n" + " aaa:^int(int *a) {\n" + " return 1;\n" + "}\n" + " bbb:f(a * )];", + ResetsIndent); + + verifyFormat("[operation setCompletionBlock:^{\n" + " [self.delegate newDataAvailable];\n" + "}];", + ResetsIndent); + verifyFormat("dispatch_async(_fileIOQueue, ^{\n" + " NSString *path = [self sessionFilePath];\n" + " if (path) {\n" + "// ...\n" + " }\n" + "});", + ResetsIndent); + verifyFormat("[[SessionService sharedService]\n" + "loadWindowWithCompletionBlock:^(SessionWindow *window) {\n" + " if (window) {\n" + "[self windowDidLoad:window];\n" + " } else {\n" + "[self errorLoadingWindow];\n" + " }\n" + "}];", + ResetsIndent); + verifyFormat("void (^largeBlock)(void) = ^{\n" + " // ...\n" + "};\n", + ResetsIndent); + + FormatStyle ResetsIndent60 = getLLVMStyleWithColumns(60); + ResetsIndent60.ObjCBlockResetsIndent = true; + verifyFormat("[[SessionService sharedService]\n" + "loadWindowWithCompletionBlock: //\n" + "^(SessionWindow *window) {\n" + " if (window) {\n" + "[self windowDidLoad:window];\n" + " } else {\n" + "[self errorLoadingWindow];\n" + " }\n" + "}];", + ResetsIndent60); + + verifyFormat("[myObject doSomethingWith:arg1\n" + "firstBlock:^(Foo *a) {\n" + " // ...\n" + " int i;\n" + "}\n" + "secondBlock:^(Bar *b) {\n" + " // ...\n" + " int i;\n" + "}\n" + "thirdBlock:^Foo(Bar *b) {\n" + " // ...\n" + " int i;\n" + "}];", + ResetsIndent); + verifyFormat("[myObject doSomethingWith:arg1\n" + " firstBlock:-1\n" + " secondBlock:^(Bar *b) {\n" + " // ...\n" + " int i;\n" + "}];", + ResetsIndent); + + verifyFormat("f(^{\n" + " @autoreleasepool {\n" + "if (a) {\n" + " g();\n" + "}\n" + " }\n" + "});", + ResetsIndent); + verifyFormat("Block b = ^int *(A *a, B *b) {}", ResetsIndent); + verifyFormat("BOOL (^aaa)(void) = ^BOOL {\n" + "};", + ResetsIndent); + + FormatStyle FourIndent = getLLVMStyle(); + FourIndent.ObjCBlockIndentWidth = 4; + FourIndent.ObjCBlockResetsI
[PATCH] D27383: Add a new clang-format style option ObjCBlockResetsIndent.
yixiang added a comment. Discover a case where my code breaks. When the block signature has a return type in it, the indent doesn't gets reset properly. I need to fix this. // clang-format -style='{ObjCBlockResetsIndent: true}' test.m [self callAsyncMethodWithParam:2 withCompletionHandler:id ^ (BOOL success) { if (true) { return nil; } else { return nil; } }]; Expected result // clang-format -style='{ObjCBlockResetsIndent: true}' test.m [self callAsyncMethodWithParam:2 withCompletionHandler:id ^ (BOOL success) { if (true) { return nil; } else { return nil; } }]; https://reviews.llvm.org/D27383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27383: Add a new clang-format style option ObjCBlockResetsIndent.
yixiang added a comment. Ping. For the additional requirements of adding a style options: - be used in a project of significant size (have dozens of contributors) - Yes, it's been used by almost all the iOS projects inside Google, including Google Maps. - have a publicly accessible style guide - https://google.github.io/styleguide/objcguide.xml - have a person willing to contribute and maintain patches - I'm willing to do the maintenance, I'm confident that I can find backup contributors too. https://reviews.llvm.org/D27383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits