ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM, with a few NITs.
Thanks for the change!
================
Comment at: clangd/Quality.h:81
+ bool NeedsFixIts =
+ false; // Whether fixits needs to be applied for that completion or not.
----------------
NIT: put the comment at the previous line to keep `false` at the same line.
NIT2: use three slashes, to make it a doxygen comment (previous one should also
be three slashes).
================
Comment at: unittests/clangd/CodeCompleteTests.cpp:99
+
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
----------------
It seems this comment got moved accidentally, move it back?
================
Comment at: unittests/clangd/QualityTests.cpp:360
+ RelevanceWithFixIt.merge(
+ CodeCompletionResult(&findDecl(AST, "x"), 0, nullptr, false, true,
{{}}));
+ EXPECT_TRUE(RelevanceWithFixIt.NeedsFixIts);
----------------
NIT: took me some time to figure out the difference between this and the next
one.
Maybe add the explicit type name to make more implicit that we're creating a
fix-it here, i.e. `{FixItHint{}}`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50193
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
