ksuther created this revision.
ksuther added a reviewer: djasper.
ksuther added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
Changes to clang-format's Objective-C block formatting over the past year have
made clang-format's output deviate from what is expected (in my opinion).
There are three changes in particular:
- Method calls with multiple block parameters has each block indented further in
- Nested blocks get indented one level in
- Method calls that end with a block parameter always get forced to a new line
and indented
The end of this message has examples of formatting with and without these
changes.
This patch undoes one revision which causes methods with multiple block
parameters to indent for each parameter (r236598) and adds two new options.
AllowNewlineBeforeBlockParameter makes the change in r235580 optional.
IndentNestedBlocks makes the change in r234304 optional.
Some relevant Bugzilla issues:
https://llvm.org/bugs/show_bug.cgi?id=23585
https://llvm.org/bugs/show_bug.cgi?id=23317
This patch came out of a discussion here
https://github.com/square/spacecommander/issues/33, where we're using a custom
version of clang-format with these options added. Based on the Bugzilla issues,
it seems that we're not alone.
Thanks!
---
Current trunk:
bin/clang-format -style="{BasedOnStyle: WebKit, ColumnLimit: 0, IndentWidth:
4}" ~/clang-format.m
```
- (void)test
{
[self block:^(void) {
doStuff();
}
completionHandler:^(void) {
doStuff();
[self block:^(void) {
doStuff();
}
completionHandler:^(void) {
doStuff();
}];
}];
[[SessionService sharedService] loadWindow:aWindow
completionBlock:^(SessionWindow* window) {
if (window) {
[self doStuff];
}
}];
}
```
With this patch applied:
bin/clang-format -style="{BasedOnStyle: WebKit, ColumnLimit: 0, IndentWidth: 4,
IndentNestedBlocks: false, AllowNewlineBeforeBlockParameter:
false}"~/clang-format.m
```
- (void)test
{
[self block:^(void) {
doStuff();
} completionHandler:^(void) {
doStuff();
[self block:^(void) {
doStuff();
} completionHandler:^(void) {
doStuff();
}];
}];
[[SessionService sharedService] loadWindow:aWindow
completionBlock:^(SessionWindow* window) {
if (window) {
[self doStuff];
}
}];
}
```
http://reviews.llvm.org/D17700
Files:
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
@@ -7606,13 +7606,6 @@
" aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa |\n"
" aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa];");
- // FIXME: This violates the column limit.
- verifyFormat(
- "[aaaaaaaaaaaaaaaaaaaaaaaaa\n"
- " aaaaaaaaaaaaaaaaa:aaaaaaaa\n"
- " aaa:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa];",
- getLLVMStyleWithColumns(60));
-
// Variadic parameters.
verifyFormat(
"NSArray *myStrings = [NSArray stringarray:@\"a\", @\"b\", nil];");
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -296,6 +296,10 @@
IO.mapOptional("IndentWidth", Style.IndentWidth);
IO.mapOptional("IndentWrappedFunctionNames",
Style.IndentWrappedFunctionNames);
+ IO.mapOptional("IndentNestedBlocks",
+ Style.IndentNestedBlocks);
+ IO.mapOptional("AllowNewlineBeforeBlockParameter",
+ Style.AllowNewlineBeforeBlockParameter);
IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -510,6 +514,8 @@
{".*", 1}};
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.IndentWrappedFunctionNames = false;
+ LLVMStyle.IndentNestedBlocks = true;
+ LLVMStyle.AllowNewlineBeforeBlockParameter = true;
LLVMStyle.IndentWidth = 2;
LLVMStyle.TabWidth = 8;
LLVMStyle.MaxEmptyLinesToKeep = 1;
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -178,9 +178,6 @@
((Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All) ||
Style.BreakConstructorInitializersBeforeComma || Style.ColumnLimit != 0))
return true;
- if (Current.is(TT_SelectorName) && State.Stack.back().ObjCSelectorNameFound &&
- State.Stack.back().BreakBeforeParameter)
- return true;
unsigned NewLineColumn = getNewLineColumn(State);
if (Current.isMemberAccess() && Style.ColumnLimit != 0 &&
@@ -234,6 +231,9 @@
State.Stack.back().FirstLessLess == 0)
return true;
+ if (Current.is(TT_SelectorName) && State.Stack.back().ObjCSelectorNameFound &&
+ State.Stack.back().BreakBeforeParameter)
+ return true;
if (Current.NestingLevel == 0 && !Current.isTrailingComment()) {
// Always break after "template <...>" and leading annotations. This is only
// for cases where the entire line does not fit on a single line as a
@@ -916,8 +916,13 @@
unsigned LastSpace = State.Stack.back().LastSpace;
bool AvoidBinPacking;
bool BreakBeforeParameter = false;
- unsigned NestedBlockIndent = std::max(State.Stack.back().StartOfFunctionCall,
- State.Stack.back().NestedBlockIndent);
+ unsigned NestedBlockIndent;
+ if (Style.IndentNestedBlocks) {
+ NestedBlockIndent = std::max(State.Stack.back().StartOfFunctionCall,
+ State.Stack.back().NestedBlockIndent);
+ } else {
+ NestedBlockIndent = State.Stack.back().NestedBlockIndent;
+ }
if (Current.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare)) {
if (Current.opensBlockOrBlockTypeList(Style)) {
NewIndent = State.Stack.back().NestedBlockIndent + Style.IndentWidth;
@@ -967,7 +972,7 @@
if (getLengthToMatchingParen(Current) + State.Column >
getColumnLimit(State))
BreakBeforeParameter = true;
- } else {
+ } else if (Style.AllowNewlineBeforeBlockParameter) {
// For ColumnLimit = 0, we have to figure out whether there is or has to
// be a line break within this call.
for (const FormatToken *Tok = &Current;
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -414,6 +414,12 @@
/// type.
bool IndentWrappedFunctionNames;
+ /// \brief If true, indent nested blocks to the start of the nested function call
+ bool IndentNestedBlocks;
+
+ /// \brief If true, allow newlines before block parameters when ColumnLimit is 0.
+ bool AllowNewlineBeforeBlockParameter;
+
/// \brief If true, empty lines at the start of blocks are kept.
bool KeepEmptyLinesAtTheStartOfBlocks;
@@ -642,6 +648,8 @@
IndentCaseLabels == R.IndentCaseLabels &&
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
+ IndentNestedBlocks == R.IndentNestedBlocks &&
+ AllowNewlineBeforeBlockParameter == R.AllowNewlineBeforeBlockParameter &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
MacroBlockBegin == R.MacroBlockBegin &&
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
