On Wed, Dec 2, 2009 at 9:19 PM, Mark Rowe <mr...@apple.com> wrote: > On 2009-12-02, at 21:00, Peter Kasting wrote: > > > I find this tricky to read and error-prone. I propose that the rule be > modified to be: > > > > * When all arms of a conditional or loop are one physical line, do not > use braces. If any arms are more than one physical line (even if they are > one logical line), use braces on all arms. > > I do not agree that this would be an improvement. >
Are you satisfied with the existing rule, then? If so, you would be the first developer I have asked who is. Admittedly my sample size is quite small, but all those I've asked would prefer either a rule like my proposal or a rule that says "braces always", which I find to be consistent and easy to remember but somewhat verbose (and with a greater impact on the existing code, for what that's worth). > In most places this will not differ from the existing code, so it will not > "cause the whole codebase to become invalid"; > > I glanced briefly at three files: RenderObject.cpp, Element.cpp and > Node.cpp. In these three files I counted over two dozen places that would > require modification to conform with this new rule. That's by no means a > majority of the relevant statements in these files, but it's not a small > number either. I stand by my statement. In Element.cpp, 9 conditionals violate this out of roughly 200 conditionals and loops. One of these violates the existing style guide too. "Not a majority" is a significant understatement. Especially when compared to our namespace change that affects nearly every header, or the proposed case indenting change that affects every switch statement. That said, I don't think "doesn't change a large percentage of the code" is a significant argument for a rule, it mostly speaks to how people wouldn't need to massively shift their thinking when writing code because the rules have changed. PK
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
