djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land.
Some remarks, but looks good. ================ Comment at: unittests/Format/CleanupTest.cpp:38 @@ +37,3 @@ + // Returns code after cleanup around \p Offsets. + std::string cleanupAroundOffsets(llvm::ArrayRef<unsigned int> Offsets, + llvm::StringRef Code) { ---------------- I thin we normally just used "unsigned" instead of "unsigned int". ================ Comment at: unittests/Format/CleanupTest.cpp:39 @@ +38,3 @@ + std::string cleanupAroundOffsets(llvm::ArrayRef<unsigned int> Offsets, + llvm::StringRef Code) { + std::vector<tooling::Range> Ranges; ---------------- nit: alignment. ================ Comment at: unittests/Format/CleanupTest.cpp:125 @@ -123,7 +124,3 @@ std::string Expected = "class A {\nA() {} };"; - std::vector<tooling::Range> Ranges; - Ranges.push_back(tooling::Range(17, 0)); - Ranges.push_back(tooling::Range(19, 0)); - std::string Result = cleanup(Code, Ranges); - EXPECT_EQ(Expected, Result); + EXPECT_EQ(Expected, cleanupAroundOffsets({17, 19}, Code)); ---------------- I think for such short test cases, at least, I'd just inline the string literals, e.g.: EXPECT_EQ("class A {\nA() {} };", cleanupAroundOffsets({17, 19}, "class A {\nA() : , {} };")); But not sure whether it's better. https://reviews.llvm.org/D24501 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits