Fix the KeybindingResetDetector#60494
Conversation
We switched VisualStudioInfoBar to no longer be a MEF service in 347cd02, but this was still importing it. It's now a helper type, so create it instead.
How did that not break everything? e.g. i would have expected some sort of exceptions happening that should have caused problems. Do we have an exception hole somewhere? |
| public KeybindingResetDetector( | ||
| IThreadingContext threadingContext, | ||
| IGlobalOptionService globalOptions, | ||
| VisualStudioInfoBar infoBarService, |
There was a problem hiding this comment.
i don't get it. shouldn't this have broken mef? how was this type getting instantiated? and how did this not break anything else importing KeybindingResetDetector?
There was a problem hiding this comment.
This type is only instantiated here:
roslyn/src/VisualStudio/Core/Def/RoslynPackage.cs
Lines 260 to 261 in 91bd6e5
There was a problem hiding this comment.
This line is hiding it:
. It's just a Forget - no fault or anything.
CyrusNajmabadi
left a comment
There was a problem hiding this comment.
Something doesn't make sense to me here.
|
@CyrusNajmabadi: It throws exceptions here: roslyn/src/VisualStudio/Core/Def/RoslynPackage.cs Lines 260 to 261 in 91bd6e5 Which is a fire-and-forget path. |
|
bleaghhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh. |
|
@CyrusNajmabadi Updated to report non-fatal telemetry if that throws. |
|
Some testing makes us think this feature is more generally broken; until we understand why we'll merge this for 17.3 to avoid the MEF composition issue, and we'll need to investigate further. |
We switched VisualStudioInfoBar to no longer be a MEF service in 347cd02, but this was still importing it. It's now a helper type, so create it instead.