[PATCH] D88084: [clang-format] Changed default styles BraceWrappping bool table to directly use variables

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In the examples you give here.. (and I have a feeling there are others in the tests) some of these fields are the same in all 3 cases

i.e. `SplitEmptyRecord, SplitEmptyFunction and SplitEmptyNamespace`

In which case why doesn't BraceWrapping have a constructor that sets the global default? (LLVMStyle wins) and the other styles only specify what is different.

I have real concerns about what happens when new options are introduced and we forget to add them everywhere (same concerns I had before). but I agree the {} without comments and even the {} with comments is far from ideal.

I have no confidence that some of the options might not be uninitialized, at a minimum some are initialized twice, both before and after it feels a little like smelly code to me. (no fault of yours)

That said I think the 1-1 replacement is an improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88084