[PATCH] D109557: Adds a BreakBeforeClosingParen option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Just as a general pattern what we see is that options start out as `bool`, shortly become `enums`, then as they get more complex become `structs` e.g. `BraceWrapping`

  bool BreakBeforeClosingParen

trying to think ahead a little can make future backwards compatibility a little easier.

It will be a lot more involved but I kind of wonder if we might not see the same here over time. I could foresee a situation where we might have:

  ParenWrapping:
        StartOfIfOpening: true
        EndOfIfExpression: true
        FunctionParameterOpening: true
        FunctionParameterClosing: true
   

of even a struct of enums (to allow special cases like short functions)

I think if we could capture in unit tests the types of situation where we would and wouldn't want to put a newline after `(` and before `)` it might help define a better set of options in the first place.

Otherwise if we are just going to use `BreakBeforeClosingParen` for all uses of `)` less the "short situations like c-style casts, then I kind of feel it should not impact parents around control statements like if,while,for etc... I think in which case I'd prefer we started out with a struct with just:  (ignore the actual names used, just something suitable)

  ParenWrapping:
        FunctionParametersClosing: <Never|Always|NonShortFunctions>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557