[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-20 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added a comment. @wanders, thank you for digging through code to get a feeling of the usage! I agree with others to have an enum option. +1 to `None` over `Neither`. I'd prefer `Before` / `After` instead of `Left` / `Right`; those seem more prevalent in names and values of existing opti

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Let’s do all 4, None, Both, Left, Right Neither is an odd word for non English speakers Also I before e can confuse the slightly dyslexic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84090/new/ https://reviews.ll

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread Anders Waldenborg via Phabricator via cfe-commits
wanders marked an inline comment as done. wanders added a comment. In D84090#2160411 , @curdeius wrote: > The changes look good to me in general. I share your doubt though about > whether a bool flag is sufficient here. We've seen in the past a few times

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. The changes look good to me in general. I share your doubt though about whether a book flag is sufficient here. We've seen in the past a few times that at some time a false/true flag is not enough. I'd rather go for a Before/After/Both/None flag (or similar, naming pro

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread Anders Waldenborg via Phabricator via cfe-commits
wanders updated this revision to Diff 279010. wanders added a comment. - Regenerated rst - Fixed clang format errors - Added release note - Added bool parsing test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84090/new/ https://reviews.llvm.org/D8

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. you need to make a full contextdiff git diff -U Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84090/new/ https://reviews.llvm.org/D84090 ___ cfe-commits maili

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In principle, this looks ok. Comment at: clang/unittests/Format/FormatTest.cpp:12163 "int oneTwoThree : 23 = 0;", Alignment); you are missing a CHECK_PARSE_BOOL test Repository: rG LLVM G

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. please clang-format Comment at: clang/include/clang/Format/Format.h:2236 + /// \endcode + bool SpaceAroundBitFieldColon; + We need to regenerate the ClangFormatStyleOptions.rst from this change run clang/docs/tool/dump_format_

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread Anders Waldenborg via Phabricator via cfe-commits
wanders created this revision. wanders added reviewers: MyDeveloperDay, klimek. wanders added a project: clang-format. Herald added a project: clang. This new option allows controlling if there should be spaces around the ':' in a bitfield declaration. Decides if it should be the