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

Make Settings usable in .NET scenarios #7647

Merged
merged 3 commits into from
Sep 30, 2021
Merged

Make Settings usable in .NET scenarios #7647

merged 3 commits into from
Sep 30, 2021

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Sep 29, 2021

Changes

  1. Pin configuration type names to the desktop identities so not only we can create necessary configuration sections and serialize data, but also that we can deserialize those as well.
    Affected types:

    • System.Configuration.ApplicationSettingsGroup
    • System.Configuration.ClientSettingsSection
    • System.Configuration.UserSettingsGroup

    Fixes Updating the Default Settings file in a WinForms .NET 5 Visual Basic app fails. #6784
    Resolves Error when trying to create Settings file (VB.Net) winforms#4308

  2. Remove ability to browse and serialize arbitrary types, and restrict the set of type to the the known list

    Resolves https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1388857/

Tests

Manual using a multi-project solution including

  • .NET Framework C# 4.0 (Console) and C# 4.7.2 (WinForms),
  • .NET C# (WinForms) 3.1, 5.0, 6.0
  • .NET VB (WinForms) 5.0, 6.0
settings400.mp4

/cc: @dotnet/dotnet-winforms

Microsoft Reviewers: Open in CodeFlow

* Pin configuration type names to the desktop identities so not only we
can create necessary configuration sections and serialize data, but also
that we can deserialize those as well.
Affected types:
    - System.Configuration.ApplicationSettingsGroup
    - System.Configuration.ClientSettingsSection
    - System.Configuration.UserSettingsGroup

Fixes #6784
Resolves dotnet/winforms#4308

* Remove ability to browse and serialize arbitrary types, and restrict
the set of type to the the known list

Resolves https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1388857/
@RussKie RussKie requested a review from a team as a code owner September 29, 2021 07:08
@ghost ghost added the Feature-WinForms Features related to bringing up the Windows Forms designer and related features. label Sep 29, 2021
@RussKie RussKie added the Feature-Settings-Designer Specific to the settings file designer. label Sep 29, 2021
@tmeschter
Copy link
Contributor

This seems like a very reasonable approach to get the designer basically functional for Dev17.0 as we look into more general solutions for 17.1.

@RussKie Did you test with a project that multi-targets to both .NET Framework and .NET Core (e.g. .NET Framework 4.7.2 and .NET Core 5.0)? If so, did you test with different orderings of items in <TargetFrameworks>? E.g., try both

<TargetFrameworks>net472;net5.0</TargetFrameworks>

and

<TargetFrameworks>net5.0;net472</TargetFrameworks>

There are, unfortunately, times when the order matters. :-(

Also, did you verify this still works with a non-SDK-style project targeting .NET Framework?

@KlausLoeffelmann
Copy link
Member

This is really cool! Also @KathleenDollard FYI!

@merriemcgaw
Copy link
Member

There are, unfortunately, times when the order matters. :-(

Yeah, that's a frequent problem for designer users. We can only display one designer at a time - so when a .NET Framework TFM is loaded, we are only working with the .NET Framework designer and everything builds etc as it did before. I would definitely want to check that the .NET Framework stuff isn't impacted, though it shouldn't be. We should check that also for CSProj, good call.

@RussKie
Copy link
Member Author

RussKie commented Sep 30, 2021

Did you test with a project that multi-targets to both .NET Framework and .NET Core (e.g. .NET Framework 4.7.2 and .NET Core 5.0)?

No, forgot about this. Will check 👍

did you verify this still works with a non-SDK-style project targeting .NET Framework?

Yes, .NET Framework C# 4.0 (Console). No observable changes.

@RussKie
Copy link
Member Author

RussKie commented Sep 30, 2021

I've created more test apps including those targeting .NET Framework 2.0, 3.5, 4.0, 4.7.2, .NET Core 3.1, .NET 5.0 and 6.0. As well as MT scenarios (e.g. net35,net472,net6.0-windows and net6.0-windows;net472). Everything appears to be working expected.

Test solution: ProjectSystemTestHarness.zip


The only unexpected behaviour I observed was that for net35,net472,net6.0-windows scenario settings were written as 4.0.0.0 and not 2.0.0.0, but this is an existing behaviour:
image

There's probably a bug, but I guess this scenario is probably in the realms of unsupported edge cases.

@tmeschter
Copy link
Contributor

Merging. FYI @RussKie @merriemcgaw @jjmew We'll need to take this through M2 approval and possibly QB approval.

@tmeschter tmeschter merged commit 5127cac into main Sep 30, 2021
@tmeschter tmeschter deleted the Make_Settings_usable branch September 30, 2021 16:34
@ghost ghost added this to the 17.0 milestone Sep 30, 2021
@RussKie
Copy link
Member Author

RussKie commented Oct 1, 2021

Thank you 🎉

@merriemcgaw
Copy link
Member

@Pilchie as FYI in case I'm still oof sick tomorrow.

@Pilchie
Copy link
Member

Pilchie commented Oct 8, 2021

So, did this make it into 17.0 yet, or do we still need another bug/PR here?

@MiYanni
Copy link
Member

MiYanni commented Oct 8, 2021

@Pilchie This was merged into main on Oct 1st. Main (at that point) was still for 17.0 changes, so these changes should be in 17.0. Insertion here: https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/355436

@Pilchie
Copy link
Member

Pilchie commented Oct 8, 2021

Thanks for confirming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-Settings-Designer Specific to the settings file designer. Feature-WinForms Features related to bringing up the Windows Forms designer and related features.
Projects
None yet
6 participants