This is an archive of the discontinued LLVM Phabricator instance.

Differential D47393

[clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜
ClosedPublic

Authored by stephanemoore on May 25 2018, 2:27 PM.

Details

Reviewers
benhamilton
jolesiak
djasper
Commits
rG1ab722ed17d3: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for…
rC334739: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for…
rL334739: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for…

Diff Detail

Repository
rC Clang

Event Timeline

stephanemoore created this revision.May 25 2018, 2:27 PM
Herald added subscribers: cfe-commits, klimek. · View Herald TranscriptMay 25 2018, 2:27 PM
stephanemoore updated this revision to Diff 148672.May 25 2018, 2:31 PM
stephanemoore updated this revision to Diff 148705.May 26 2018, 12:28 AM
stephanemoore updated this revision to Diff 149338.May 31 2018, 12:15 PM

Removed the last format from the tests. That particular scenario was failing and it might need additional changes to pass. I also think that particular scenario is not critical to the change and can be considered in a followup.

stephanemoore added a comment.Jun 1 2018, 4:17 PM

Polling the Google Objective-C community to see if anyone objects to this change. So far no concerns have been raised.

stephanemoore added reviewers: benhamilton, jolesiak, djasper.Jun 6 2018, 2:41 PM

I believe I have consensus that this is the correct change for Google Objective-C style.

benhamilton accepted this revision.Jun 7 2018, 11:34 AM
This revision is now accepted and ready to land.Jun 7 2018, 11:34 AM
stephanemoore added a comment.Jun 13 2018, 6:49 PM

Are additional approvals needed to land this? I pinged jolesiak earlier this week asking if he had any comments but have not heard back yet.

(also, once necessary approvals have been granted can I request help from someone to land it for me? 😇)

benhamilton added a comment.Jun 14 2018, 10:33 AM

I'll just land it for you.

Closed by commit rL334739: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for… (authored by benhamilton). · Explain WhyJun 14 2018, 10:34 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: llvm-commits. · View Herald TranscriptJun 14 2018, 10:34 AM
jolesiak added a comment.Jun 14 2018, 6:03 PM

Sorry, I missed that.
LGTM, I would consider adding ObjC method expression tests (multiline string as an argument). Will look into that next week.

jolesiak mentioned this in D48432: [clang-format] Add AlwaysBreakBeforeMultilineString tests.Jun 21 2018, 7:46 AM
jolesiak mentioned this in rC335338: [clang-format] Add AlwaysBreakBeforeMultilineString tests.Jun 22 2018, 5:02 AM
jolesiak mentioned this in rL335338: [clang-format] Add AlwaysBreakBeforeMultilineString tests.

Revision Contents

PathSize
lib/
Format/
1 line
unittests/
Format/
12 lines

Diff 148705

lib/Format/Format.cpp

Context not available.
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false; GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.SpacesInContainerLiterals = false; GoogleStyle.SpacesInContainerLiterals = false;
GoogleStyle.Cpp11BracedListStyle = false; GoogleStyle.Cpp11BracedListStyle = false;
// This affects protocol buffer options specifications and text protos. // This affects protocol buffer options specifications and text protos.
// Text protos are currently mostly formatted inside C++ raw string literals // Text protos are currently mostly formatted inside C++ raw string literals
// and often the current breaking behavior of string literals is not // and often the current breaking behavior of string literals is not
// beneficial there. Investigate turning this on once proper string reflow // beneficial there. Investigate turning this on once proper string reflow
// has been implemented. // has been implemented.
GoogleStyle.BreakStringLiterals = false; GoogleStyle.BreakStringLiterals = false;
} else if (Language == FormatStyle::LK_ObjC) { } else if (Language == FormatStyle::LK_ObjC) {
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.ColumnLimit = 100; GoogleStyle.ColumnLimit = 100;
} }
return GoogleStyle; return GoogleStyle;
} }
FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) { FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
FormatStyle ChromiumStyle = getGoogleStyle(Language); FormatStyle ChromiumStyle = getGoogleStyle(Language);
if (Language == FormatStyle::LK_Java) { if (Language == FormatStyle::LK_Java) {
ChromiumStyle.AllowShortIfStatementsOnASingleLine = true; ChromiumStyle.AllowShortIfStatementsOnASingleLine = true;
Context not available.

unittests/Format/FormatTestObjC.cpp

Context not available.
Style.ColumnLimit = 60; Style.ColumnLimit = 60;
// If the statement starting with 'a = ...' is put on a single line, the ';' // If the statement starting with 'a = ...' is put on a single line, the ';'
// is at line 61. // is at line 61.
verifyFormat("int f(int a) {\n" verifyFormat("int f(int a) {\n"
" a = [self aaaaaaaaaa:bbbbbbbbb\n" " a = [self aaaaaaaaaa:bbbbbbbbb\n"
" ccccccccc:dddddddd\n" " ccccccccc:dddddddd\n"
" ee:fddd];\n" " ee:fddd];\n"
"}"); "}");
} }
TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
Style = getGoogleStyle(FormatStyle::LK_ObjC);
verifyFormat("aaaa = @\"bbbb\"\n"
" @\"cccc\";");
verifyFormat("aaaa(@\"bbbb\"\n"
" @\"cccc\");");
verifyFormat("aaaa(qqq, @\"bbbb\"\n"
" @\"cccc\");");
verifyFormat("NSString *const kString = @\"aaaa\"\n"
" @\"bbbb\");");
}
} // end namespace } // end namespace
} // end namespace format } // end namespace format
} // end namespace clang } // end namespace clang
Context not available.