HazardyKnusperkeks added a comment.

In D95168#3099739 <https://reviews.llvm.org/D95168#3099739>, @MyDeveloperDay 
wrote:

> I'd like to carry the mantle for this, before making any further changes I'd 
> like to address some of the review comments
>
> 1. Add documentation warning about a modifying change (as agreed by the RFC)
> 2. Add version introduced information
> 3. Fix issue with Macro continuation (don't add the \n before the } unless 
> you see a \\ line comment, otherwise let clang-format deal with the \n 
> depending on the BraceWrapping style
> 4. Add unit tests for Macro continuation and Comment handling
> 5. Allow InsertBraces in other languages
>
> TODO:
>
> - break out into its own file (Format.cpp is getting too big)

This also holds for FormatTest.cpp.

> - Look further into possible Removal (I have an idea for how this might be 
> possible, and super useful for LLVM where we don't like single if {} ), I'd 
> like to round out on this before introducing the options rather than having 
> to change them later

Yeah we should have that.

> - Should we add the possibility of removal should we change the option name 
> to "AutomaticBraces" (thoughts?)

Name change is good, the options then also need to be renamed.

> - @tiagoma please add your name and email addreess to this review so when the 
> time comes to land this I can attribute to you as "Author".





================
Comment at: clang/include/clang/Format/Format.h:999
+  /// The default is the disabled value of ``BIS_Never``.
+  /// \warning
+  ///  Setting ``InsertBraces``  to something other than `Never`, COULD
----------------
Could we just add a big notification on top, that some specially marked options 
may break the code, and then only mark them, so one does not need to write that 
much for all possible options?


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

https://reviews.llvm.org/D95168

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to