[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Hm, can't really remember what I meant by "my comment". Probably not important.

So, I still see two problems:

- I would not count the link you mentioned as a publicly accessible style guide.
- I don't think the naming and granularity of this option is right.

You specifically want to control two things. The space in front of constructor initializers colons and of range-based for loop colons. Then you also (accidentally?) change the spacing in dict literals. However, the latter is already controlled by SpacesInContainerLiterals. So there even seems to be a conflict here. You do not want to change anything for ternary expressions or labels. Oh, and there are also colons in inline assembly. Colons are probably the most overloaded tokens.

Now, we could of course allow much more fine-grained control over colons, but really, that adds quite a bit of complexity to the clang-format configuration. And I am not sure whether the added costs (both in maintenance of clang-format and discoverability for users) are covered by the very small added value of that removed space for you.

I am sorry I don't have to better answer, but I have to weigh off the benefit this adds for you vs. the reduced usefulness to all other users of clang-format. In general, we'd rather have clang-format have fewer configuration options and support them really well. For now, I am against moving forward with this patch, unless we find a good way to structure the options and a publicly accessible coding style that really supports this (explicitly, not just in an example somewhere that's actually meant to show something else).


https://reviews.llvm.org/D32525