[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Please reupload with the complete context.

Please add tests.



================
Comment at: clang/include/clang/Format/Format.h:827
+  /// \code
+  ///   someFunction(
+  ///       int argument1,
----------------
That's not a valid declaration (missing return type), so not a good example.


================
Comment at: clang/include/clang/Format/Format.h:831
+  /// \endcode
+  bool AlwaysBreakBeforeFunctionParameters;
+
----------------
Please sort alphabetically. (So above `AlwaysBreakBeforeMultilineStrings`.)


================
Comment at: clang/include/clang/Format/Format.h:831
+  /// \endcode
+  bool AlwaysBreakBeforeFunctionParameters;
+
----------------
HazardyKnusperkeks wrote:
> Please sort alphabetically. (So above `AlwaysBreakBeforeMultilineStrings`.)
You are missing the \since 17.


================
Comment at: clang/lib/Format/Format.cpp:1510
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
+  GoogleStyle.AlwaysBreakBeforeFunctionParameters = false;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
----------------
Only set it in getLLVMStyle, which you are missing right now. Every other style will inherit it.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:4740-4742
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+      !Right.is(tok::r_paren)) {
+    if (Left.Previous) {
----------------
Merge the `if`s.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125171/new/

https://reviews.llvm.org/D125171