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).