malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
================
Comment at: clangd/ClangdLSPServer.cpp:228
llvm::toString(Items.takeError()));
+
C.reply(json::ary(Items->Value));
----------------
remove extra line
================
Comment at: clangd/ClangdUnit.cpp:1054
+ return llvm::errc::no_such_file_or_directory;
+ } else {
+ FileID FID = AST.getASTContext().getSourceManager().translateFile(F);
----------------
else is not needed because it returns in the if
================
Comment at: clangd/ClangdUnit.cpp:1080
+ return llvm::errc::no_such_file_or_directory;
+ } else {
+ SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(),
----------------
else is not needed since you return in the if
================
Comment at: clangd/ClangdUnit.cpp:1209
+ H.range = L->range;
+ else
+ H.range = Range();
----------------
```
else
H.range = Range();
```
Remove this, once the bug in Protocol.cpp is fixed, the Range will be truly
optional.
================
Comment at: clangd/ClangdUnit.cpp:1247
+ H.range = L->range;
+ else
+ H.range = Range();
----------------
```
else
H.range = Range();
```
Remove this, once the bug in Protocol.cpp is fixed, the Range will be truly
optional.
================
Comment at: clangd/ClangdUnit.cpp:1505
*CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
- /*StoreInMemory=*/That->StorePreamblesInMemory,
- SerializedDeclsCollector);
+ /*StoreInMemory=*/true, SerializedDeclsCollector);
----------------
revert this change
================
Comment at: clangd/Protocol.cpp:498
+ // Default Hover values
+ Hover H;
+ return json::obj{
----------------
remove, we have to return the contents of the H that was passed as parameter,
not a new one. I hit this bug while testing with no range (hover text in
another file)
So this should be
```
if (H.range.hasValue()) {
return json::obj{
{"contents", json::ary(H.contents)},
{"range", H.range.getValue()},
};
}
return json::obj{
{"contents", json::ary(H.contents)},
};
```
================
Comment at: clangd/Protocol.h:26
#include "llvm/ADT/Optional.h"
-#include <string>
+#include "llvm/Support/YAMLParser.h"
#include <vector>
----------------
revert this change?
================
Comment at: clangd/Protocol.h:463
+
+ /**
+ * The hover's content
----------------
Documentation should use /// like the others
================
Comment at: clangd/Protocol.h:468
+
+ /**
+ * An optional range is a range inside a text document
----------------
Documentation should use /// like the others
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D35894
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
