{"thread":{"id":"30760","subject":"[PATCH/RFC] Documenation update: use of braces in if/else if/else chain","startedAt":"2012-06-10T17:26:30Z","lastAt":"2012-06-10T20:52:18Z","messageCount":3,"participants":["Leila Muhtasib","Jonathan Nieder","Leila"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"193268","messageId":"1339349190-84552-1-git-send-email-muhtasib@gmail.com","threadId":"30760","inReplyTo":null,"subject":"[PATCH/RFC] Documenation update: use of braces in if/else if/else chain","fromName":"Leila Muhtasib","fromEmail":"muhtasib@gmail.com","sentAt":"2012-06-10T17:26:30Z","receivedAt":"2012-06-10T17:26:30Z","isPatch":true,"sender":{"key":"muhtasib@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1618875?v=4"},"body":"\nSigned-off-by: Leila Muhtasib <muhtasib@gmail.com>\n---\nDoes the below more accruately represent the coding guidelines?\nI can modify it, if it doesn't.\n\nThanks,\nLeila\n\n\n Documentation/CodingGuidelines |   23 ++++++++++++++++++++---\n 1 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 4557711..ea90521 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -117,9 +117,26 @@ For C programs:\n \n    is frowned upon.  A gray area is when the statement extends\n    over a few lines, and/or you have a lengthy comment atop of\n-   it.  Also, like in the Linux kernel, if there is a long list\n-   of \"else if\" statements, it can make sense to add braces to\n-   single line blocks.\n+   it.  Also, like in the Linux kernel, if one of the\n+   \"if/else if/else\" chain has a multiple statement block, use {}\n+   even for a single statement block in that chain. And \"else\"\n+   should come on the same line as the closing \"}\" of its \"if\" block.\n+\n+\t//correct\n+\tif (bla) {\n+\t\tx = 1;\n+\t\t...\n+\t} else {\n+\t\tx = 2;\n+\t}\n+\n+\t//incorrect\n+\tif (bla) {\n+\t\tx = 1;\n+\t\t...\n+\t}\n+\telse\n+\t\tx = 2;\n \n  - We try to avoid assignments inside if().\n \n-- \n1.7.7.5 (Apple Git-26)\n"},{"id":"193280","messageId":"20120610202205.GA2052@burratino","threadId":"30760","inReplyTo":"1339349190-84552-1-git-send-email-muhtasib@gmail.com","subject":"Re: [PATCH/RFC] Documenation update: use of braces in if/else if/else chain","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-10T20:22:05Z","receivedAt":"2012-06-10T20:22:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nLeila Muhtasib wrote:\n\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -117,9 +117,26 @@ For C programs:\n>  \n>     is frowned upon.  A gray area is when the statement extends\n>     over a few lines, and/or you have a lengthy comment atop of\n> -   it.  Also, like in the Linux kernel, if there is a long list\n> -   of \"else if\" statements, it can make sense to add braces to\n> -   single line blocks.\n> +   it.  Also, like in the Linux kernel, if one of the\n> +   \"if/else if/else\" chain has a multiple statement block, use {}\n> +   even for a single statement block in that chain. And \"else\"\n> +   should come on the same line as the closing \"}\" of its \"if\" block.\n\nI don't think that's quite accurate.  Current best practice in both\ngit and the Linux kernel is a little looser than that.\n\n> +\n> +\t//correct\n> +\tif (bla) {\n> +\t\tx = 1;\n> +\t\t...\n> +\t} else {\n> +\t\tx = 2;\n> +\t}\n\nTrue.\n\n> +\n> +\t//incorrect\n> +\tif (bla) {\n> +\t\tx = 1;\n> +\t\t...\n> +\t}\n> +\telse\n> +\t\tx = 2;\n\nAlso true.  But:\n\n\t/* correct */\n\tif (bla) {\n\t\tx = 1;\n\t\t...\n\t} else\n\t\tx = 2;\n\nAnd:\n\nIf you have a long \"if\" with a one-line \"else\", consider whether you\nare needlessly keeping the reader in suspense about something simple.\nIt might be more pleasant to read with the exceptional case up front:\n\n\tif (!bla) {\n\t\tx = 2;\n\t} else {\n\t\tx = 1;\n\t\t...\n\t}\n\nThis is especially true when the exceptional case returns or exits.\n\n\tif (bla && no_bla)\n\t\treturn error(\"--blah and --no-blah cannot be used together\");\n\tx = 1;\n\t...\n\nHope that helps,\nJonathan\n"},{"id":"193283","messageId":"CAA3EhHLepTC2WThDvAWk5WPcMPQapqo=D7KhpdCESy9D9JO4vg@mail.gmail.com","threadId":"30760","inReplyTo":"20120610202205.GA2052@burratino","subject":"Re: [PATCH/RFC] Documenation update: use of braces in if/else if/else chain","fromName":"Leila","fromEmail":"muhtasib@gmail.com","sentAt":"2012-06-10T20:52:18Z","receivedAt":"2012-06-10T20:52:18Z","isPatch":true,"sender":{"key":"muhtasib@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1618875?v=4"},"body":"On Sun, Jun 10, 2012 at 4:22 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Also true.  But:\n>\n>        /* correct */\n>        if (bla) {\n>                x = 1;\n>                ...\n>        } else\n>                x = 2;\n>\n\nAccording to a discussion on my patch, this was cited (not in ref to my code):\nJunio C Hamano <gitster@pobox.com> a écrit :\n\nTwo points on style (also appear elsewhere in this patch):\n\n       if (!\"applying\") {\n               ...\n       } else {\n               state->rebase_in_progress = 1;\n       }\n\n - \"else\" comes on the same line as closing \"}\" of its \"if\" block;\n\n - if one of if/else if/else chain has multiple statement block, use {}\n  even for a single statement block in the chain.\n\n\nSo I opened this documentation patch to gain consensus around this\nissue, and update the docs accordingly. Depending on the agreement\nreached, I can modify the wording of the patch. I don't need to site\nthe linux Kernel part, I can just say what is supported in this\nproject.\n\n\nI think we should still keep this part of the patch:\n\n+    And \"else\"\n+   should come on the same line as the closing \"}\" of its \"if\" block.\n\n\n> And:\n>\n> If you have a long \"if\" with a one-line \"else\", consider whether you\n> are needlessly keeping the reader in suspense about something simple.\n> It might be more pleasant to read with the exceptional case up front:\n>\n>        if (!bla) {\n>                x = 2;\n>        } else {\n>                x = 1;\n>                ...\n>        }\n>\n> This is especially true when the exceptional case returns or exits.\n>\n>        if (bla && no_bla)\n>                return error(\"--blah and --no-blah cannot be used together\");\n>        x = 1;\n>        ...\n\nAgreed!\n\nThanks,\nLeila\n"}]}