[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-25 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. In D93986#2519013 , @curdeius wrote: > Fixed! Thanks a lot! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 __

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-25 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Fixed! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-25 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I've committted it, but... something went wrong and you're not visible as the author. Sorry for that @tinloaf. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-25 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf00a20e51c1d: [clang-format] Add the possibility to align assignments spanning empty lines or… (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-24 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. In D93986#2518402 , @HazardyKnusperkeks wrote: > I did not re-accept this, because of the script change. I'm okay with it, but > I never looked really at the script. I think it should be changed afterwards > to error or warn aga

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-24 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. I did not re-accept this, because of the script change. I'm okay with it, but I never looked really at the script. I think it should be changed afterwards to error or warn again, but not on this enum. Maybe one could

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-24 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. In D93986#2518259 , @curdeius wrote: > Do you have commit access? Since this is my first contribution to LLVM ever, probably not. > If no, please write your Name Surname that should be used for > the commit. Lukas Barth Than

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-24 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Do you have commit access? If no, please write your Name Surname that should be used for the commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 _

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-24 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. If I understand the LLVM mailing list correctly, everything in the repo by the 26th will still go into LLVM 12, right? Any chance we could get this in by then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-22 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added inline comments. Comment at: clang/docs/tools/dump_format_style.py:194 +# is used. +pass elif state == State.InEnumMemberComment: curdeius wrote: > Just a nit, maybe we should have a warning at least? Not sure. After this commit

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-22 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/tools/dump_format_style.py:194 +# is used. +pass elif state == State.InEnumMemberComment: Just a nit, maybe we should have a warning at least? Repository: rG LLVM Github Monorepo CHA

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-21 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 318193. tinloaf added a comment. Fix error introduced by rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 Files: clang/docs/ClangFormatStyleOptions.rst clang/

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-21 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 318182. tinloaf added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Of course there was another merge conflict in the release notes. Mental note to self: Put release note changes in separate commit. Repository: rG LLV

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-21 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf accepted this revision. tinloaf added a comment. Fixed the last spelling issues in the documentation. If the automated checks pass, this is ready for takeoff from my side. Thanks for your reviews! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-21 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 318181. tinloaf marked an inline comment as done. tinloaf added a comment. Fix spelling issues in doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 Files: clang/doc

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-21 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked 3 inline comments as done. tinloaf added inline comments. Comment at: clang/include/clang/Format/Format.h:87 - /// \brief If ``true``, aligns consecutive C/C++ preprocessor macros. + /// Styles for alignment of consecutive tokens. Tokens can be assignment sign

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93986#2509079 , @tinloaf wrote: > In D93986#2509045 , > @HazardyKnusperkeks wrote: > >> How is it going? I would like to add some new alignment options and it would >> be st

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-20 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. In D93986#2509045 , @HazardyKnusperkeks wrote: > How is it going? I would like to add some new alignment options and it would > be stupid not to do it on top of this. Sorry, much to do work-wise currently. I'll get to the last d

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. How is it going? I would like to add some new alignment options and it would be stupid not to do it on top of this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 _

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:103 /// - /// This will align C/C++ preprocessor macros of consecutive lines. - /// Will result in formattings like + /// ``Consecutive`` will result

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thank you for changing the dump_style to make this clearer F15070790: image.png F15070797: image.png F15070803: image.png F15070810: image.png

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thie LGTM, just to check a few nits /usr/bin/sphinx-build -n ./docs ./html /clang/llvm-project/clang/docs/ClangFormatStyleOptions.rst:330: WARNING: Bullet list ends without a blank line; unexpected unindent. /clang/llvm-project/clang/docs/ClangFormatStyleOpt

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Not what I had in mind, but for me that's okay. I can not say anything to the change of the script though. What I hat in mind was explaining (maybe in length) the enum and for the alignment settings just show an example with `None` and one of the other optio

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-16 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 317166. tinloaf added a comment. Fixed documentation for the `AlignConsecutive*` options. I have used the suggested approach: Remove all documentation from `AlignConsecutiveStyle` and instead put the documentation of the possible values and what they do in t

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93986#2495561 , @MyDeveloperDay wrote: > I think I would remove the code examples from the "AlignConsecutive style" to > avoid confusion (that would be the first change) > > then perhaps regenerate and update the d

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think I would remove the code examples from the "AlignConsecutive style" to avoid confusion (that would be the first change) then perhaps regenerate and update the documentation so we can then see, its seems most have a "code block", can the specific examples n

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-12 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 316068. tinloaf added a comment. Fixed formatting issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/Re

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-10 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. In D93986#2489140 , @HazardyKnusperkeks wrote: > Apart from my inline comment and the pre merge check this is superb. (I will > not accept it, until we have reached a conclusion for the documentation.) Regarding the pre-merge ch

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Apart from my inline comment and the pre merge check this is superb. (I will not accept it, until we have reached a conclusion for the documentation.) Comment at: clang/docs/ClangFormatStyleOptions.rst:338 + + int = 12; + int

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 ___ cfe-commits mailing list cfe-

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-10 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 315665. tinloaf added a comment. Fix merge conflict in the release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 Files: clang/docs/ClangFormatStyleOptions.rs

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-10 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked 3 inline comments as done. tinloaf added a comment. This is now an all-in-one revision, including bit fields, assignments, declarations and macros. I did not reproduce the full "across empty lines, across comments, across empty lines *and* comments" test suite for all four of the

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-10 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. In D93986#2477860 , @HazardyKnusperkeks wrote: > In D93986#2477474 , @tinloaf wrote: > >> In D93986#2477383 , @MyDeveloperDay >> wrote: >> >>> Idea

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-10 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 315664. tinloaf marked an inline comment as done. tinloaf added a comment. - Unify the type of the options for `AlignConsecutive`{`BitFields`, `Declarations`, `Assignments`, `Macros`} - Implement across-alignment for bit fields, declarations, assignments and

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) - If ``true``, aligns consecutive assignments. +**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``) + Style of aligning

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-05 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked 2 inline comments as done. tinloaf added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) - If ``true``, aligns consecutive assignments. +**AlignConsecutiveAssignments** (``AlignConsecutiveAssign

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) - If ``true``, aligns consecutive assignments. +**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``) + Style of aligning

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thank you for submitting this patch? Comment at: clang/include/clang/Format/Format.h:117 +/// int b= 23; +/// +/// int ccc = 23; tinloaf wrote: > HazardyKnusperkeks wrote: > > You are adding a Tab here, or do

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-04 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added inline comments. Comment at: clang/include/clang/Format/Format.h:117 +/// int b= 23; +/// +/// int ccc = 23; HazardyKnusperkeks wrote: > You are adding a Tab here, or do I misinterpret this? Shouldn't that be fixed > by clang-f

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-04 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. In D93986#2477474 , @tinloaf wrote: > In D93986#2477383 , @MyDeveloperDay >

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-04 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. In D93986#2477383 , @MyDeveloperDay wrote: > Ideally we should add a comment to the release notes, (which you could do via > a separate NFC commit if you wanted) Thank, I'll do that. But before that, I will create a revision tha

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This LGTM, I'm not sure if others have any further comments Ideally we should add a comment to the release notes, (which you could do via a separate NFC commit if you wanted)

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-04 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 314377. tinloaf marked 3 inline comments as done. tinloaf added a comment. Fix bogus default case reported by clang-tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-04 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked 5 inline comments as done. tinloaf added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; MyDeveloperDay wrote: > tinloaf wrote: > > MyDeveloperDay wrote: > > > Is there a case for

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-04 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 314361. tinloaf added a comment. Address reviewer comments, noteworthy changes: - Add an option to span alignment only across comments, not across newlines - Factor out spanning options to `AlignTokens` to an enum Repository: rG LLVM Github Monorepo CH

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; tinloaf wrote: > MyDeveloperDay wrote: > > Is there a case for aligning over empty lines and comments? > > > > ``` > > int a

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked an inline comment as done. tinloaf added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; MyDeveloperDay wrote: > Is there a case for aligning over empty lines and comments? > > `

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:421 // matching token, the sequence ends here. - if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine) + if (((Changes[i].NewlinesBefore > 1) && (!AcrossEmpty)) || + (!F

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; Is there a case for aligning over empty lines and comments? ``` int a = 5; /* comment */ int oneTwoThree = 123; `

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked an inline comment as done. tinloaf added inline comments. Comment at: clang/include/clang/Format/Format.h:110 + + /// Styles for alignment of consecutive assignments + enum AlignConsecutiveAssignmentsStyle { MyDeveloperDay wrote: > you need to pr

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 314293. tinloaf marked 3 inline comments as done. tinloaf added a comment. Address MyDeveloperDay's review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 Fil

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:12581 +"\n" +"/* block comment */\n" +"\n" can you add an example with a block comment that spans multiple lines e.g. ``` int a = 5 /* *

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. by and large, this looks pretty good to me.. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) +**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``) If ``true``, aligns conse