HazardyKnusperkeks added inline comments.
================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1404
+ isArrayOfStructuresInit(TheLine));
+ if (AlignArrayOfStructuresInit) {
+ Penalty += AlignedArrayOfStructuresInitLineFormatter(
----------------
feg208 wrote:
> HazardyKnusperkeks wrote:
> > I'm not sure that you should go before the no column limit. In either case
> > you should have a test case for that.
> I kept the ordering but I will admit to misgivings about it. None of the
> tests fail and it seems like it needs to be ordered in this way but as a
> newbie contributor to this project I don't feel confident in that assertion.
> Do the included tests cover it? What test that isn't currently there would
> better create issues?
See below.
================
Comment at: clang/unittests/Format/FormatTest.cpp:16352
+template <size_t M, size_t N>
+auto createStylesImpl(
----------------
This is a nice approach!
Could it be made constexpr? I'm not sure about C++14.
================
Comment at: clang/unittests/Format/FormatTest.cpp:16371
+ Style);
+}
+
----------------
feg208 wrote:
> HazardyKnusperkeks wrote:
> > curdeius wrote:
> > > I think we need more tests:
> > > * with a comma after the last element of array
> > > * with assignment and not only initialization?
> > > * without `struct` keyword
> > > * with short lines/long values to see how wrapping behaves
> > > * also, with multiple string literals in one struct, e.g. `"hello"
> > > "world"` (that will get splitted into multiple lines)
> > > * with non-plain-array types (i.e. missing `[]`/`[3]`)
> > > * interaction with other Align* options (e.g.
> > > `AlignConsecutiveAssignments`, two arrays one after another,
> > > `AlignConsecutiveDeclarations`).
> > In addition to that I want to see:
> > * How it behaves with a smaller column limit which would be violated if
> > there is no additional wrapping. There I also want one without
> > `verifyFormat`.
> > * With no column limit.
> > * Maybe with a `const`, `inline` or something similar. (How about east
> > `const` vs. west `const`?)
> I added
>
> - with a comma after the last element of array
> - with assignment and not only initialization?
> - without struct keyword
> - with short lines/long values to see how wrapping behaves (also multiple
> string literals) But I still should add another test I think
> - with non-plain-array types (i.e. missing []/[3])
> - interaction with other Align* options (e.g. AlignConsecutiveAssignments,
> two arrays one after another, AlignConsecutiveDeclarations). I will say that
> I did this in a brute force way. Maybe there is something else I should do
> here?
> - With no column limit.
>
> I still need tests with east and west const and without verifyFormat. For the
> latter is there an example test I could look at? Idly scrolling through
> FormatTest.cpp it seems like most stuff uses verifyFormat? Or maybe I
> misunderstood the request?
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
What's missing is to format your last code with a column limit of 20 or so.
Regards using `EXPECT_EQ`:
```
EXPECT_EQ("test demo[] = {\n"
" {56, 23, \"hello world i am a very long line that really, in any "
"\"\n"
" \"just world, ought be split over multiple lines\" },\n"
" {-1, 93463, \"world\" },\n"
" {7, 5, \"!!\" },\n"
"};\n",
format("test demo[] = {{56, 23, \"hello world i am a very long line that
really, in any just world, ought be split over multiple lines\"},{-1, 93463,
\"world\"},{7, 5, \"!!\"},};", Style));
```
This should be done with ColumnLimit 0, 20, 100.
With `verifyFormat` you verify that the formatting is stable, but especially
with no column limit there is often no stable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101868/new/
https://reviews.llvm.org/D101868
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
