[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