Re: [PATCH] D11837: Add Qt brace style configuration option.

2015-08-10 Thread Manuel Klimek via cfe-commits
klimek added a comment.

Ah, ok. Interestingly, our current Webkit style uses BS_Stroustrup, which is 
incorrect for } else {. So  yep, implementing BS_Webkit sounds like a win.


http://reviews.llvm.org/D11837



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11837: Add Qt brace style configuration option.

2015-08-10 Thread Roman Kashitsyn via cfe-commits
lifted added a comment.

 Hm, the referenced style guide doesn't say anything about namespaces.


You're right. Futhermore, it's hard to find any namespaces in qt repos at all - 
Qt uses a macro for conditional compilation with namespaces.

 I also couldn't find anything in the qt source repo (just took a quick look 
 though).


Hm, I've just browsed through the qtcore and seen no evidence that qt uses 
attached braces everywhere but after classes and function. Qt use of style is 
inconsistent (sometimes even within a single file), and few examples I've 
looked at actually use something close to Mozilla style - they break on 
structs, enums, classes, and functions 
(http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qchar.h?id=13c16bbd06e0034210ba2164f1be208c49a6e3a7).

My bad, I read the Qt Brace style description and decided that it could fit my 
current project style (we break after functions, and some people also break 
after classes). But it turned out that Qt style is not as well defined as I 
thought, so I'm out of luck.

I believe it's better to abandon this patch.

I hope I will find some time to introduce extra configuration options to 
override pre-defined brace styles, as it was discussed long time ago.


http://reviews.llvm.org/D11837



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11837: Add Qt brace style configuration option.

2015-08-10 Thread Daniel Jasper via cfe-commits
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good. Do you have commit access?



Comment at: include/clang/Format/Format.h:180
@@ +179,3 @@
+BS_GNU,
+/// Like ``Attach``, but break before braces on functions, and classes.
+BS_Qt

Remove the second comma (and regenerate the docs)


http://reviews.llvm.org/D11837



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits