Conversation
| assert.DeepEqual(t, *actualConfig2.JS(), *expectedPrefs2) | ||
| }) | ||
|
|
||
| t.Run("definition with stale position after file shrink does not panic", func(t *testing.T) { |
There was a problem hiding this comment.
Is this not an entirely illegal situation? The client can't give us positions that our out of range for its own view...
There was a problem hiding this comment.
Fair, I mentioned this is is a tradeoff here: #2966 (comment) . I think that in the long run this should return an explicit error (instead panicing) if the drift like this is not acceptable (which is a fair stance!).
As to the "proper" solution. I suspect this will require "snapshot freezing" like the one in #2905 . Once that PR settles - I'll take another look at this specific crash here again.
| char := core.TextPos(lineAndCharacter.Character) | ||
|
|
||
| if line < 0 || int(line) >= len(lineMap.LineStarts) { | ||
| panic(fmt.Sprintf("bad line number. Line: %d, lineMap length: %d", line, len(lineMap.LineStarts))) |
There was a problem hiding this comment.
A rogue client can always send some invalid position. It's definitely a tradeoff and we could return an error instead but clamping to the bounds seems somewhat benign to me here.
|
It is possible/probable that we will eventually do something like this (or at least silently error in the output pane), but I think right now we do want to find which sorts of issues are coming up so we can either figure out what we're doing wrong in request handling, or in VS Code's language server client. |
|
superseded by #3365 |
3 participants
fixes #2959