[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-08-29 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. @pbos can you abandon this revision? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147969/new/ https://reviews.llvm.org/D147969 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) MyDeveloperDay wrote: > owenpan wrote: > > aaro

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) owenpan wrote: > aaron.ballman wrote: > > Haza

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-11 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) aaron.ballman wrote: > HazardyKnusperkeks wrote: > >

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) HazardyKnusperkeks wrote: > MyDeveloperDay wrot

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-11 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) MyDeveloperDay wrote: > This is an code mo

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. RFC https://groups.google.com/g/llvm-dev/c/wka1Bnrd-aU?pli=1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147969/new/ https://reviews.llvm.org/D147969 ___ cfe-commits mai

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-10 Thread Peter Boström via Phabricator via cfe-commits
pbos added a comment. That's reasonable. If I don't hear something else we'll own InsertBraces on our end. Is there anything I can point to regarding the decision to not let clang-format add/rearrange tokens by default so that I can refer to that in a comment saying why it won't be upstreamed?

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. These features tend to have an additional performance penalty that not all may want to pay, even if they did base their style on your style. I.E. chromium isn’t likely the only ones using chromium style Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This was the agreement reached when we pushed for clang format to start being allowed to add or rearrange tokens, I.E. in this case we add {} as such we could, as you saw yourselves make mistakes if our assumptions are not 100% correct and break code, we prefer p

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-10 Thread Peter Boström via Phabricator via cfe-commits
pbos added a comment. What defines a code-modifying feature? This shouldn't change control flow, so are superfluous {}s seen as code-modifying while additional spaces/tabs/whatnot are not? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147969/new/

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-10 Thread Peter Boström via Phabricator via cfe-commits
pbos added a comment. Does this need additional testing besides clang/unittests/Format/BracesInserterTest.cpp ? I haven't seen tests that EXPECT_TRUE(getChromiumStyle(lang).InsertBraces); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147969/new/

[PATCH] D147969: Add InsertBraces to ChromiumStyle

2023-04-10 Thread Peter Boström via Phabricator via cfe-commits
pbos created this revision. pbos added a reviewer: hans. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. pbos requested review of this revision. Herald added a comment. NOTE: Clang