MyDeveloperDay added a comment.
I'm a little uncomfortable about all the tests changing, and I'm unsure if we
should be changing the default behaviour.
The rules in the past here was to show that this is part of a public style
guide. The assumption here is google style wants this. Could you please point
to that documentation so at least there is some comeback when we break the
world.
================
Comment at: clang/lib/Format/Format.cpp:956
GoogleStyle.CommentPragmas = "(taze:|^/[ \t]*<|tslint:|@see)";
+ GoogleStyle.InsertTrailingCommas = FormatStyle::TCS_Wrapped;
GoogleStyle.MaxEmptyLinesToKeep = 3;
----------------
Are you sure the right decision is to make this on by default for something
that's going to insert the comma? is this in google's javascript style guide?
I think this could cause clang-format to suddenly start adding lost of commas
(as we see with the tests)
================
Comment at: clang/lib/Format/Format.cpp:1477
+/// TrailingCommaInserter inserts trailing commas into container literals.
+/// E.g.:
----------------
sammccall wrote:
> Inlining this in format.cpp seems worse than having passes in their own
> files, but is the pattern so far. Ugh, up to you.
Actually I think there is precedent to put TokenAnalyzers in their own class, I
don't think it should be in Format.cpp
================
Comment at: clang/unittests/Format/FormatTestJS.cpp:1229
" (param): param is {\n"
- " a: SomeType\n"
+ " a: SomeType;\n"
" }&ABC => 1)\n"
----------------
is this correct?
================
Comment at: clang/unittests/Format/FormatTestJS.cpp:1701
verifyFormat("type X = {\n"
- " y: number\n"
+ " y: number;\n"
"};\n"
----------------
is this correct?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73354/new/
https://reviews.llvm.org/D73354
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
