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
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
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
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:
> >
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
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
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
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?
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
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
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/
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
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/
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
14 matches
Mail list logo