provide proper start and end positions in the LSP protocol#2655
Conversation
|
@swift-ci please test |
rintaro
left a comment
rintaro
left a comment
There was a problem hiding this comment.
Nice fix.
Please update the test cases 🙏
| location = Location( | ||
| uri: uri, | ||
| range: Range(uncheckedBounds: (lower: positionStart, upper: positionEnd)) | ||
| ) |
There was a problem hiding this comment.
Could you use ..<? There's no reason to use "unchecked" version here.
| location = Location( | |
| uri: uri, | |
| range: Range(uncheckedBounds: (lower: positionStart, upper: positionEnd)) | |
| ) | |
| location = Location(uri: uri, range: positionStart..<positionEnd) |
| let line: Int = dict[keys.line], | ||
| let column: Int = dict[keys.column] | ||
| let column: Int = dict[keys.column], | ||
| let length: Int = dict[keys.length] |
There was a problem hiding this comment.
We know length is always there in cursorInfo response, but just to be on the safe side, could you do make it defaulted to zero? i.e. (dict[keys.length] ?? 0), so it's not going to be a part of if conditions.
apologies, I'm inexperienced with the tooling - it showed green checkmarks at the end, but that doesn't actually seem to be a global summary. |
femaref
force-pushed
the
feature/proper-end
branch
from
8b996cc to
3b4a908
Compare
|
@swift-ci Please test |
|
I just realized it's more than that test failing. |
femaref
force-pushed
the
feature/proper-end
branch
from
3b4a908 to
0021490
Compare
| } | ||
|
|
||
| let location: Location? | ||
| let length = (dict[keys.length] ?? 0) |
There was a problem hiding this comment.
Please move this inside if let snapshot { branch.
vscode provides an alternative action if the target of goto definition is in the same character range we requested. this requires the start and end in the LSP response to be properly set, so vscode can see that the current character is actually part of the request
femaref
force-pushed
the
feature/proper-end
branch
from
0021490 to
c4c5522
Compare
|
I think I have now caught everything. I'm still getting two more failures, but those also happen on
|
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
Those 2 tests fails if you're testing against older SDK where swiftlang/swift#87020 has not been applied. We probably should update the tests to accept both signatures... |
2 participants
vscode provides an alternative action if the target of goto definition is in the same character range we requested. this requires the start and end in the LSP response to be properly set, so vscode can see that the current character is actually part of the request. Currently, this functionality only works when invoking "go to definition" on the start of the character range.
broken.mp4
fixed.mp4