[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-09-24 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. I am. I'll jump on this over the weekend. Sorry been a bit buried in my day job Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 ___ cfe-

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. There are a number of bugs logged against this feature, are you still around to look into them? https://bugs.llvm.org/show_bug.cgi?id=51926 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-15 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. 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!

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-15 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment. 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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-15 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. 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 formatt

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Do we actually need this lit test? Mostly we only add lit tests when we add a new command line argument, I feel your are just testing what we also test in the unittests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. The test is still intermittently failing on bots. Is there some non-determinism in the change? It may be a good idea to revert this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-14 Thread Fred Grim via Phabricator via cfe-commits
feg208 added inline comments. Comment at: clang/test/Format/struct-array-initializer.cpp:5 +// RUN: grep -Ev "// *[A-Z-]+:" %s \ +// RUN: | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s \ +// RUN: | FileCheck -strict-whitespace -check-prefix=CHE

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-14 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments. Comment at: clang/test/Format/struct-array-initializer.cpp:5 +// RUN: grep -Ev "// *[A-Z-]+:" %s \ +// RUN: | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s \ +// RUN: | FileCheck -strict-whitespace -check-prefix

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-13 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG673c5ba58497: [clang-format] Adds a formatter for aligning arrays of structs (authored by feg208, committed by HazardyKnusperkeks). Repository: rG

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-10 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. 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 pos

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. 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 o

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-10 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. If I can get someone to submit this on my behalf I think we can call it a day. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 ___ cfe-c

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM 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-com

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked 3 inline comments as done. feg208 added a comment. All the review comments are addressed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 ___ cfe-c

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350984. feg208 marked 11 inline comments as done. feg208 added a comment. Rolls up the remaining review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files:

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked an inline comment as done. feg208 added a comment. In D101868#2808826 , @curdeius wrote: > LGTM. That's a great piece work @feg208. Thank you! Awww thanks. I learned a lot from all the comments honestly. I appreciate the patience. > I've

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. That's a great piece work @feg208. Thank you! I've added many nit comments, but I didn't do it for all code comments. Please check that all comments are full phrases (with full stops

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked an inline comment as done. feg208 added a comment. addressed the remaining review comment Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221 +struct test demo[] = +{ +{56,23, "hello"}, +{-1, 93463, "world"}, +{ 7, 5,

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350974. feg208 added a comment. Regenerated the mangled ClangFormatStyle.rst file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleO

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221 +struct test demo[] = +{ +{56,23, "hello"}, +{-1, 93463, "world"}, +{ 7, 5,"!!"} +}; feg208 wrote: > curdeius wrote: > > Don

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350924. feg208 added a comment. Ick. Missed a semi colon in the test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350920. feg208 added a comment. Fixes the busted test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/Rele

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. Got some changes. The failing test is an open issue Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221 +struct test demo[] = +{ +{56,23, "hello"}, +{-1, 93463, "world"}, +{ 7, 5,"!!"} +}; --

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350910. feg208 marked 4 inline comments as done. feg208 added a comment. Gets some of the formatting issues. Still chasing a test issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/WhitespaceManager.h:233 + /// Align Array Initializers over all \c Changes + void alignArrayInitializers(); Can ensure the comment starts with a capital and finishes with punctuation (I perso

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221 +struct test demo[] = +{ +{56,23, "hello"}, +{-1, 93463, "world"}, +{ 7, 5,"!!"} +}; Don't forget to re-generate .rst. Rep

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350774. feg208 added a comment. Fixes from clang-tidy checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst clang/do

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350746. feg208 added a comment. This alters the array alignment to allow for left alignment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFo

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Save the name. I should have it rolled up shortly. You an keep the name, just change the type to be an enum Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 _

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. 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! Yeah once I s

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. 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! A strong +1

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Better still how about having ArraryMemberAligmentEnum AlignArrayOfStructures of "None, Left and Right" I find boolean options don't stay boolean for long! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D101868#2803670 , @feg208 wrote: > Ok. Given @HazardyKnusperkeks comment I'll make the default be left alignment > and update the tests accordingly You could add `AlignArrayOfStructuresMemberAlignment` `Left:Right` y

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-07 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. Ok. Given @HazardyKnusperkeks comment I'll make the default be left alignment and update the tests accordingly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D101868#2799563 , @HazardyKnusperkeks wrote: > Remains the issue with the alignment, I would like to know @MyDeveloperDay 's > opinion on that. Should the values be right aligned, or left aligned? As far > as I see al

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Remains the issue with the alignment, I would like to know @MyDeveloperDay 's opinion on that. Should the values be right aligned, or left aligned? As far as I see all alignment in clang-format is left until now. =

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349730. feg208 added a comment. Had some invalid code in the lit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349728. feg208 added a comment. Rebased against main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/Relea

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. Got both of these Comment at: clang/lib/Format/TokenAnnotator.cpp:737-740 +const auto End = Contexts.rbegin() + 2; +auto Last = Contexts.rbegin(); +unsigned Depth = 0; +for (; Last != End; Last = std::next(Last)) { Hazard

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349589. feg208 marked 2 inline comments as done. feg208 added a comment. Grabs up some review comments and adds lit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:737-740 +const auto End = Contexts.rbegin() + 2; +auto Last = Contexts.rbegin(); +unsigned Depth = 0; +for (; Last != End; Last = std::next(Last)) { I actual

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349434. feg208 added a comment. Adds a simple lit test for some sanity checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptio

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. one remaining 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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349374. feg208 marked an inline comment as done. feg208 added a comment. Missed a request Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangForm

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349373. feg208 marked 2 inline comments as done. feg208 added a comment. Captured review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/C

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked 6 inline comments as done. feg208 added a comment. I rolled up the suggested changes. Comment at: clang/lib/Format/ContinuationIndenter.cpp:603 State.Column + Spaces + PPColumnCorrection); - // If "BreakBeforeInheritanceComma"

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. I've added a few comments, and I would like to hear the opinion of others regarding the left or right alignment of the elements. Comment at

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349324. feg208 added a comment. oops left some debugging messages in Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349299. feg208 added a comment. 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 Repository: rG LLVM Github Monorepo

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:16478 + "{56, /* a comment */ 23, \"hello\" },\n" + "{-1, 93463, \"world\" },\n" + "{ 7, 5,\"!!\" }\

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-27 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 348346. feg208 marked an inline comment as done. feg208 added a comment. Rolls up review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/C

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-27 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked 3 inline comments as done. feg208 added a comment. I picked up most of these. One of the tests is already covered (I think) maybe I am misunderstanding Comment at: clang/unittests/Format/FormatTest.cpp:16478 + "{56, /* a comment */ 23, \"hello\"

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/FormatToken.cpp:86 + (SplitMax > 0 && SplitMax < ColumnWidth) ? SplitMax : ColumnWidth; + ColWidth += (is(tok::comment)) ? 1 : 0; + const auto *NextColEntry = Next; Personal style, don'

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-26 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 348020. feg208 added a comment. Still need to fix and added the tests I said I would but the comment tests are added Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 F

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-25 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 347709. feg208 added a comment. Addresses review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-25 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked 2 inline comments as done. feg208 added a comment. The tests still need to be added Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 ___ cfe-commit

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D101868#2777423 , @feg208 wrote: > In D101868#2776633 , > @HazardyKnusperkeks wrote: > >> In D101868#2775550 , @feg208 wrote: >> >>

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-24 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 347419. feg208 added a comment. clang-tidy and clang-format changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst c

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-24 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. 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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-24 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. 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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-23 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 347252. feg208 added a comment. Fixes/adds a test that yielded a seg fault and quiets some clang-tidy checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: c

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-22 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 347211. feg208 added a comment. Adds some tests and fixes a broken test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-22 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 347209. feg208 added a comment. Forgot to alter the documentation to reflect right justified column alignment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files:

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-22 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 347208. feg208 added a comment. 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 Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:16389 + for (const auto &Style : Styles) { +verifyFormat("struct test demo[] = {\n" + "{56, 23,\"hello\" },\n" may be worth testing a case with co

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This is a valiant effort, and I definitely don't want to discourage..especially as you shown a good understand of clang-format. Its been a long asked for feature.. but we should really ensure this covers the bulk of the complaints if possible.. For me I use this

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-21 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. In D101868#2774152 , @MyDeveloperDay wrote: > This looks like a good start.. Thanks. I am reworking it so I handle line breaking in a sane fashion and dropping the LineFormatter override since that really can't handle reformatti

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This looks like a good start.. All your tests are 3x3 have you considered mixing it up a bit. i.e. 2x3, what is the impact on 1x5 and 5x1 ? Also how about nested structs, I'm interested to see what happens {56, 23, { "ABC", 35 }} {57, 24, { "DEF", 36 }} R

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-08 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. 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 wra

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-08 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. I still need to handle the 20 character case appropriately Comment at: clang/unittests/Format/FormatTest.cpp:16465 + "test demo[] = {\n" + "{56, 23,\"hello world i am a very long line that really, in any " + "just world, ought to

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-08 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. 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. c

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-08 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added inline comments. This revision now requires changes to proceed. Comment at: clang/unittests/Format/FormatTest.cpp:16465 + "test demo[] = {\n" + "{56, 23,\"hello world i am a very lon

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. Missed a request in the release notes 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.llv

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 343748. feg208 marked an inline comment as done. feg208 added a comment. Oops missed a request Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/Clan

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 343747. feg208 added a comment. Added more tests and addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 Files: clang/docs/ClangFormatStyleOpti

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked 4 inline comments as done. feg208 added a comment. I think I have all the tests and requested review comments rolled up Comment at: clang/unittests/Format/FormatTest.cpp:16352 +template +auto createStylesImpl( curdeius wrote: > HazardyKnusperke

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-07 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ReleaseNotes.rst:247-248 +- Option ``AlignArrayOfStructure`` has been added to allow for ordering array-like + initializers into nice orderly columns while respecting column limits. +

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-06 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1404 + isArrayOfStructuresInit(TheLine)); + if (AlignArrayOfStructuresInit) { +Penalty += AlignedArrayOfStructuresInitLineFormatt

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-06 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. I have a few outstanding questions about additional testing Comment at: clang/unittests/Format/FormatTest.cpp:16371 + Style); +} + HazardyKnusperkeks wrote: > curdeius wrote: > > I think we need more tests: > > * with a com

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-06 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment. To answer the question of why I think this is different then other alignment optionsIt 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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-06 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 343534. feg208 marked 8 inline comments as done. feg208 added a comment. This addresses most of the review comments. There remain a few tests to add Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ http

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I really like alignment! :) But why is this completely different than the other alignment options? You should also add an entry to the release notes. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:32 +// The notion here is that we

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-05 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. I like the idea, but we need more tests. Comment at: clang/include/clang/Format/Format.h:93 + /// if ``true``, when using static initialization for an array