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.

Sign up for GitHub

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

Jump to bottom

Correctly show supported frameworks for .NETCore WPF and WinForms projects #5011

Closed
wants to merge 1 commit into from
Closed

Correctly show supported frameworks for .NETCore WPF and WinForms projects #5011

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/Microsoft.VisualStudio.Editors/PropPages/ApplicationPropPageInternalBase.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Imports System.Windows.Forms

Imports Microsoft.VisualStudio.Editors.Common
Imports Microsoft.VisualStudio.Shell.Interop
Imports Microsoft.VisualStudio.Shell

Imports NativeMethods = Microsoft.VisualStudio.Editors.Interop.NativeMethods

Expand Down Expand Up @@ -126,15 +127,21 @@ Namespace Microsoft.VisualStudio.Editors.PropertyPages
Try
Dim siteServiceProvider As OLE.Interop.IServiceProvider = Nothing
VSErrorHandler.ThrowOnFailure(ProjectHierarchy.GetSite(siteServiceProvider))
Dim sp As New Shell.ServiceProvider(siteServiceProvider)
Dim sp As New ServiceProvider(siteServiceProvider)
Dim vsFrameworkMultiTargeting As IVsFrameworkMultiTargeting = TryCast(sp.GetService(GetType(SVsFrameworkMultiTargeting).GUID), IVsFrameworkMultiTargeting)
' TODO: Remove IsTargetFrameworksDefined check after issue #800 is resolved.
If (TargetFrameworksDefined() = False And vsFrameworkMultiTargeting IsNot Nothing) Then

Dim supportedTargetFrameworksDescriptor = GetPropertyDescriptor("SupportedTargetFrameworks")
Dim supportedFrameworks As IEnumerable(Of TargetFrameworkMoniker) = TargetFrameworkMoniker.GetSupportedTargetFrameworkMonikers(vsFrameworkMultiTargeting, DTEProject, supportedTargetFrameworksDescriptor)

Dim isWPForWindowsForms = ProjectHierarchy.IsCapabilityMatch("WPF") Or ProjectHierarchy.IsCapabilityMatch("WindowsForms")

For Each supportedFramework As TargetFrameworkMoniker In supportedFrameworks
Dim frameworkName As New FrameworkName(supportedFramework.Moniker)
If isWPForWindowsForms And frameworkName.Version.Major < 3 And String.Equals(frameworkName.Identifier, ".NETCoreApp") Then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if the framework identifier is case sensitive, but its worth finding out and passing in the right StringComparer.

Also, using AndAlso instead of And will short circuit and be infinitesimally faster :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nguerrera @dsplaisted Can we please introduce handshake between us and the SDK so that we don't have business logic in the UI layer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to work on the right handshake, but we need to understand what information the project system will need. FWIW, I can retarget a .NET Framework WPF app to .NET Framework 2.0 in Visual Studio, so the equivalent check doesn't exist for .NET Framework and I'm not sure that we necessarily need it for .NET Core.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, .NET Framework 2.0 does appear in the dropdown, but when you try to retarget it fails with the following dialog:

Attempted re-targeting of the project has been canceled. Required assemblies 'WindowsBase', 'PresentationCore', and 'PresentationFramework' are missing from the target framework.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The listis populated from items in the sdk, right. Should we exclude/remove as appropriate in sdk?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That check from legacy only runs in WPF flavor, but uses IVsFrameworkMultiTargeting to figure if those assemblies are present in the target framework. We need a data driven approach to avoid hardcoding these specific target frameworkism's in the project-system/UI layer, ie "Does this FrameworkReference exist in this TFM?"

Copy link
Member

@davkean davkean Jul 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach, simialr to what we did we project references, is give up UI validation at all, and just have the build tell you that these references cannot be referenced in the target. I like that approach because it works everywhere regardless of how you edit the project.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a light investigation it seems that the supported TFM's are added here but I am not sure it is possible to determine if it is a WPF or WinForms project at that time, however

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue here dotnet/sdk#3438 to discuss this further

Continue For
End If
targetFrameworkComboBox.Items.Add(supportedFramework)
Next

Expand Down