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

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin marked 4 inline comments as done.
jrmolin added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:1336
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  LLVMStyle.AlwaysBreakBeforeFunctionParameters = false;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
----------------
MyDeveloperDay wrote:
> ok, we have this think that the lifetime of an option goes from bool -> enum -> struct, sometimes we pick up early that true/false aren't good enough
> 
> so here is the think... `AlwaysBreakBeforeFunctionParameters` should this be  an enum and `BreakBeforeFunctionParameters` but with values
> 
> `Leave, Never, Always`
> 
> i.e. does `AlwaysBreakBeforeFunctionParameters` = false mean never break? or sometimes break.
> 
> We don't really want "false" to mean do something..we want it to mean don't do anything i.e. Leave
> 
> 
> 
> 
Please let me know if I did this wrong! This took more than I was expecting. I added parsing for "false" and "true" to be "Leave" and "Always". That is kind of confusing, and there is no need for "backwards compatibility" in this, but it looked easy. Should I remove that?


================
Comment at: clang/unittests/Format/FormatTest.cpp:25437
+  // verify that there is no break by default
+  verifyFormat("int function1(int param1, int param2, int param3);\n"
+               "int function2();\n",
----------------
MyDeveloperDay wrote:
> I would say all these function could go to the single format of `verifyFormat`
I consolidated the tests and expanded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171