On 05/06/2016 06:07 PM, Jakob Stoklund Olesen wrote:
On May 6, 2016, at 09:59, Jason Orendorff <[email protected]> wrote: On Fri, May 6, 2016 at 10:43 AM, Jakob Stoklund Olesen <[email protected] <mailto:[email protected]>> wrote: Unfortunately, the way SpiderMonkey indents case labels is too odd for clang-format. I don’t think it has a configuration flag that can do that half-indent. Feel free to mass-change it to whatever Gecko does and update the style guide. We'll cope.The mozilla style is to indent the case label by one level from the switch, and the code inside the case by one further level. With 4-space indent, it looks like this: switch (tag) { case SCRIPT_INT: { uint32_t i; if (mode == XDR_ENCODE) i = uint32_t(vp.toInt32()); if (!xdr->codeUint32(&i)) return false; if (mode == XDR_DECODE) vp.set(Int32Value(int32_t(i))); break; } case SCRIPT_DOUBLE: { double d; if (mode == XDR_ENCODE) d = vp.toDouble(); if (!xdr->codeDouble(&d)) return false; if (mode == XDR_DECODE) vp.set(DoubleValue(d)); break; } Applied to the current SM code base, this style change would move all lines inside a switch, not just the case labels. I think that if we can cope with such an invasive mass change, we should instead go with Terrence’s suggestion and simply adopt the same style as the rest of Gecko, including the 2-space indent.
I would not go for indenting case labels by 4, as this would basically make us avoid using switch-case statements in favor of "else-if" which does not gives us the same guarantee.
One other solution would be to remove the half-indent and replace them by no indent, i-e all the visibility modifiers would be on the same line as the class keyword, and all the case would be on the same line as the switch statement.
-- Nicolas B. Pierron _______________________________________________ dev-tech-js-engine-internals mailing list [email protected] https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
