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

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] D90238: [clang-format] Added ReferenceAlignmentStyle option - (Update to D31635)

MyDeveloperDay added a subscriber: HazardyKnusperkeks. MyDeveloperDay added a comment. @curdeius , @HazardyKnusperkeks any thoughts, personally I don't have any problems with this even if its not my personal preference. @skirkovski if this is something you need we might need for you to take ove

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

MyDeveloperDay added a comment. Just so I'm clear are you proposing that a newlines should always be added or that single blank lines should be ignored? I can't tell if the bug is that the line isn't being removed or sometimes not being added, either way there will be someone who wants it the o

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

MyDeveloperDay added a comment. Devils advocate how is this any different from class Foo { class Bar {} ; } }; This would become class Foo { class Bar {}; }; i.e. its going to remove the extra lines, just asking so we can understand if the removal of the line is the err

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

MyDeveloperDay added a comment. Do we need a set options for when we want to insert/retain/add a newline after various constructs? frankly I've been mulling over the concept of adding a NewLinesBetweenFunctions: 1 I personally don't like code written like this as I find it hard to read, I'd

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

MyDeveloperDay added a comment. We should have perhaps though about this when we added ```EmptyLineAfterAccessModifier ``` and ```EmptyLineBeforeAccessModifier ``` Repository: rZORG LLVM Github Zorg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104044/new/ https://reviews.llvm.org/D

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

MyDeveloperDay added a comment. A new nits but I'd say it was pretty good. Comment at: clang/lib/Format/Format.cpp:1356 Style.PointerAlignment = FormatStyle::PAS_Left; + Style.ReferenceAlignment = FormatStyle::RAS_Pointer; Style.SpaceBeforeCpp11BracedList = true; ---

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

MyDeveloperDay added a comment. My point being there is inconsistency between how different types of blocks of code are handled, and rather than trying to fix another corner case maybe we should take a more holistic approach, all these KeepEmptyLines and EmptyLineAfterXXX options and what you'l

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/include/clang/Format/Format.h:2647 - /// The ``&`` and ``*`` alignment style. + /// The ``&``, ``&&`` and ``*`` alignment style.

[PATCH] D19066: clang-format: `SpaceAfterTemplate` and `SpacesInBraces` options

MyDeveloperDay added a comment. This is already resolved with `SpaceAfterTemplateKeyword` and `SpaceInEmptyBlock` I'm going to commandeer and abandon CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19066/new/ https://reviews.llvm.org/D19066 __

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

MyDeveloperDay added a comment. A crash (https://bugs.llvm.org/show_bug.cgi?id=50663) in 12.0.1 is fixed by your changes here when merged to the 12 branch, I'm not sure if this needs to be backported to 12 I guess it might depend on if we think it's critical enough or if there will more 12 rel

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

MyDeveloperDay added a comment. I think we can agree its complicated, how about you take your unit tests and show us what the "code change" looks like to fix the bug If that gets overly convoluted then perhaps we can bring the idea forward of a more generalised approach. Repository: rZORG L

[PATCH] D43957: Fixing issue where a space was not added before a global namespace variable when SpacesInParentheses is set

MyDeveloperDay added a comment. This issue was resolved by https://github.com/llvm/llvm-project/commit/d5e9ff4fe20e66d53a245645c95f0bb816b747cb could you please close out this review Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43957/new/ https://reviews.llvm.

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

MyDeveloperDay added a comment. Herald added a subscriber: sstefan1. Tracking a possible bug, https://bugs.llvm.org/show_bug.cgi?id=50702 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44609/new/ https://reviews.llvm.org/D44609

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

MyDeveloperDay added a comment. namespace test { class Test { public: Test() = default; }; } // namespace test Getting miss formatted when BeforeLambdaBody is true Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44609/new/ http

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3741 tok::colon, tok::l_square, tok::at) || + (Style.BraceWrapping.BeforeLambdaBody && Right.is(tok::l_brace)) || (Left.is(tok::r_paren) &&

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3312 + auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine; + if (Style.BraceWrapping.BeforeLambdaBody && + (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) || ---

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, Wawha, krasimir. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=50702 I believe D44609: [clang-format] N

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. @Wawha do you have a project that perhaps uses this work your did? I would like to ensure I didn't break anything Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104222/new/ https://reviews.llvm.org/D104222 _

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay updated this revision to Diff 351895. MyDeveloperDay added a comment. Remove unused functions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104222/new/ https://reviews.llvm.org/D104222 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

MyDeveloperDay added a subscriber: tstellar. MyDeveloperDay added a comment. I think we've missed the deadline for 12.0.1 which I believe was last friday. I did a practice merge and it didn't quite go in cleanly to 12.0.X branch Normally we would mark the bug in the bug tracker for the next rele

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

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] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. @Wawha your cases could be covered by the following (in mustBreakBefore) if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) && Left.isOneOf(tok::star, tok::amp, tok::ampamp)) { return true; } As I think its the presence of & and *

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. @christophe-calmejane are you able to post your .clang-format styles that you are using, I'm struggling to see where its not matching your style (other than brace styles on the function and argument placing) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay updated this revision to Diff 352136. MyDeveloperDay added a comment. Add some of the failing cases identified by @Wawha CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104222/new/ https://reviews.llvm.org/D104222 Files: clang/lib/Format/TokenAnnotator.cpp clang/unitte

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay updated this revision to Diff 352193. MyDeveloperDay added a comment. Add support for TemplateCloser before LambdaBrace CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104222/new/ https://reviews.llvm.org/D104222 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittes

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

MyDeveloperDay added a comment. This has been landed in the 12.0.1 RC https://github.com/llvm/llvm-project/commit/c7d7ace46258b04aa4b5df08952bfebc6fc4ce94 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94500/new/ https://reviews.llvm.org/D94500 __

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. Part of the problem here is we are not really parsing the Lambda but trying to break based on token annotation only. This can tend to be a bit fragile (normally around corner cases) The previous approach I think was not constrained enough to just lambdas which i

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. > And also another different in that case (without lambda): > > ASSERT_NO_THROW( > { > iterator += 507408; > }); > > is now format like this: > > ASSERT_NO_THROW({ iterator += 507408; }); I think this

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. For this case: > return iter::chain.from_iterable( > [](auto&& ite) -> auto& > { > return ite.second; > }); The code has not been classified as a Lambda AnnotatedTokens(L=0): M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=return L=6 PPK=2 Fa

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. In D104222#2820001 , @HazardyKnusperkeks wrote: > Just asking: Would it be different if there is `auto`, `decltype(auto)`, > `decltype(a+b)`, `typename` or `template` as trailing return type? It would be good if you coul

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

MyDeveloperDay added a comment. > @MyDeveloperDay > Of course, sorry for the delay, here it is: > https://github.com/christophe-calmejane/Hive/blob/master/.clang-format You don't have `AllowShortLambdasOnASingleLine` set to be `false` or `None` If you did The only difference (with this curre

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, exv, lbk, jbcoe. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=50727 When processing C# Lambda expressi

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

MyDeveloperDay updated this revision to Diff 352444. MyDeveloperDay added a comment. I wanted to add some more tests and this really only manifested itself using `lock` and `using` although the use of them is not generally broken when not using them in a Lambda expression CHANGES SINCE LAST A

[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

This revision was automatically updated to reflect the committed changes. Closed by commit rG873308fd8c96: [Format] Fix incorrect pointer/reference detection (authored by Nuu, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

MyDeveloperDay added a comment. This patch needs rebasing, please also make comments as "Done" once you have addressed them. Comment at: clang/lib/Format/TokenAnnotator.cpp:4162 +return FormatStyle::PAS_Middle; + } +} you are missing a return value here

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay updated this revision to Diff 352744. MyDeveloperDay added a comment. I have reworked this patch to utilise the fact that clang-format can format a JSON object in javascript reasonably well. This technique adds for JSON files only a replacement to the front of the JSON transformi

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay updated this revision to Diff 352750. MyDeveloperDay added a comment. Missing new test file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93528/new/ https://reviews.llvm.org/D93528 Files: clang/docs/ClangFormat.rst clang/docs/ClangFormatStyleOptions.rst clang/inclu

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay updated this revision to Diff 352773. MyDeveloperDay added a comment. JSON strings can't be split when the line is too long like c++ strings CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93528/new/ https://reviews.llvm.org/D93528 Files: clang/docs/ClangFormat.rst cla

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:281 + CustomStyle)); + EXPECT_EQ("/* something */ namespace N\n" +"{\n" What does ``` namespace N { /* comment */ ``` or ``` namespace N

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay updated this revision to Diff 353290. MyDeveloperDay added a comment. Ensure git clang-format can handle json CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93528/new/ https://reviews.llvm.org/D93528 Files: clang/docs/ClangFormat.rst clang/docs/ClangFormatStyleOption

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay updated this revision to Diff 353295. MyDeveloperDay added a comment. Add more unit tests and ensure clang-format-diff is setup to check json CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93528/new/ https://reviews.llvm.org/D93528 Files: clang/docs/ClangFormat.rst cl

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:280 + "}", + CustomStyle)); + EXPECT_EQ("/* something */ namespace N\n" I'm not sure I understand this.. why is this not ``` namespace N

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay added a comment. something @sammccall said about support `foo["name"]` made me realise that our javascript support doesn't always support what all the options x = { "firstName" : "John", "lastName" : "Smith", "isAlive" : true, "age" : 27, "address" : {

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104044/new/ https://reviews.llvm.org/D104044 ___

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay updated this revision to Diff 353570. MyDeveloperDay added a comment. - Add release note - Add default Column Limit of 0 for default Json Style (add unit test to ensure column limits can still be used) - this minimises the differences between what Notepad++'s JSON Format plugin

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:284 +- Basic Support has been adding for Formatting .json files (with very limited options) + curdeius wrote: > Maybe instead of putt

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

MyDeveloperDay added a comment. >> Since you use `== ' '` twice, `remainingLineCharCount` will count only >> consecutive spaces, right? >> But you want to count other characters, no? >> So, IIUC, the condition you want is `rF[wI] != '\n' && !(rF[wI] == ' ' && >> rF[wI - 1] == ' ')` (mind the n

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. @HazardyKnusperkeks thank you for the accept, if there are no objections from @curdeius , @krasimir and @sammccall I'd like to proceed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93528/new/ https://r

[PATCH] D102730: [clang-format] Support custom If macros

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102730/new/ https://reviews.llvm.org/D102730 ___ cfe-commits mailing list cfe-com

[PATCH] D104774: [clang-format] Fix a bug that indents else-comment-if incorrectly

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104774/new/ https://reviews.llvm.org/D104774 ___

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

MyDeveloperDay added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104388/new/ https://reviews.llvm.org/D104388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

MyDeveloperDay added a comment. `lock` and `using` will be considered identifiers where as if and while will be seens as if/while M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=99 Name=identifier L=66 PPK=2 FakeLParens= FakeRParens=0 II=0x1fdc7a0 Text='lock' From what I can tell they will be handled by

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

MyDeveloperDay updated this revision to Diff 354205. MyDeveloperDay added a comment. Fix `parseBracedList()` to do what `parseStructuralElement()` does (with regard to breaking `AfterFunction`, I'm not completely convinced `AfterFunction` should have been reused for that but might be too late to

[PATCH] D104774: [clang-format] Fix a bug that indents else-comment-if incorrectly

MyDeveloperDay added a comment. @HazardyKnusperkek Its probably my "bad" I should said "LGTM but maybe wait for the others to comment", but I'm fundamentally ok I think with the change. (we'll just revert if it breaks stuff! ;-)) We are free to add review comments after the fact, @owenpan has

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1022 LLVMStyle.BreakBeforeConceptDeclarations = true; + LLVMStyle.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; LLVMStyle.BreakBeforeTernaryOperators = true; Are

[PATCH] D107267: [clang-format] don't break between function and function name in JS

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107267/new/ https://reviews.llvm.org/D107267 ___

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added subscribers: djasper, klimek. MyDeveloperDay added a comment. In D69764#2932929 , @steveire wrote: > @steveire, thanks for your comments, I also agree that a second tool shouldn't be needed, especially as this functionality is off b

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. In D69764#2934378 , @erichkeane wrote: > I've just been watching this from the sideline, but the cases where this > breaks code are unacceptable for this tool, it is a complete direction change > for the tool, and making t

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. In D69764#2934378 , @erichkeane wrote: > I've just been watching this from the sideline, but the cases where this > breaks code are unacceptable for this tool, it is a complete direction change > for the tool, and making t

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. In D69764#2934483 , @erichkeane wrote: > In D69764#2934473 , @MyDeveloperDay > wrote: > >> In D69764#2934378 , @erichkeane >> wrote: >> >>>

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. > My 'requires changes' is that this needs an LLVM-project-level RFC to change > the charter of one of its projects, doing so in a 15 month long patch, > against the wishes of TWO maintainers is a violation of the LLVM community > practice. I'm completely willin

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. > It _IS_ a democracy where we can find a fair consensus! And the mechanism > with which to obtain said `fair consensus` is an RFC. Then I think in the interest of finding one we should start with the RFC. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. > I was referring to @rsmith and @aaron.ballman (to clarify), both are > maintainers in 'clang', the former of which is the 'superset' maintainer of > this format project based on its directory. While Aaron is a peer-maintainer > to this project, his contributi

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. > I find it weird that we aren't handling ALL of the CV qualifiers. I will probably try and address this, I do have some ideas, but this will I believe complicate the implementation. For now I really want to understand if conceptually such a feature can be land

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. In D69764#2935036 , @klimek wrote: > Happy to go the RFC route if @MyDeveloperDay wants to do that. I'm happy to do that (in fact I've written a draft), do people want to code review the RFC draft (as I could easily be pre

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. In D69764#2934666 , @erichkeane wrote: >> Someone internally pointed out the anti-inclusivity of the terminology, so I >> figured I'd bring it up. I apologise if I'm proliferating that, but could you educate me why its "a

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

MyDeveloperDay added a comment. In D69764#2935269 , @erichkeane wrote: > In D69764#2935218 , @MyDeveloperDay > wrote: > >> In D69764#2934666 , @erichkeane >> wrote: >>

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay updated this revision to Diff 365276. MyDeveloperDay retitled this revision from "[clang-format] Add East/West Const fixer capability" to "[clang-format] Add Left/Right Const fixer capability". MyDeveloperDay added a comment. Remove East/West terminology for inclusivity CHANGES S

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay updated this revision to Diff 365456. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. - Add support for both const and volatile alignment - change `ConstPlacement` to `CVQualifierAlignment` - add `CVQualifierOrder` to allow control over `const volati

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay updated this revision to Diff 365472. MyDeveloperDay added a comment. - elide the braces - use references CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rs

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay added a comment. In D69764#2936827 , @HazardyKnusperkeks wrote: > First off, I think it should be configured in a different way, to prepare the > path for also formatting static, inline, etc. To be honest I think if we wanted to do that w

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay added a comment. > In D69764#2936827 , > @HazardyKnusperkeks wrote: > >> First off, I think it should be configured in a different way, to prepare >> the path for also formatting static, inline, etc. I'm wondering if I'm gravitating more a

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay updated this revision to Diff 365502. MyDeveloperDay added a comment. - Add CVQualifierOrder configuration validation and unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay updated this revision to Diff 365519. MyDeveloperDay added a comment. - Rename the ConstFixer to QualifierAligmentFixer (as now we handle more than just const) and in preparation for handling others CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay added a comment. With the introduction of `CVQualifierOrder` the need for `CVQualifierAlignment:` doesn't make so much sense CVQualifierAlignment: Left CVQualifierOrder: [ "inline", "static", "volatile", "const" ] I'm leaning toward introducing into the `CVQualifierOrder` al

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay added a subscriber: asb. MyDeveloperDay added a comment. > In D69764#2939037 , > @HazardyKnusperkeks wrote: > I would like to remove the CV, only QualifierOrder. Oh gosh, I agree here so much with both you and @curdeius , I keep miss sp

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

MyDeveloperDay updated this revision to Diff 365765. MyDeveloperDay added a comment. - Remove the "CV" from the options - Add support for ordering static/inline/const/volatile around the type, allowing for mixing both left and right alignment at the same time QualifierAlignment: Custom Quali

[PATCH] D107907: [clang-format] handle trailing comments in function definition detection

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107907/new/ https://reviews.llvm.org/D107907 ___

[PATCH] D107950: [clang-format] improve distinction of K&R function definitions vs attributes

MyDeveloperDay added a comment. Thanks for doing this, it LGTM. I personally think tok::identifier tends to be just too general, its hard to use it correctly in the rules especially in the presence of macros. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D107958: [clang-format] NFC update the ClangFormatStyleOption.rst following previous change

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, lunasorcery. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. clang/docs/tool/dump_format_style.py was not run as part of D99840: [clang-format

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added a comment. Did the original change make it into the 13 branch? I'm seeing some unexpected behavior. bool foo(int a, Bar) override; bool foo(int a, Bar) override; // comment becomes bool foo(int a, Bar) override; bool foo(int a, Bar) override; // comment CHANG

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added a comment. Logged as https://bugs.llvm.org/show_bug.cgi?id=51470 @krasimir fix resolves this CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107961/new/ https://reviews.llvm.org/D107961 ___ cfe-commits mailing list cfe-co

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added a comment. FWIW with this additional fix is also still ok and doesn't revert the behaviour, but we'd have to land this in order for @tstellar to move it to the branch (and I think he said that needed to be done by Friday 13th) bool foo(int a, Bar) override; bool foo(in

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. The following doesn't feel consistent bool foo(a, Bar) final; // comment bool foo(a, Bar) final; Bar foo(a, Bar) final; // comment Bar foo(a, Bar) fina

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added a comment. In D107961#2943266 , @owenpan wrote: > In D107961#2943223 , > @MyDeveloperDay wrote: > >> Did the original change make it into the 13 branch? > > ~~What's //the 13 branch//?~~ > >>

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added a comment. This is better as it will catch more cases but there can still be cases we could fall foul. Bar foo(a, Bar) LLVM_OVERRIDE; Bar foo(a, Bar) LLVM_FINAL; Bar foo(a, Bar) LLVM_NOEXCEPT; Is there any way this K&R stuff can be put behind an option, I just f

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. bool foo(int a, Bar) override; bool foo(int a, Bar) override; // comment bool foo(int a, Bar) final; bool foo(int a, Bar) final; // comment bool foo(a, Bar) final;

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1388 break; - if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) && - isC78ParameterDecl(FormatTok)) { + const FormatToken *Next = AllTokens[Tokens->getPosition

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:8258 + verifyFormat("bool f(int a, int) override;\n" + "Bar g(int a, Bar) final; // comment", + Style); can you check with out the comment and wit

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added a comment. @owenpan If its been reverted is it OK to just reopen the review? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107961/new/ https://reviews.llvm.org/D107961 ___ cfe-commit

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107961/new/ https://reviews.llvm.org/D107961 ___ cfe-commits mailing li

[PATCH] D107958: [clang-format] NFC update the ClangFormatStyleOption.rst following previous change

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0391165134fc: [clang-format] NFC update the ClangFormatStyleOption.rst following previous… (authored by MyDeveloperDay). Repository: rG LLVM Githu

[PATCH] D95168: [clang-format] Add InsertBraces option

MyDeveloperDay added a comment. Any thoughts about actually removing braces? eliding braces on single line functions would be very useful for LLVM, its like the most common review comment! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95168/new/

[PATCH] D105479: [clang-tidy] [PR50069] readability-braces-around-statements doesn't work well with [[likely]] [[unlikely]]

This revision was automatically updated to reflect the committed changes. Closed by commit rGfe866327c1f9: [clang-tidy] [PR50069] readability-braces-around-statements doesn't work well… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

MyDeveloperDay added a comment. Nit: There is something niggling in the back of my mind that this is too much logic here to be in parseStructuralElement that sort of suggests to me that this isn't the correct place to handle this. I don't really see any other structural element being handled li

[PATCH] D108094: [clang-format] Improve detection of parameter declarations in K&R C

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1025 if (!isC78Type(*Tok) && !Tok->isOneOf(tok::kw_register, tok::kw_struct, tok::kw_union)) return false; should we have test cases showing these examples?

[PATCH] D108094: [clang-format] Improve detection of parameter declarations in K&R C

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108094/new/ https://reviews.llvm.org/D108094 ___ cfe-commits mailing li

[PATCH] D108094: [clang-format] Improve detection of parameter declarations in K&R C

MyDeveloperDay added a comment. @owenpan Can we push all these commits to 13 RC2 via https://bugs.llvm.org/show_bug.cgi?id=51470 We need to mark the commits we want to cherry pick I think. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108094/new/ https://reviews.llvm.org/D108094 ___

[PATCH] D104900: [clang-format] PR50525 doesn't handle AlignConsecutiveAssignments correctly in some situations

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, darwin. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=50525 AlignConsecutiveAssignments/Declarations ca

<    8   9   10   11   12   13   14   15   16   17   >