Re: [PATCH] CodingGuidelines: clarify multi-line brace style
From: Junio C Hamano <hidden>
Date: 2017-01-17 21:41:13
Jeff King [off-list ref] writes:
Yeah. I obviously was adapting the original text, and I think I left too much of the wishy-washy-ness in. As the point of the patch is to avoid that, let's take your suggestion. A re-rolled patch is below. Now the patch is at least self-consistent. The bigger question remains of: do we want to dictate these rules, or did we prefer the vague version?
I would say no to make all of them "rules to be dictated". It is OK to leave some things "gray". But obviously others can vote differently ;-)
I _thought_ the vague rules were doing fine, but this whole discussion has made me think otherwise. And I'd just as soon not ever have to repeat it. ;-/ OTOH, I really do not want to review a bunch of patches that do nothing but change brace style (and I am sure there is a mix of styles already in the code base).
Yes, exactly, that is why the last sentence is in there below.
quoted
When the project says it does not have strong preference among multiple choices, you are welcome to write your new code using any of them, as long as you are consistent with surrounding code. Do not change style of existing code only to flip among these styles, though.
The existing document says:
Make your code readable and sensible, and don't try to be clever.
As for more concrete guidelines, just imitate the existing code
(this is a good guideline, no matter which project you are
contributing to). It is always preferable to match the _local_
convention. New code added to Git suite is expected to match
the overall style of existing code. Modifications to existing
code is expected to match the style the surrounding code already
uses (even if it doesn't match the overall style of existing code).
But if you must have a list of rules, here they are.
I guess that is the place to expound on how to interpret the rules. I
dunno. Some of the individual rules that go into "gray areas" already
spell out the "surrounding code" rule.We are not saying one important thing and that was what invited useless arguing this time: For "gray" things, the choice is yours in your new code, but do not churn existing code for "gray" guidelines. If we made _that_ clear as a rule we dictate, hopefully we'd have less chance to see "a bunch of patches that do nothing but change brace style", I would think.
quoted hunk
-- >8 -- Subject: [PATCH] CodingGuidelines: clarify multi-line brace style There are some "gray areas" around when to omit braces from a conditional or loop body. Since that seems to have resulted in some arguments, let's be a little more clear about our preferred style. Signed-off-by: Jeff King <redacted> --- Documentation/CodingGuidelines | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 4cd95da6b..a4191aa38 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines@@ -206,11 +206,38 @@ For C programs: x = 1; } - is frowned upon. A gray area is when the statement extends - over a few lines, and/or you have a lengthy comment atop of - it. Also, like in the Linux kernel, if there is a long list - of "else if" statements, it can make sense to add braces to - single line blocks. + is frowned upon. But there are a few exceptions: + + - When the statement extends over a few lines (e.g., a while loop + with an embedded conditional, or a comment). E.g.: + + while (foo) { + if (x) + one(); + else + two(); + } + + if (foo) { + /* + * This one requires some explanation, + * so we're better off with braces to make + * it obvious that the indentation is correct. + */ + doit(); + } + + - When there are multiple arms to a conditional and some of them + require braces, enclose even a single line block in braces for + consistency. E.g.: + + if (foo) { + doit(); + } else { + one(); + two(); + three(); + } - We try to avoid assignments in the condition of an "if" statement.