[PATCH] D95168: [clang-format] Add InsertBraces option
Owen Pan via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 3 01:33:31 PDT 2021
owenpan added a comment.
In D95168#3102511 <https://reviews.llvm.org/D95168#3102511>, @MyDeveloperDay wrote:
> Then this is definitely why we want to think about these now and NOT leave them to a separate review after the Insert case is committed.
We can just leave a placeholder for the RemoveBraces options and deal with them when we get there. Unless I misunderstood, your suggestion of the AutomaticBraces option (with `enum` values Insert, Remove, LLVM, etc) would accomplish that.
> If we think remove braces would be a struct then we'd better go for a struct now and not later so there is some symmetry, I'm slightly wondering what would be in the struct?
I already had an implementation of RemoveBraces for the LLVM style (minus some exceptions). I started with an `enum` to accommodate different levels of aggressiveness of removal with LLVM at the minimum end. After looking at several prominent styles, I realized that a `struct` would work better.
> 1. Do we want to remove {} from an else or elseif when the if needs braces
>
> if (x==1) {
> foo();
> bar();
> }
> else if (x==2)
> baz();
> else
> foo();
>
> 2. Do we want to remove braces from a commented if (this can throw your eye out when looking)
>
> if (x==1)
> // some longish comment
> return x;
>
> 3. Do we want sub scope to be removed of braces too?
>
> if (x==1)
> for (int i=0;i<10;i++)
> foo();
>
> 4. Do we want to prevent the remove for MultiLine scope, where what follows would be spread over multiple lines (a little like the comment really)
>
> if (x== 1)
> os << "This is a very very very long"
> << data
> << "More data"
> << "endl;"
> `
I've seen both yes and no for all the above.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95168/new/
https://reviews.llvm.org/D95168
More information about the cfe-commits
mailing list