sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/unittests/TweakTesting.cpp:74 + return true; + llvm::consumeError(PrepareResult.takeError()); + return false; ---------------- ilya-biryukov wrote: > Maybe print the input in case of failure? This is a matcher against the input, so gtest will always print it. ================ Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:44 + // Snippet is an expression. + Expression, + }; ---------------- ilya-biryukov wrote: > I wonder whether we could use `Function` and get rid of 'expression' mode > completely? WDYT? > One can easily turn an expression into a statement by adding a semicolon. We could, but I think it obfuscates intent a little bit, and doesn't really simplify anything (we're already paying for the function wrapping logic, adding a couple more strings is ~free) ================ Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:86 + for (const auto &Case : expandCases(MarkedCode)) \ + EXPECT_THAT(Case, isAvailable()) + ---------------- ilya-biryukov wrote: > ilya-biryukov wrote: > > NIT: add a level of indent > NIT: fully-qualify in a macro `::clang::clangd::TweakTest::isAvailable()` Indentation is clang-format's fault. Wrapped it in the do-while-0 to fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65525/new/ https://reviews.llvm.org/D65525 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
