[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

[Andi via Phabricator via cfe-commits]

Abpostelnicu added inline comments.


================
Comment at: include/clang/Format/Format.h:309
+  /// inheritance.
+  bool BreakInhertianceBeforeColonAndComma;
+  
----------------
djasper wrote:
> Hm. I am still not sure about this flag and it's name. Fundamentally, this is currently controlling two different things:
> - Whether to wrap before or after colon and comma
> - Whether or not to force wraps for multiple inheritance
> 
> That's going to get us in trouble if at some point people also want other combinations of these two things. I have to alternative suggestions:
> - Add a single flag (e.g. InheritanceListWrapping) with multiple enum values describing the
>   various choices.
> - Don't add a flag at all, but instead break inheritance lists exactly like constructor initializers.
>   I *think* that should solve all the know use cases for now, is easier to configure for users
>   and we can cross the bridge of naming additional flags when we have to.
I see your worries here and indeed the current flags does two things. I'm more inclined to use the num that you suggested since in this way it's easier to separate the coding style differences that somebody uses. 


================
Comment at: lib/Format/TokenAnnotator.cpp:2400
+// Returns 'true' if there is an TT_InheritanceComma in the current token list.
+static bool hasMultipleInheritance(const FormatToken &Tok) {
+  for (const FormatToken* nextTok = Tok.Next; nextTok; nextTok = nextTok->Next)
----------------
djasper wrote:
> I don't think you need this. If you set MustBreakBefore to true for the InheritanceCommas and set NoLineBreak to true when adding the InheritanceColon on the same line, then clang-format will already do the right thing.
> 
> Fundamentally, this seems to be identical to how we wrap constructor initializer lists in Mozilla style. So I think we should also implement this the same way (if not even reusing the same implementation).
Now that i've seen the behaviour of NoLineBreak thanks for pointing it out to me, but still correct me if i'm wrong but shouldn't i use: ``NoLineBreakInOperand``. 
My guess is if i use NoLineBreak, than breaking on the current line where we would have : or , would be prohibited.


Repository:
  rL LLVM

https://reviews.llvm.org/D30487