HazardyKnusperkeks marked an inline comment as done. HazardyKnusperkeks added a comment.
In D93240#2454268 <https://reviews.llvm.org/D93240#2454268>, @MyDeveloperDay wrote: > I generally don't have the same aversion to new options than others have, but > can you point out a style guide that might want this option. (that is kind of > the process) Mostly: My own style :) But I was really surprised that this is not in already, when nearly all other colons can be given a space and actually are in the default. But I can show something I got from the web: 1. Uncrustify has the Option, so there was the need https://github.com/uncrustify/uncrustify/blob/1d3d8fa5e81bece0fac4b81316b0844f7cc35926/etc/uigui_uncrustify.ini#L1013 2. On this tutorial it is actually using a space: https://www.tutorialspoint.com/cplusplus/cpp_switch_statement.htm 3. And the cppreference uses both in its article on switch https://en.cppreference.com/w/cpp/language/switch ================ Comment at: clang/unittests/Format/FormatTest.cpp:12115 + " break;\n" "}", InheritanceStyle); ---------------- MyDeveloperDay wrote: > maybe add a `default` example with {} > > try not to change existing tests just add your own new ones First one: Will do Second one: See below ================ Comment at: clang/unittests/Format/FormatTest.cpp:12158 "default:\n" + " break;\n" "}", ---------------- MyDeveloperDay wrote: > Add new test leave the old one alone Do you mean a whole new function? I would strongly disagree, because this function is the right one since its called `ConfigurableSpaceBeforeColon`. And without it I would not have got the wrong assignment of the colon of a default statement. Or do you mean the function is okay, but I should not touch this string (and probably the other strings too)? I would mildly disagree because I all to have to have the same unformatted text. But this one I'm willing to revert if that's what it's takes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93240/new/ https://reviews.llvm.org/D93240 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
