Conversation
| var utf16Char core.TextPos | ||
|
|
||
| for i, r := range script.Text()[start:] { | ||
| for i, r := range script.Text()[start:lineEnd] { |
There was a problem hiding this comment.
This was actually a preexisting bug; we were not stopping at the end of lines. Oops...
There was a problem hiding this comment.
I think if you do this you may technically break the fuzzer. 😅
There was a problem hiding this comment.
There was a problem hiding this comment.
| if int(line) >= len(lineMap.LineStarts) { | ||
| return textLen | ||
| } | ||
|
|
There was a problem hiding this comment.
What if we errored in debug builds?
There was a problem hiding this comment.
We don't have that concept anymore. Only "debug builds" that are deoptimized for better delve debugging.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR makes LSP position conversions more defensive by clamping out-of-range inputs instead of panicking, improving interoperability with clients that send invalid positions.
Changes:
- Clamp line/character inputs in
LineAndCharacterToPositionto valid text bounds. - Clamp
positionand computedlineinPositionToLineAndCharacterto prevent out-of-range behavior. - Avoid iterating beyond the current line when converting UTF-16 character offsets.
|
Uh oh, that test failure is fun, I swear I ran tests locally before! |
|
I had a similar one here: #2966 - maybe you could recycle some of its tests or something |
|
Ah, yeah... I truthfully forgot |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
As much as I hate it, we probably have to do this to deal with clients that are sending out of range requests.
Hopefully this isn't masking any lingering unicode bugs. I don't think so, though...