[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.


================
Comment at: clang/include/clang/Format/Format.h:946
+    /// Remove if there is no comment
+    BIS_RemoveNoComment
+  };
----------------
HazardyKnusperkeks wrote:
> Maybe differentiate between single line and multi line comments?
do you think we might want do remove braces from:

```
if (x) {
  // Remove the braces
  return true;
}
```

but not from?

```
if (x) {
  // Don't Remove the braces
  // As the comment it longer
  return true;
}
```

I could see that


================
Comment at: clang/include/clang/Format/Format.h:3675
                R.AlwaysBreakTemplateDeclarations &&
            AttributeMacros == R.AttributeMacros &&
            BinPackArguments == R.BinPackArguments &&
----------------
HazardyKnusperkeks wrote:
> Add it here
So I notice we don't do this for BraceWrapping, I was going to swing by on this one and see why not.


================
Comment at: clang/include/clang/Format/Format.h:3696
            IndentRequires == R.IndentRequires && IndentWidth == R.IndentWidth &&
-           Language == R.Language &&
+           AutomaticBraces == R.AutomaticBraces && Language == R.Language &&
            IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
----------------
HazardyKnusperkeks wrote:
> Resort ;)
ouch! thats someone elses bug... but I can look at it.


================
Comment at: clang/lib/Format/BraceInserter.h:1
+//===--- BraceInserter.h - Format C++ code --------------------------------===//
+//
----------------
HazardyKnusperkeks wrote:
> Inserter is misleading, if we want it to be able to remove.
Yes I'm going to rename the files, AutomaticBraces.cpp/.h  AutomaticBracesTests.cpp what do you think?


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

https://reviews.llvm.org/D95168