Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented IVsEditorFactoryChooser on AbstractEditorFactory class to invoke WinForms designer editor factory directly by VS Shell #77306

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Shyam-Gupta
Copy link
Member

@Shyam-Gupta Shyam-Gupta commented Feb 21, 2025

Implemented IVsEditorFactoryChooser interface on AbstractEditorFactory class. This interface provides a method ChooseEditorFactory which is invoked by VS Shell before CreateEditorInstance method is invoked.

ChooseEditorFactory method provides an opportunity to the current editor factory to sniff the file and return another editor factory guid if required. If an alternate editor factory guid is returned, VS Shell will invoke that editor factory directly. Else it will continue to invoke CreateEditorInstance method of current editor factory. We use this mechanism to check if rguidLogicalView is Designer. If it is designer, then ChooseEditorFactory will return editor factory guid of a new WinForms Editor Factory WinFormsDesignerEditorFactory. This new editor factory will return an instance of IAsyncDocView interface which will enable it to load the designer asynchronously.

We have safeguarded the changes behind a feature flag which will be enabled by default.

…y class. This interface provides a method 'ChooseEditorFactory' which is invoked by VS Shell before 'CreateEditorInstance' method is invoked.

'ChooseEditorFactory' method provides an opportunity to the current editor factory to sniff the file and return another editor factory guid if required. If an alternate editor factory guid is returned, VS Shell will invoke that editor factory directly. Else it will continue to invoke 'CreateEditorInstance' method of current editor factory.
We use this mechanism to check if 'rguidLogicalView' is 'Designer'. If it is designer, then ChooseEditorFactory will return editor factory guid of a new WinForms Editor Factory.

We have safeguarded the changes behind a feature flag which will be enabled by default.
@Shyam-Gupta Shyam-Gupta requested a review from a team as a code owner February 21, 2025 08:18
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 21, 2025
@dotnet-policy-service dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Feb 21, 2025
@jasonmalinowski
Copy link
Member

@Shyam-Gupta Once this is rolled out fully, would this mean we can delete all the other code we have in our editor factory that special cases WinForms by simply always delegating to the other editor factory?

@jasonmalinowski
Copy link
Member

I've restarted the integration test failures, but they weren't looking entirely optimistic:

Description: The process was terminated due to an unhandled exception.
Exception Info: System.AccessViolationException
   at EnvDTE.ProjectItem.Open(System.String)
   at Microsoft.VisualStudio.TemplateWizard.Wizard.OpenNotedItems(System.String, System.Collections.Generic.List`1<Microsoft.VisualStudio.TemplateWizard.WizardExtension>, EnvDTE.Project)
   at Microsoft.VisualStudio.TemplateWizard.Wizard.Execute(System.Object, IntPtr, System.Object[] ByRef, System.Object[] ByRef, EnvDTE.wizardResult ByRef)
   at Microsoft.VisualStudio.CommonIDE.Solutions.Interop.IVsNativeEnvironmentInternal.WizLoadAndRunFile(System.String, System.String, Boolean, Boolean, System.Array, System.String)
   at Microsoft.VisualStudio.CommonIDE.Solutions.Dte.DteSolution+<AddFromTemplateExAsync>d__248.MoveNext()
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].Start[[Microsoft.VisualStudio.CommonIDE.Solutions.Dte.DteSolution+<AddFromTemplateExAsync>d__248, Microsoft.VisualStudio.CommonIDE, Version=17.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<AddFromTemplateExAsync>d__248 ByRef)
   at Microsoft.VisualStudio.CommonIDE.Solutions.Dte.DteSolution.AddFromTemplateExAsync(System.String, System.String, System.String, System.String, Boolean, UInt32, System.Threading.CancellationToken)
   at Microsoft.VisualStudio.CommonIDE.Solutions.Dte.DteSolution+<>c__DisplayClass247_0.<AddFromTemplateEx>b__0()
   at Microsoft.VisualStudio.Threading.JoinableTaskFactory.ExecuteJob[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]](System.Func`1<System.Threading.Tasks.Task>, Microsoft.VisualStudio.Threading.JoinableTask)
   at Microsoft.VisualStudio.Threading.JoinableTaskFactory.RunAsync[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]](System.Func`1<System.Threading.Tasks.Task`1<System.__Canon>>, Boolean, System.String, Microsoft.VisualStudio.Threading.JoinableTaskCreationOptions)
   at Microsoft.VisualStudio.Threading.JoinableTaskFactory.Run[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]](System.Func`1<System.Threading.Tasks.Task`1<System.__Canon>>, Microsoft.VisualStudio.Threading.JoinableTaskCreationOptions)
   at Microsoft.VisualStudio.CommonIDE.Solutions.Dte.DteSolution.AddFromTemplateEx(System.String, System.String, System.String, System.String, Boolean, UInt32)

So that could be related to a crash in the editor factory since it was trying to open a file.

@Shyam-Gupta
Copy link
Member Author

@Shyam-Gupta Once this is rolled out fully, would this mean we can delete all the other code we have in our editor factory that special cases WinForms by simply always delegating to the other editor factory?

Yes thats right. Once it all works as expected, I will share another PR to cleanup that code.

…s to succeed as the VS build against which the tests are running, does not have WinForms inproc designer side of changes yet.
@Shyam-Gupta
Copy link
Member Author

I've restarted the integration test failures, but they weren't looking entirely optimistic:

Description: The process was terminated due to an unhandled exception.
Exception Info: System.AccessViolationException

So that could be related to a crash in the editor factory since it was trying to open a file.

Its happening because the VS build against which integration tests are running does not have WinForms inproc designer side of changes yet. To workaround this issue, I have set defaultValue = false for the feature flag. This way the tests will use earlier code path which doesn't utilize the new editor factory. It will use the new EF once the change gets inserted into VS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants