reuk added a comment. This looks much better now, thanks for taking another look! I've flagged some formatting/spelling nits, and I think the tests/docs could be a little more explicit about the behaviour of `WithoutElse`. Other than that, looks great!
================ Comment at: clang/docs/ClangFormatStyleOptions.rst:375 + * ``SIS_Never`` (in configuration: ``Never``) + Do not allow short if functions + ---------------- For consistency, these descriptions should end with periods `.` ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:386 + Allow short if functions on the same line, as long as else + is not a compound statement ---------------- Missing `.` ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:390 + + if (a) return; + else ---------------- I think an example of the generated formatting *with* a compound else statement would be useful here too, in addition to the existing example. ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:395 + * ``SIS_Always`` (in configuration: ``Always``) + Allow short if statements even if the else is a compounf statement + ---------------- compounf -> compound ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:395 + * ``SIS_Always`` (in configuration: ``Always``) + Allow short if statements even if the else is a compounf statement + ---------------- reuk wrote: > compounf -> compound Missing `.` ================ Comment at: clang/include/clang/Format/Format.h:264 + /// Always put short ifs on the same line if + /// the else is not a compound statement or not + /// \code ---------------- Missing `.` ================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:417 + // Only inline simple if's (no nested if or else), unless specified + if (Style.AllowShortIfStatementsOnASingleLine!=FormatStyle::SIS_Always) { + if (I + 2 != E && Line.startsWith(tok::kw_if) && ---------------- Has this line been clang-formatted? I'd expect spaces around the binop ================ Comment at: clang/unittests/Format/FormatTest.cpp:494 + AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse; + verifyFormat("if (a)\n" + " f();\n" ---------------- I think having a test here which shows the expected behaviour for non-compound else statements with `WithoutElse` would be helpful, as in the documentation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59087/new/ https://reviews.llvm.org/D59087 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits