[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> I'm sorry for not having a positive answer here, but I'm not in favor of this change. The style rule looks like it introduces arbitrary complexity at a point where I don't understand at all  how it matters. We cannot possibly support all style guides that come up with random variance. I do not think this option pulls its weight.

So is this a personal opinion or based on a technical reason (other than the normal mantra about risk & cost),  Could we give @reuk some positive feedback about what he needs to change in this review to get this accepted? He has the other boxes checked about public style etc..

It may not be your preference but its not without some merit for those of us who like to fine tune our style, if that can be done with minimal impact to other styles and is well covered by tests, then can we at least have some discussion first

@reuk please lets continue to fix this revision so it builds as I want to pull it into

https://github.com/mydeveloperday/clang-experimental

As an aside, Wouldn't it be great if JUCE was a public style so you could just say

BasedOnStyle: JUCE

Where that style defines what JUCE uses? I've always found this odd, I've never understood why we only have Google,WebKit,Mozilla,LLVM, I would have thought it would have been nice to have other big guns Microsoft,Qt generally its only going to be a single function setting the defaults.

I actually think a change like that would really help people so they don't have to work out all the individual styles, it would help keep .clang-format files small and neat and not full of a bunch of different options.

Even if JUCE was based on another style say "Google" the code should really be saying

if (Style.isGoogleDerivedStyle())

  ......

and not

Style == FormatStyle::Google

When its specific to JUCE it would say

if (Style.isJUCEDerivedStyle())

  ......

I know when using LLVM, that being able to have a .clang-format file that just says

BasedOnStyle: LLVM

is really nice, I don't have to go work out what the LLVM style is, I just get what the project defines, I think it would be great that anything that passes the public style test for submissions should really be able to add that kind of configuration.


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

https://reviews.llvm.org/D55170