MyDeveloperDay added a comment. Thank you for the patch, was there a github issue for this? Can we just validate that those requirements are covered here too while we are at it?
https://github.com/llvm/llvm-project/issues/54703 https://github.com/llvm/llvm-project/issues/41501 ( I know this is yours, but ensure you mention it in the summary!) so the commit gets tied to the review ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:3399 + return Style.SpaceBeforeParensOptions.AfterPlacementNew || + spaceRequiredBeforeParens(Right); if (Line.Type == LT_ObjCDecl) ---------------- what case is covered here by spaceRequiredBeforeParens(Right)? ================ Comment at: clang/unittests/Format/FormatTest.cpp:15276 + FormatStyle SpacePlacementNew = getLLVMStyle(); + SpacePlacementNew.SpaceBeforeParens = FormatStyle::SBPO_Custom; ---------------- curdeius wrote: > Are there any tests with `AfterPlacementNew = false;`? Could you add those > please? you should probably give yourself your own test, for SpacePlacementNew Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127270/new/ https://reviews.llvm.org/D127270 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
