[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think you can spin it any number of ways, but bottom line its a massive truth table and I sort of feel however you change it, it will just become a different equally incomprehensible pattern. For me I'd prefer to stick with the devil I know, unless its broken,

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. FWIW I proposed to go this way: https://discourse.llvm.org/t/clang-format-spacerequiredbefore-vs-spacerequiredbetween/60901/5?u=hazardyknusperkeks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121755/new/ https:

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-18 Thread sstwcw via Phabricator via cfe-commits
sstwcw added a comment. By the way, last time I used a breakpoint on `spaceRequiredBefore` and stepped until return. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121755/new/ https://reviews.llvm.org/D121755 __

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D121755#3391606 , @MyDeveloperDay wrote: > This just made a 300 lines function and a 500 line function with minimal > comments into a 800 line function.. For no real benefit? > > Because from what I can tell you haven't worke

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. This just made a 300 lines function and a 500 line function with minimal comments into a 800 line function.. For no real benefit? Because from what I can tell you hav

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-16 Thread sstwcw via Phabricator via cfe-commits
sstwcw updated this revision to Diff 416025. sstwcw added a comment. Use the name spaceRequiredBetween. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121755/new/ https://reviews.llvm.org/D121755 Files: clang/lib/Format/TokenAnnotator.cpp clang

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3069 -bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line, - const FormatToken &Left, - co

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. No objections here, but I need to see other patches to understand where it takes us before accepting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121755/new/ https://reviews.llvm.org/D121755 ___

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

2022-03-15 Thread sstwcw via Phabricator via cfe-commits
sstwcw created this revision. sstwcw added a reviewer: clang-format. Herald added a project: All. sstwcw requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. After all these years, having the two functions now serves to confuse people. Reposit