[FUSE] Fix OnAutoInsert and override completion and possible others #11122
Conversation
|
Oh, at runtime when I manually validate this, Roslyn is sending us a response that includes the entire generated document. That doesn't happen in the test, even when its failing, so, I don't know, but I wanted to mention it because it's really weird. |
| bool isCodeActionFormattingRequest, | ||
| bool automaticallyAddUsings, |
There was a problem hiding this comment.
@phil-allen-msft you were absolutely right to complain about the naming of this variable, and there is a slim chance that if you'd pushed harder, I might have thought more about it and realised the association it was making was insufficient, and there was more to fix than just code actions.
ie, it's all your fault 😛
|
Can't comment on the PR as I'm not familiar with this code at all, but can confirm the Preview FUSE option (? Force use of runtime code generation for Razor) was enabled for me. Thanks for getting this sorted - happy to test if there's something of value I can add. R |
|
Going to retarget to 17.13 P1 |
Sadly, to get this far I had to opt cohosting into FUSE, which will have untold consequences
Previously this was using a boolean flag that I thought was the correct pivot point, but I was wrong, and it made things very confusing. This is much clearer, and more importantly, correct. Essentially if we are trying to format and we get passed a C# edit, then we can't force design time or it would be a different document than the original edit came from, if FUSE is on.
This was returning the passed in changes, which were for a C# document, when the formatting engine should be producing changes for the Razor document, so that was wrong. Additionally, the guard itself was wrong
Since this service was written, IDocumentSnapshot has been updated such that now, all implementations just passed true for a parameter that is part of the existing API
davidwengier
force-pushed
the
FixAutoInsertEtc
branch
from
7240539
to
5838cd5
Compare
|
Thank you for confirming @richardhauer. One quick question if you don't mind: Do you remember if you checked that checkbox yourself? We don't think you should have been opted in to it automatically, so would be good to confirm if that is something we need to look in to. |
|
I definitely didn't check it recently, but I do often opt into preview features for razor as I work with Blazor a lot and these (usually) improve my workflow. |
|
Whoops! Not sure I should have merged this @phil-allen-msft or if I was supposed to wait. Should make it in to M2 so hopefully not a big deal :D |
Fixes #11112
Hopefully fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2290299/ but I'm not sure, as whilst OnAutoInsert was definitely broken for me, it didn't cause that repro. It just sent telemetry for a fault and did nothing in the IDE.
Fixes https://developercommunity.visualstudio.com/t/Razor-editor-is-broken-in-1712-Preview-/10777527
but awaiting confirmation that FUSE is on for the user who reported the issue (@richardhauer I don't suppose that's you?)Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2294069
Sadly, in order to get a working test for this, I had to enable FUSE in cohosting. I say sadly because whilst it proves the cohosting tests are awesome, it does potentially break us into jail a little bit. But only in cohosting, so it's a pretty small jail :)
The changes aren't very exciting, as this is just #10967 but for more than code actions (and these changes probably should have been part of that PR originally 🤦♂️), but each commit has an explanation for why it's there so commit-at-a-time might be enlightening. I could not resist a little clean up, but what's one little deleted service between friends?