ilya-biryukov added inline comments.
================
Comment at: clangd/CodeComplete.cpp:289
}
+ std::stable_sort(Completion.FixIts.begin(), Completion.FixIts.end(),
+ [](const TextEdit &X, const TextEdit &Y) {
----------------
We shouldn't have duplicate/overlapping fix-its, right? Maybe use `std::sort`?
================
Comment at: clangd/CodeComplete.cpp:291
+ [](const TextEdit &X, const TextEdit &Y) {
+ return X.range.start.line < Y.range.start.line ||
+ X.range.start.character <
----------------
use built-in tuples comparison `std::tie(X.line, X.column) < std::tie(Y.line,
Y.column)`?
================
Comment at: clangd/CodeComplete.cpp:1057
+ Range TextEditRange;
+ if (CodeCompletionRange.isValid()) {
+ TextEditRange = halfOpenToRange(Recorder->CCSema->getSourceManager(),
----------------
Maybe add a comment describing the cases where `isValid()` is false?
Does it happen when we complete with empty identifier?
================
Comment at: clangd/CodeComplete.cpp:1310
+ // other.
+ for (const auto &FixIt : FixIts) {
+ if (IsRangeConsecutive(FixIt.range, LSP.textEdit->range)) {
----------------
Maybe keep the `reserve` call? (we could reserve one extra element, but that's
fine)
================
Comment at: clangd/CodeComplete.h:126
+ TextEdit textEdit;
+
----------------
Could we avoid adding `textEdit` here?
It's an intermediate representation that is only used in clangd, and we should
be able materialize the actual `textEdit` when converting to LSP.
================
Comment at: unittests/clangd/SourceCodeTests.cpp:40
+Range range(const std::pair<uint64_t, uint64_t> p1,
+ const std::pair<uint64_t, uint64_t> p2) {
----------------
we convert those `uint64_t` into `int` when setting the positions anyway.
Maybe use `int` here too?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50449
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
