In D121069#3362450, @MyDeveloperDay wrote:

@feg208 I could do with some clarity on the algorithm, am I correct in thinking it requires that the first row, to contain at least the maximum number of columns for the rest of the array. I sort of noticed it was fine if the first row was larger

{
    {1,2,3 }
    {4,5}
    {6}
}

I have been watching the issues pile up around this but work has prevented me from focusing on them. This seems like a reasonable compromise but the note of disabling this feature is also reasonable. Given where it had to be placed in the formatting flow I suspect it will be challenging to resolve most of the troublesome corner cases. I did investigate one bug I picked up but ran into the problems you described earlier and was going down the same compromise path.

Absolutely in agreement

By one other bug I mean an entirely separate bug that I will handle in a separate PR. Sorry if that was confusing.

So I added in the second case but commented it out as it gets misformatted. However it doesn't assert which was the alarming outcome. I have one other bug to clean up here and I am going to open a bug to fix the line breaking after initial { in certain cases since the code there needs to be revisited. I am doing formatting in the info gathering loop where really I should add all that in the back end. But thats a bigger change that I'd like to do outside of mitigating this users crash.

Gets rid of the crash for the second added case

I am. I'll jump on this over the weekend. Sorry been a bit buried in my day job

I have a few outstanding questions

Addresses some review comments

I'd just wanted to follow up and see if, minimally, my changes addressed the concerns raised or if you think there is further work here?

In D101868#2820075, @dyung wrote:

Another +1 for it failing intermittently on the bot I watch over, llvm-clang-x86_64-sie-ubuntu-fast. Anything that can be done to stabilize the test or to remove it would be appreciated!

If I could get someone to land this commit on my behalf I'd be obliged. I am working on the getting commit rights since that seems to be a pretty significant impediment to developing in llvm land but as of now I can't land this myself.

I put up a separate commit that just removes the test. I think this would resolve the builder issues but I don't think its worth even trying. We should abandon this (I think)

I think, at this point, the sensible thing to do is to pull the test. As @MyDeveloperDay points out it isn't really adding any value above and beyond the unit tests. If it were non-determinism in the change itself the unit tests would catch it. They test the same formatting and test for non-determinism explicitly. Yet they've never failed. And there are quite a few of them.

I don’t have commit rights currently. Could I get someone to land this on my behalf?

Looks like the dead code is removed in main

In D104242#2817815, @HazardyKnusperkeks wrote:

Could you split this into two diffs? Or are those two changes in any way related?

Forgot the clang-tidy checks

Clang-tidy checks cleaned up and test fixes

Ok. I am going to see if I can replicate the issue locally. Otherwise the easiest path (imho) is to pull the lit test that is failing. It isn't telling us anything we don't already know from the unit tests in any case. But maybe its something odd like the spacing or the single '-' in the option flags on FileCheck

Added command line print

This is not yet complete. I missed step 4a. "4a. If we crash, we generate the reproducer and print the dsymutil invocation to replay the reproducer." It currently doesn't print the command line

Fixes the broken tests

Rolled up review comments

I think this covers the comments

In D101868#2811290, @HazardyKnusperkeks wrote:

In D101868#2810452, @feg208 wrote:

If I can get someone to submit this on my behalf I think we can call it a day.

Please post the name and email for the commit. And if no one else commits, I will do it on the weekend.

If I can get someone to submit this on my behalf I think we can call it a day.

All the review comments are addressed

Rolls up the remaining review comments

In D101868#2808826, @curdeius wrote:

LGTM. That's a great piece work @feg208. Thank you!

addressed the remaining review comment

Regenerated the mangled ClangFormatStyle.rst file

Ick. Missed a semi colon in the test

Fixes the busted test

Got some changes. The failing test is an open issue

Gets some of the formatting issues. Still chasing a test issue.

Fixes from clang-tidy checks

This alters the array alignment to allow for left alignment

In D101868#2805180, @MyDeveloperDay wrote:

Better still how about having

ArraryMemberAligmentEnum AlignArrayOfStructures

of "None, Left and Right"

I find boolean options don't stay boolean for long!

Ok. Given @HazardyKnusperkeks comment I'll make the default be left alignment and update the tests accordingly

Gets some review comments

I rolled up all the comments save one where I just have an outstanding question and then I'll roll that up as well

Had some invalid code in the lit test

Rebased against main

Got both of these

Grabs up some review comments and adds lit test

Adds a simple lit test for some sanity checks

one remaining

Missed a request

Captured review comments

I rolled up the suggested changes.

oops left some debugging messages in

I am wondering if I could get someone to look at this? I think I have addressed the review comments but I'd be happy to fix anything I missed

I am of the view that this is complete. Or at a minimum all the tests that have
been requested pass and I would be surprised if there were gross errors or other
major things I have missed

Rolls up review comments

I picked up most of these. One of the tests is already covered (I think) maybe I am misunderstanding

Still need to fix and add the tests I said I would but the comment tests are added

Addresses review comments

The tests still need to be added

Broke a regression test

I think I addressed all the comments? If so I can't push the changes into the repo. Could you? Assuming there aren't outstanding concerns of course.

Addresses some review comments

clang-tidy and clang-format changes

In D101868#2776633, @HazardyKnusperkeks wrote:

In D101868#2775550, @feg208 wrote:

This reworks substantially this commit. I recognize there are lacking/broken
tests but I just would like to ensure that the general direction doesn't
seem likely to end in tears

I'm not deep enough in clang-format and currently have not enough time to check that in depth. But why are you right aligning?

Fixes/adds a test that yielded a seg fault and quiets some clang-tidy checks

Adds some tests and fixes a broken test

Forgot to alter the documentation to reflect right justified column alignment

This reworks substantially this commit. I recognize there are lacking/broken
tests but I just would like to ensure that the general direction doesn't
seem likely to end in tears

In D101868#2774152, @MyDeveloperDay wrote:

This looks like a good start..

In D101868#2745942, @HazardyKnusperkeks wrote:

Something which just came to my mind. Since you wrote your own LineFormatter, you have to add test cases for all kinds of indention and wrapping. It seems that the string wrapping does not happen, how about comments? Are classes, namespaces, functions, etc. correctly indented with various configurations?

Given my expectations I think the alignment has to happen differently, but I'm happy to be proven wrong.

I still need to handle the 20 character case appropriately

Missed a request in the release notes

Oops missed a request

Added more tests and addressed review comments

I think I have all the tests and requested review comments rolled up

I have a few outstanding questions about additional testing

To answer the question of why I think this is different then other alignment options....It seems to me that each alignment option emphasizes a specific thing, be it macros, bitfields, or (maybe closer in spirit) more simple declarations and assignments. I think this case is unique and not currently addressed by any of the existing alignment mechanisms.

This addresses most of the review comments. There remain a few tests to add

clang-format misses

Cleans up clang-tidy/format issues and some minor branch pecadillos

Forgot to remove raise(SIGSEGV)

This alters the prior commit by incorporating the Reproducer for the purpose of persisting the files on crash

Missing more clang-tidy checks

Fixed clang-tidy warnings