Details
- Reviewers
hokein - Commits
- rGb31486ad9717: [clangd] textDocument/implementation (LSP layer)
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
just nits.
Looks like the patch is based on the old revision (pre-merging tests are failing), I assume you have fixed the failure tests last week?
clang-tools-extra/clangd/ClangdLSPServer.cpp | ||
---|---|---|
615 | nit: move it to line 605 (right after definitionProvider). | |
1443 | nit: onGoToImplementation, move a line up to be closer with Go-to-{declaration/definition}. | |
clang-tools-extra/clangd/Protocol.h | ||
1482 | nit: since this is just a mirror of TextDocumentPositionParams, I'd use TextDocumentPositionParams directly (looks like this is a pattern of other features, go-to-definition etc.) |
Looks like the patch is based on the old revision (pre-merging tests are failing), I assume you have fixed the failure tests last week?
Yes. That was fixed last week. Rebased.
clang-tools-extra/clangd/ClangdLSPServer.cpp | ||
---|---|---|
1303 | any particular reason for preferring declarations here that I am missing (in presence of a definition)? |
nit: move it to line 605 (right after definitionProvider).