[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. Nit: you should use a full context diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54628/new/ https://reviews.llvm.org/D54628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. ok sounds good, @koalo do you have the tests to ensure AfterEnum=true and AllowShortEnumsOnASingleIne=true show on a single line? otherwise perhaps we are already good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54628/new/ https://reviews.llvm.org/D5

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

mitchell-stellar added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3109 return Right.HasUnescapedNewline; + if (isAllmanBrace(Left) && !Style.AllowShortEnumsOnASingleLine && + (Line.startsWith(tok::kw_enum) || MyDeveloperDay wrote:

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. In D54628#1745645 , @koalo wrote: > Yes, that is at least my understanding. I just rebased to master. > > In my understanding, "short" means "put it on a single line if it fits > considering the current maximum line length"

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a subscriber: mitchell-stellar. MyDeveloperDay added a comment. Thanks for rebasing, I think this is a good idea I'm just not sure about how the option presents itself, would you consider changing it? Comment at: clang/lib/Format/TokenAnnotator.cpp:3109

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

koalo updated this revision to Diff 229298. koalo added a comment. Rebased to current master CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54628/new/ https://reviews.llvm.org/D54628 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

koalo added a comment. Yes, that is at least my understanding. I just rebased to master. In my understanding, "short" means "put it on a single line if it fits considering the current maximum line length". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54628/new/ https://reviews.llvm.o

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. Ironically the LLVM style guide says enumerators should be written out as you requested, but from what I can tell its not actually possible to get that format without this sort of change, do you agree? F10735507: image.png R

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. In D54628#1744108 , @koalo wrote: > Yes it does not. Therefore, I have followed the same pattern as the the other > AllowShort* options. I'm not completely sure, but I don't think its quite the same, the AllowShort* opti

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

koalo added a comment. Yes it does not. Therefore, I have followed the same pattern as the the other AllowShort* options. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54628/new/ https://reviews.llvm.org/D54628 ___

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. It feels like it should somehow be following `BreakBeforeBraces` style but understand it might not Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54628/new/ https://reviews.llvm.org/D54628 ___

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

koalo added a comment. The difference is that BraceWrapping.AfterEnum also wraps before the brace. With AfterEnum we have the following true: enum X : int { B }; false: enum X : int { B }; But what I want is this: enum X : int { B }; Repository: rC Clang CHANGES

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. I'm thinking this is the same as BraceWrapping.AfterEnum, if you think your use case is covered would you consider Abandoning this revision so we know this functionality exists already, if not let work out what is missing if (Right.is(TT_InlineASMBrace)) r

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

koalo added a comment. In D54628#1742781 , @MyDeveloperDay wrote: > sorry searching through old issues, are you still interested in this patch? Yes, would be nice to have either this or an equivalent possibility that implements the same feature. Repo

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

MyDeveloperDay added a comment. sorry searching through old issues, are you still interested in this patch? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54628/new/ https://reviews.llvm.org/D54628 ___ cfe-commits mai

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

koalo created this revision. koalo added reviewers: djasper, rsmith. Herald added a subscriber: cfe-commits. Before, clang-format has tried to put enums on a single line whenever possible (unless in styles where the opening brace was put on a seperate line anyway). AllowShortEnumsOnASingleLine th