[PATCH] D93240: [clang-format] Add SpaceBeforeCaseColon option
Björn Schäpers via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 15 12:23:26 PST 2020
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
More information about the cfe-commits
mailing list