Use a custom CodeActionParams to prevent NRE in Web Tools #8207
Conversation
| var request = new VSCodeActionParams() | ||
| { | ||
| TextDocument = new TextDocumentIdentifier { Uri = new Uri(documentPath) }, | ||
| TextDocument = new VSTextDocumentIdentifier { Uri = new Uri(documentPath) }, |
There was a problem hiding this comment.
For future reference, is there any reason why we should ever do TextDocumentIdentifier instead of VSTextDocumentIdentifier? It seems like if someone doesn't want the extra VS stuff (and IIRC it's all additive) they'll just drop it when they serialize right?
There was a problem hiding this comment.
Personally, I think this whole PR shouldn't exist and we should never use VS* types. I think its a failing of the platform that we have to.
There was a problem hiding this comment.
Internestingly Web Tools has a VSCodeActionParams of their own, presumably to acheive the same thing as this PR, but they use TextDocumentIdentifier. I'm just more lazy and did a direct copy/paste.
|
Thanks for reviews.. Web Tools don't need this is P1, so i'm rebasing and fixing conflicts... |
davidwengier
force-pushed
the
SendVSInternalTypesToWebTools
branch
from
60a38ad to
56a703e
Compare
|
Nope, gonna move this back to 17.6P1, since it fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1733577 |
davidwengier
force-pushed
the
SendVSInternalTypesToWebTools
branch
from
56a703e to
ab789e6
Compare
|
lol that was a fun github-ism.. i targeted release/dev17.6, builds passed. I retargeted main, set auto-complete, builds failed. I retargeted release/dev17.6, PR auto-merged without any builds. Hopefully things will be fine!! |
3 participants
We're losing data between our endpoint and middleware (custom message target) because the platform converters aren't used for writing.
Poiinting this to release/dev17.6 on the assumption web tools wants it for M2. @ToddGrun @jimmylewis let me know if P2 is okay.