HazardyKnusperkeks added a comment. In D95168#3102247 <https://reviews.llvm.org/D95168#3102247>, @owenpan wrote:
> In D95168#3100969 <https://reviews.llvm.org/D95168#3100969>, @MyDeveloperDay > wrote: > >> In D95168#3099920 <https://reviews.llvm.org/D95168#3099920>, @owenpan wrote: >> >>> In D95168#3099739 <https://reviews.llvm.org/D95168#3099739>, >>> @MyDeveloperDay wrote: >>> >>>> - 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 >>>> >>>> - Should we add the possibility of removal should we change the option >>>> name to "AutomaticBraces" (thoughts?) >>> >>> As mentioned in D95168#3039033 <https://reviews.llvm.org/D95168#3039033>, I >>> think it would be better to handle the removal separately. The LLVM Coding >>> Standards has an entire section >>> <https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements> >>> about this. Some of the listed exceptions/examples there can make things >>> more difficult. >> >> I'm thinking more about not adding a "InsertBraces" only later to find it >> should have been `InsertRemoveBraces` or `AutomaticBraces` i.e. I want to >> have some idea as to how this might work, if it might be possible even if we >> land separately. > > I think the InsertBraces options can be handled by an `enum`, but the > RemoveBraces options most likely will use a `struct`. Does it make sense to > have both turned on in the same configuration? I'm in favor of a struct of enums: AutomaticBraces: AfterIf: OnlyIfElse AfterElse: Remove #this is obviously only to remove, if the else body is a single line AfterWhile: ... And we can gradually add new enumerators to handle the delicate nuances of adding or removing the braces. And we should add them one after another. ================ 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 && ---------------- Resort ;) ================ Comment at: clang/lib/Format/BraceInserter.cpp:105 + FormatToken *getNext(int &Line, FormatToken *current) { + if (Line == 0 && current == nullptr) { + return Lines[0]->First; ---------------- Remove the { Oh the irony. :) ================ Comment at: clang/lib/Format/BraceInserter.h:1 +//===--- BraceInserter.h - Format C++ code --------------------------------===// +// ---------------- Inserter is misleading, if we want it to be able to remove. Repository: rG LLVM Github Monorepo 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
