djasper added a comment. Could you please upload a diff with the entire file as context? That makes reviewing this easier.
================ Comment at: docs/ClangFormatStyleOptions.rst:428 +**BreakBeforeInhertianceDelimiter** (``boolt``) + If ``true``, in the class inheritance expression clang-format will ---------------- Auto-generate this with docs/tools/dump_format_style.py ================ Comment at: include/clang/Format/Format.h:309 + /// inheritance. + bool BreakBeforeInhertianceDelimiter; + ---------------- s/Delimiter/Colon/ Not because it is better, but just because it's more consistent with much of the rest of clang-format. ================ Comment at: lib/Format/TokenAnnotator.cpp:950 bool InCtorInitializer = false; + bool InInhertiance = false; bool CaretFound = false; ---------------- Maybe "InInheritanceList"? ================ Comment at: lib/Format/TokenAnnotator.cpp:1012 + Current.Previous->is(TT_InheritanceColon)) { + Contexts.back().IsExpression = true; + Contexts.back().InInhertiance = true; ---------------- Why would we be in an expression here? ================ Comment at: lib/Format/TokenAnnotator.cpp:2398 +// Determine if the next token from the closing scope is an inheritance token +static bool hasMultipleInheritance(const FormatToken &Tok) { ---------------- I don't understand this comment or what the function is supposed to do. It seems to search whether there is an inheritance comma somewhere in the rest of the line. ================ Comment at: lib/Format/TokenAnnotator.cpp:2490 return true; + //Break only if we have multiple inheritance + if (Style.BreakBeforeInhertianceDelimiter && ---------------- nit: Use periods of the end of sentences and a space after //. ================ Comment at: unittests/Format/FormatTest.cpp:1023 +TEST_F(FormatTest, BreakBeforeInhertianceDelimiter) { + FormatStyle StyleWithInheritanceBreak = getLLVMStyle(); ---------------- I am missing tests that show the behavior when there are multiple base classes, but they do fit on one line. ================ Comment at: unittests/Format/FormatTest.cpp:1033 + StyleWithInheritanceBreak); + verifyFormat("class MyClass" + " : public X\n" ---------------- What is this supposed to test? ================ Comment at: unittests/Format/FormatTest.cpp:1039 + verifyFormat("class MyClass\n" + " : public X // When deriving from more than one class, put " + "each on its own\n" ---------------- Sure, but the comment is forcing that, so I don't know what this test does. Repository: rL LLVM https://reviews.llvm.org/D30487 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits