Re: [PATCH] D6833: Clang-format: Braces Indent Style Whitesmith

[JVApen via cfe-commits]

Hello Tim,

I am no longer working on this. I've spent my time convincing my colleagues
that clang-format is useful enough to change our braces, with success.

I remember also being stuck on the enumerations. Good luck!

On Wed, Jun 12, 2019, 04:16 Tim Wojtulewicz via Phabricator <
revi...@reviews.llvm.org> wrote:

> timwoj added a comment.
>
> I updated this patch to remove all of the code from `ContinuationIndenter`
> and to use the newer `BraceWrapping` style option instead of setting each
> one individually in `UnwrappedLineParser`.
>
> I still have the same issue with enums. Looking at the `RootToken` in
> `UnwrappedLineFormatter::formatFirstToken` when parsing an enum, the first
> pass through it has the entire line for the enum in it with all of the
> newlines stripped from my test case. It then doesn't ever return the braces
> as separate lines. If someone could give me a pointer as to how/why enums
> are tokenized differently from everything else, I can likely fix it. The
> test case I'm using looks like:
>
>   verifyFormat("enum X\n"
>"  {\n"
>"  Y = 0,\n"
>"  }\n",
>WhitesmithsBraceStyle);
>
> I also have the issue with a `break` after a block inside of a case
> statement, as mentioned in https://reviews.llvm.org/D6833#107539. Other
> than those two, it's mostly working correctly. I removed the test for
> lambdas because I'm not entirely sure how that should even be formatted
> with Whitesmiths, but I can add it back in again.
>
> Should I update this thread with the new diff against HEAD, or should I
> open a new one and close this one as dead?
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D6833/new/
>
> https://reviews.llvm.org/D6833
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14484: Formatting constructor initializer lists by putting them always on different lines

[JVApen via cfe-commits]

JVApen created this revision.
JVApen added a reviewer: djasper.
JVApen added a subscriber: cfe-commits.
JVApen set the repository for this revision to rL LLVM.
Herald added a subscriber: klimek.

Hi all,

I've been playing around with the clang for a while now and really enjoy it. 
Unfortunately clang-format does not yet do what I like it to do, so I started 
hacking it. So here is my first successful attempt to get something working.

The issue: ConstructorInitializerAllOnOneLineOrOnePerLine only works if 'If the 
constructor initializers don’t fit on a line', while I prefer it to always 
work. In other words, I use the following formatting:

```
Constructor()
  : a(a)
  , b(b)
```

Since everyone can benefit from upstreaming, I like to share my changes and get 
some feedback.
Here is already some of the stuff which I was uncertain about:

  - Should I keep ConstructorInitializerAllOnOneLineOrOnePerLine or rename it 
(currently the second one)
  - How to name the values, currently: Compact (old: false), BestFit (old: 
true), OnePerLine (new)
  - Is the back-ward compatibility in ScalarEnumerationTraits a good idea? (On 
rename most likely not)

JVApen



Repository:
  rL LLVM

http://reviews.llvm.org/D14484

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3586,8 +3586,51 @@
"  aaa(aaa),\n"
"  at() {}");

+  FormatStyle BestFit = getLLVMStyle();
+  BestFit.ConstructorInitializer = FormatStyle::CI_BestFit;
+  verifyFormat("SomeClass::Constructor()\n"
+   ": a(aa),\n"
+   "  a(aa),\n"
+   "  a(aa) {}",
+   BestFit);
+  verifyFormat("SomeClass::Constructor()\n"
+   ": a(aa), // Some comment\n"
+   "  a(aa),\n"
+   "  a(aa) {}",
+   BestFit);
+  verifyFormat("MyClass::MyClass(int var)\n"
+   ": some_var_(var),// 4 space indent\n"
+   "  some_other_var_(var + 1) { // lined up\n"
+   "}",
+   BestFit);
+  verifyFormat("Constructor() : a(a), a(a) {}\n",
+   BestFit);
+  verifyFormat("Constructor()\n"
+   ": a(aa),\n"
+   "  a(aa),\n"
+   "  a(aa),\n"
+   "  a(aa),\n"
+   "  a(aa) {}",
+   BestFit);
+  verifyFormat("Constructor()\n"
+   ": a(aa, aa,\n"
+   "aa) {}",
+   BestFit);
+  BestFit.ColumnLimit = 60;
+  verifyFormat("Constructor()\n"
+   ": (a),\n"
+   "  (b) {}",
+   BestFit);
+
+  EXPECT_EQ("Constructor()\n"
+": // Comment forcing unwanted break.\n"
+"  () {}",
+format("Constructor() :\n"
+   "// Comment forcing unwanted break.\n"
+   "() {}"));
+
   FormatStyle OnePerLine = getLLVMStyle();
-  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.ConstructorInitializer = FormatStyle::CI_OnePerLine;
   verifyFormat("SomeClass::Constructor()\n"
": a(aa),\n"
"  a(aa),\n"
@@ -3604,6 +3647,10 @@
"}",
OnePerLine);
   verifyFormat("Constructor()\n"
+   ": a(a),\n"
+   "  a(a) {}",
+   OnePerLine);
+  verifyFormat("Constructor()\n"
": a(aa),\n"
"  a(aa),\n"
"  a(aa),\n"
@@ -3696,7 +3743,7 @@

   // This test takes VERY long when memoization is broken.
   FormatStyle OnePerLine = getLLVMStyle();
-  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.ConstructorInitializer = FormatStyle::CI_BestFit;
   OnePerLine.BinPackParameters = false;
   std::string input = "Constructor()\n"
   ": (a,\n";
@@ -9560,7 +9607,6 @@
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
-  CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_B

Re: [PATCH] D14484: Formatting constructor initializer lists by putting them always on different lines

[JVApen via cfe-commits]

JVApen added a comment.

In http://reviews.llvm.org/D14484#284767, @djasper wrote:

> Please read
>  
> http://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options
>
> Does your style option qualify?




- Is it used by a project of significant size? Yes, we use the same style at 
work, where I try to get this used by about 70 developers. (Which I consider 
already dozens of contributors) Though it is also the same style I learned at 
school.
- Does it have a publicly accessible style guide? Yes, it's both an accepted by 
the Google Style guide 

 as Ubuntu Unity 
,
 which do not state that you have to use the compact form. Further more the 
graphisoft 

 styleguide doesn't appear to allow the condensed form.
- Does it have a person willing to contribute and maintain patches? Maybe, I 
currently don't believe I'm familiar enough with the code to maintain it. 
Though without putting it out here (e.g. LLVM community) the answer will 
definitely be no. Furthermore, beside the 2 lines in TokenAnnotator.cpp, this 
style does not differentiate from the current implementation.

Though since you are the/a maintainer of this code, you are more qualified to 
give a final answer on this question.

Regardless, even if you don't want this patch upstreamed, I like to understand 
the codebase better as well as the considerations which lead to certain 
decisions for code changes. (See also the uncertainties I listed in the 
original post) Which at least will make my local changes similar to what they 
would be in case you would allow an upstream of them.


Repository:
  rL LLVM

http://reviews.llvm.org/D14484



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

Re: [PATCH] D18300: [clang-tidy] ProTypeMemberInitCheck - check that field decls do not have in-class initializer

[JVApen via cfe-commits]

JVApen accepted this revision.
JVApen added a comment.
This revision is now accepted and ready to land.

I'm not sure if I'm the right person to do reviews, as I'm mostly a user of the 
tool and have not yet done any developments or bug fixes to it.
However, from what I know from coding, this looks good to me.


Repository:
  rL LLVM

http://reviews.llvm.org/D18300



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