[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:401
+  * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``)
+    Allow short if/else if statements even if the else is a compound statement.
+
----------------
MyDeveloperDay wrote:
> klimek wrote:
> > I'd try to make this either unconditionally what we do, or decide against doing it.
> see note before, this is really about maintaining compatibility, I don't want to make the assumption that everyone likes
> 
> 
> ```
> if (x) return 1;
> else if (x) return 2;
> else return 3;
> 
> ```
> 
> There could easily be users who want this.
> 
> ```
> if (x) return 1;
> else if (x) return 2;
> else 
>    return 3;
> ```
> 
> I don't think it over complicates it, but it keeps the flexibility. The name of the option may not be the best 'suggestions welcome'
The second one seems simply broken. I think fixing this is great, but keeping the broken version seems odd :)


================
Comment at: clang/unittests/Format/FormatTest.cpp:520
+  verifyFormat("if (a) f();\n"
+               "else if (b) g();\n",
+               AllowsMergedIf);
----------------
krasimir wrote:
> I don't understand why is this preferred over:
> ```
> if (a) f();
> else
>   if (b) g();
> ```
> 
> Isn't syntactically the nested `if (b) g();` a compound statement? I might be wrong.
There's no compound statement, but the else-stmt is another if-stmt. That said, I don't think this is about what structure the C++ standard has (as that's not what clang-format understands anyway), but whether the structure we put in helps readability. In this case, why does the irregular break before the if help readability? It seems like we're having a break for no good reason.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59408/new/

https://reviews.llvm.org/D59408