The style script flagged an issue in my code yesterday for an issue I didn't even know existed. How do you indent case clauses for a switch statement? The WebKit style states that case clauses have the same indentation as their switch. I HATE that style. And I had no idea that was the WebKit style. I use the "indent the case" style and have never had anyone flag it in the past. Without getting into style religion, I was looking at the code and it seems that there are many more uses of the "indent the case" style than the "correct" style. Maybe we could change the style rule in the interest of changing fewer files (and because I think it generally reads better)? I'm fine with changing my code to match the style. But the style script is going to be kicking out a lot of these errors and I think we should make sure we want to go down this road before that happens. ----- ~Chris cmarrin@apple.com
On 02.12.2009, at 15:25, Chris Marrin wrote:
Maybe we could change the style rule in the interest of changing fewer files (and because I think it generally reads better)?
I support changing or dropping this rule. Because of this rule, there is no good way to format cases that need braces, such as: switch (i) { case 1: { String a("a"); break; } case 2: { String b("b"); break; } } The downside is that some code can get indented too far, which is particularly unfortunate for large switches. But I'm not convinced that having a standard for this improves consistency of the code in any meaningful way (*), perhaps this should be decided on a case by case basis. (*) meaningful == helps to read the code, or to search, or to process with scripts, or maybe even to copy/paste. - WBR, Alexey Proskuryakov
The downside is that some code can get indented too far, which is particularly unfortunate for large switches.
We could issue a fuzzy declaration such as, "Indent case blocks, except in situations where an unreasonable amount of code would end up so-indented, causing readability problems". Two examples of situations where indenting case blocks would cause readability problems are the JavaScriptCore interpreter and, to a lesser extent, the JavaScriptCore JIT. All of Interpreter.cpp would suddenly be indented by an extra 4 spaces, sucking up valuable horizontal real estate. Geoff
On Dec 2, 2009, at 7:56 PM, Geoffrey Garen wrote:
The downside is that some code can get indented too far, which is particularly unfortunate for large switches.
We could issue a fuzzy declaration such as, "Indent case blocks, except in situations where an unreasonable amount of code would end up so-indented, causing readability problems".
Two examples of situations where indenting case blocks would cause readability problems are the JavaScriptCore interpreter and, to a lesser extent, the JavaScriptCore JIT. All of Interpreter.cpp would suddenly be indented by an extra 4 spaces, sucking up valuable horizontal real estate.
I believe one rule that could work is something like this: - Indent case labels inside a switch two spaces. - Indent actual statements inside a switch four spaces. - In the case where a case label is followed by a block, include the open brace on the same line as the case label and indent the matching close brace only two spaces (but still 4 spaces for the contained statements). That would not cause any code to be excessively indented, but would avoid some of the downsides of not indenting at all mentioned by Chris. I would rather have a clear rule that makes sense in a variety of situations than a fuzzy guideline. Regards, Maciej
On Wed, Dec 2, 2009 at 8:05 PM, Maciej Stachowiak <mjs@apple.com> wrote:
I believe one rule that could work is something like this:
- Indent case labels inside a switch two spaces. - Indent actual statements inside a switch four spaces. - In the case where a case label is followed by a block, include the open brace on the same line as the case label and indent the matching close brace only two spaces (but still 4 spaces for the contained statements).
This is precisely the rule that the Google C++ Style Guide uses ( http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Loops_and_Swi...). I think it works well. I support it. PK
Sounds good to me. Geoff On Dec 2, 2009, at 8:58 PM, Peter Kasting wrote:
On Wed, Dec 2, 2009 at 8:05 PM, Maciej Stachowiak <mjs@apple.com> wrote: I believe one rule that could work is something like this:
- Indent case labels inside a switch two spaces. - Indent actual statements inside a switch four spaces. - In the case where a case label is followed by a block, include the open brace on the same line as the case label and indent the matching close brace only two spaces (but still 4 spaces for the contained statements).
This is precisely the rule that the Google C++ Style Guide uses ( http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Loops_and_Swi... ). I think it works well. I support it.
PK
participants (5)
-
Alexey Proskuryakov
-
Chris Marrin
-
Geoffrey Garen
-
Maciej Stachowiak
-
Peter Kasting