[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

Sylvestre Ledru via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 18 05:59:41 PST 2017


sylvestre.ledru marked 3 inline comments as done.
sylvestre.ledru added inline comments.


================
Comment at: test/Format/check-coding-style-mozilla.cpp:10
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+if (true) {
----------------
krasimir wrote:
> What is tested here? Brace styles?
Yes, do you want me to add a comment to explicit that?


================
Comment at: test/Format/check-coding-style-mozilla.cpp:48
+                ,
+                public Y
+{
----------------
krasimir wrote:
> This does not check precisely what the comment says, because the comment affects the indentation decisions. Better put the comment before the class declaration.
I know, this is one of the thing I would like to see fixed in clang format or us. 
I am adding it in the test suite to make sure that we address it


================
Comment at: test/Format/check-coding-style-mozilla.cpp:75
+    return mVar;
+  } // Tiny functions can be written in a single line.
+
----------------
krasimir wrote:
> I don't get it - shouldn't then TinyFunction be on a single line? Also the long trailing comment might affect formatting, so I suggest putting it before the function definition.
Same as above


================
Comment at: test/Format/check-coding-style-mozilla.cpp:90
+template<typename T> // Templates on own line.
+static int           // Return type on own line for top-level functions.
+  MyFunction(int a)
----------------
krasimir wrote:
> Trailing comments affect line breaking, so this is not really testing what the comments say. Suggest to put them on a line before the template.
Yeah, we are trying to fix this issue.
but you are correct, I will move it


================
Comment at: test/Format/check-coding-style-mozilla.cpp:106
+             !GetCachedStyleData(aSID),
+           "bar");
+
----------------
krasimir wrote:
> What does this fragment and the following ones test?
Testing some issues which were reported. I added comments and removed some


https://reviews.llvm.org/D30111





More information about the cfe-commits mailing list