[webkit-dev] Resolution on switch statement indentation
I saw another patch get rejected today because of switch statement indentation. We discussed this last week, and I saw a lot of support for my proposal of indenting case labels from their switch. But the discussion did not end in resolution. To summarize, here are the options mentioned: 1) Case labels always have the same indentation as their switch (today's rule) 2) Case labels always indent 4 spaces in from their switch (my preference) 3) Case labels indent 2 spaces in from their switch. (Maciej's rule) I was a little unclear on Maciej's rule. The last part of his rule is 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). Did he mean that the contained statements would be indented 4 spaces from the case label, meaning they would be indented 6 spaces from the switch? That's the only way the closing brace could be indented 2 spaces from the switch and the code indented 4 spaces from the brace. If so, I especially dislike this rule because it places the entire body of a block at a nonstandard indentation. Anyway, how do we come to resolution on this? - ~Chris cmar...@apple.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Wednesday 09 December 2009 10:26:24 am Chris Marrin wrote: I saw another patch get rejected today because of switch statement indentation. We discussed this last week, and I saw a lot of support for my proposal of indenting case labels from their switch. But the discussion did not end in resolution. To summarize, here are the options mentioned: 1) Case labels always have the same indentation as their switch (today's rule) ... Anyway, how do we come to resolution on this? What is wrong with keeping the current rule? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Dec 9, 2009, at 7:26 AM, Chris Marrin wrote:
I saw another patch get rejected today because of switch statement
indentation. We discussed this last week, and I saw a lot of support
for my proposal of indenting case labels from their switch. But the
discussion did not end in resolution. To summarize, here are the
options mentioned:
1) Case labels always have the same indentation as their switch
(today's rule)
2) Case labels always indent 4 spaces in from their switch (my
preference)
3) Case labels indent 2 spaces in from their switch. (Maciej's rule)
I was a little unclear on Maciej's rule. The last part of his rule
is 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). Did he mean that the contained statements
would be indented 4 spaces from the case label, meaning they would
be indented 6 spaces from the switch?
I meant 4 spaces from the switch (i.e. 2 additional spaces from the
case label).
switch (x) {
case foo: {
fooFunc();
}
case bar:
barFunc();
}
- Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Dec 9, 2009, at 8:13 AM, Adam Treat wrote: On Wednesday 09 December 2009 10:26:24 am Chris Marrin wrote: I saw another patch get rejected today because of switch statement indentation. We discussed this last week, and I saw a lot of support for my proposal of indenting case labels from their switch. But the discussion did not end in resolution. To summarize, here are the options mentioned: 1) Case labels always have the same indentation as their switch (today's rule) ... Anyway, how do we come to resolution on this? What is wrong with keeping the current rule? As I pointed out in the previous thread, I feel like it makes the code harder to read, and got several responses of agreement. Also most of the switch statements in the code currently indent the case labels, so it will mean lots of code changes. I think it would be better to change the rule. - ~Chris cmar...@apple.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Dec 9, 2009, at 9:41 AM, Maciej Stachowiak wrote:
On Dec 9, 2009, at 7:26 AM, Chris Marrin wrote:
I saw another patch get rejected today because of switch statement
indentation. We discussed this last week, and I saw a lot of
support for my proposal of indenting case labels from their switch.
But the discussion did not end in resolution. To summarize, here
are the options mentioned:
1) Case labels always have the same indentation as their switch
(today's rule)
2) Case labels always indent 4 spaces in from their switch (my
preference)
3) Case labels indent 2 spaces in from their switch. (Maciej's rule)
I was a little unclear on Maciej's rule. The last part of his rule
is 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). Did he mean that the contained statements
would be indented 4 spaces from the case label, meaning they would
be indented 6 spaces from the switch?
I meant 4 spaces from the switch (i.e. 2 additional spaces from the
case label).
switch (x) {
case foo: {
fooFunc();
}
case bar:
barFunc();
}
Ok, the example above seems to be missing a space (it indents the case
label 1 and the block 3). But I assume you meant to indent 2 and 4. If
so, I understand what you are proposing. I still don't like it, but I
understand it. I think a consistent 4 space indentation scheme avoids
confusion and makes all the indentation tools in editors work
correctly. If excessive indentation really is that big of a concern
(which I don't think it is) I would rather see the current rule (rule
1) used.
-
~Chris
cmar...@apple.com
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Wednesday 09 December 2009 01:03:23 pm Chris Marrin wrote: What is wrong with keeping the current rule? As I pointed out in the previous thread, I feel like it makes the code harder to read, and got several responses of agreement. Also most of the switch statements in the code currently indent the case labels, so it will mean lots of code changes. I think it would be better to change the rule. Ok, well FWIW I disagree that the current rule makes things hard to read and I do not like the idea of changing it. I object on the same grounds as the other recent styling change proposals as well as substantively that it makes anything easier to read. Cheers, Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Dec 9, 2009, at 10:57 AM, Adam Treat wrote: Ok, well FWIW I disagree that the current rule makes things hard to read and I do not like the idea of changing it. I object on the same grounds as the other recent styling change proposals as well as substantively that it makes anything easier to read. Sorry to keep this topic open — I try to stay out of these — but this one is slightly different than the namespace indenting or whatever else was recently discussed in that there is still a roughly 50/50 split in existing code despite the style guide listing an unambiguous rule. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Wednesday 09 December 2009 02:04:07 pm Darin Adler wrote: On Dec 9, 2009, at 10:57 AM, Adam Treat wrote: Ok, well FWIW I disagree that the current rule makes things hard to read and I do not like the idea of changing it. I object on the same grounds as the other recent styling change proposals as well as substantively that it makes anything easier to read. Sorry to keep this topic open — I try to stay out of these — but this one is slightly different than the namespace indenting or whatever else was recently discussed in that there is still a roughly 50/50 split in existing code despite the style guide listing an unambiguous rule. -- Darin Yep, that does distinguish. That being said, my personal preference would be either to keep the current rule and change the code to meet it or even better just eliminate the rule altogether since the community hasn't been following it anyways. That speaks negatively to the real world value of the rule IMHO. Cheers, Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Wed, Dec 9, 2009 at 10:11 AM, Chris Marrin cmar...@apple.com wrote: 1) Case labels always have the same indentation as their switch (today's rule) If excessive indentation really is that big of a concern (which I don't think it is) I would rather see the current rule (rule 1) used. From the last email thread: 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 Dave who likes consistency within a code base but doesn't care too much about the details of that consistency Levin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Dec 9, 2009, at 11:25 AM, Adam Treat wrote: That speaks negatively to the real world value of the rule IMHO. A lot of the code predates the rule, so I wouldn’t judge the effectiveness of the rule too harshly. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Resolution on switch statement indentation
On Dec 9, 2009, at 10:11 AM, Chris Marrin wrote:
On Dec 9, 2009, at 9:41 AM, Maciej Stachowiak wrote:
I meant 4 spaces from the switch (i.e. 2 additional spaces from the
case label).
switch (x) {
case foo: {
fooFunc();
}
case bar:
barFunc();
}
Ok, the example above seems to be missing a space (it indents the
case label 1 and the block 3). But I assume you meant to indent 2
and 4. If so, I understand what you are proposing. I still don't
like it, but I understand it. I think a consistent 4 space
indentation scheme avoids confusion and makes all the indentation
tools in editors work correctly. If excessive indentation really is
that big of a concern (which I don't think it is) I would rather see
the current rule (rule 1) used.
Just to be unambiguous, here is my proposal set in a monospace font:
01234567890123456789
switch (x) {
case foo: {
fooFunc();
}
case bar:
barFunc();
}
(Hoping this doesn't get mangled in email.) Reasons I suggested this
rule:
A) Actual statements end up indented 4 spaces, preventing excessive
indent, and perhaps aiding readability in the no-brace case.
B) You can't end up with a brace at the same indent as the switch that
doesn't actually close it; or two braces at the same indent level in a
row.
I think using a partial or half indent for case labels is not a wild
or uncommon style. Some also use half-indents for goto labels, but no
one seems to have a problem with leaving those unindented as far as I
can tell.
That said, if no one prefers this to the status quo, we should stick
to the current rule. But I don't think we should allow multiple
different indent styles.
Regards,
Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
